Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow forwarder to be used by all controllers #1514

Merged
merged 6 commits into from
Nov 21, 2024
Merged

Conversation

celenechang
Copy link
Contributor

@celenechang celenechang commented Nov 6, 2024

What does this PR do?

  • Define metricsForwarder one level up, outside of the DatadogAgent controller, so it can be used by other controllers
  • Allow metricsForwarder to use credentials configured in the Operator

Motivation

Make metricsForwarder more generic

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

Make sure that metrics forwarder works as expected (datadog.operator metrics are received), under two scenarios:

  • Credentials are not set in the Datadog Operator deployment, but are set in the DatadogAgent CR
  • Credentials are set in both the Datadog Operator deployment and in the DatadogAgent CR

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@celenechang celenechang added enhancement New feature or request refactoring labels Nov 6, 2024
@celenechang celenechang added this to the v1.11.0 milestone Nov 6, 2024
@celenechang celenechang requested a review from a team as a code owner November 6, 2024 21:25
@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 73.91304% with 6 lines in your changes missing coverage. Please review.

Project coverage is 48.64%. Comparing base (b70d772) to head (9556e4c).

Files with missing lines Patch % Lines
internal/controller/setup.go 63.63% 2 Missing and 2 partials ⚠️
internal/controller/datadogagent/controller.go 0.00% 1 Missing ⚠️
internal/controller/datadogagent_controller.go 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1514      +/-   ##
==========================================
+ Coverage   48.63%   48.64%   +0.01%     
==========================================
  Files         227      227              
  Lines       20223    20231       +8     
==========================================
+ Hits         9836     9842       +6     
- Misses       9871     9872       +1     
- Partials      516      517       +1     
Flag Coverage Δ
unittests 48.64% <73.91%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/controller/utils/datadog/forwarders_manager.go 0.00% <ø> (ø)
pkg/controller/utils/datadog/metrics_forwarder.go 31.94% <100.00%> (+0.86%) ⬆️
internal/controller/datadogagent/controller.go 53.57% <0.00%> (ø)
internal/controller/datadogagent_controller.go 67.28% <66.66%> (+0.31%) ⬆️
internal/controller/setup.go 51.85% <63.63%> (-0.82%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b70d772...9556e4c. Read the comment docs.

---- 🚨 Try these New Features:

Copy link
Contributor

@levan-m levan-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one nit comment, otherwise looks good to me!

internal/controller/datadogagent_controller.go Outdated Show resolved Hide resolved
@celenechang celenechang merged commit 715c6f6 into main Nov 21, 2024
20 checks passed
@celenechang celenechang deleted the celene/forwarder branch November 21, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants