Add metadata forwarder #1485
Add metadata forwarder #1485
Conversation
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| func getHeaders() http.Header { | ||
| header := http.Header{} | ||
| header.Set(apiHTTPHeaderKey, os.Getenv("DD_API_KEY")) | ||
| header.Set(useragentHTTPHeaderKey, fmt.Sprintf("datadog-operator/%s", "test")) // TODO: use operator version |
There was a problem hiding this comment.
🟠 Code Quality Violation
| header.Set(useragentHTTPHeaderKey, fmt.Sprintf("datadog-operator/%s", "test")) // TODO: use operator version | |
| header.Set(useragentHTTPHeaderKey, "datadog-operator/test") // TODO: use operator version |
No need to use fmt.Sprintf("%s", var) when var is a string. (...read more)
In Go, it is recommended to avoid using fmt.Sprintf() unnecessarily when printing a string and instead use the string directly. This guideline promotes code simplicity, readability, and execution efficiency. Here are a few reasons why using the string directly is preferred over fmt.Sprintf() for simple string printing:
- Readability: Using the string directly is more concise and easier to read, especially when the format specifiers offered by
fmt.Sprintf()are not needed. By avoiding the additionalfmt.Sprintf()call, the code becomes cleaner and more straightforward, conveying its intention more effectively. - Performance: Invoking
fmt.Sprintf()requires additional CPU cycles and memory allocation. Although the overhead may seem insignificant in isolation, repeated usage or in performance-critical code paths can impact the overall runtime efficiency of your program. By directly printing the string, you eliminate the unnecessary overhead of formatting and allocation associated withfmt.Sprintf(). - Type safety: When using
fmt.Sprintf(), the compiler cannot check the correctness of the format specifiers and arguments. This can potentially lead to runtime errors or incorrect output if the format specifiers or arguments do not match. By directly using the string, you avoid the risk of format string-related errors and ensure type safety.
That said, fmt.Sprintf() can still be useful in scenarios where formatting is needed, such as when building complex strings or including variable values within the output. fmt.Sprintf() offers powerful formatting options and is essential for more advanced string formatting requirements.
In summary, when it comes to simple string printing without any formatting needs or complex variable substitutions, it is best to use the string directly instead of fmt.Sprintf(). This approach promotes code simplicity, better performance, and improved code readability. However, when formatting is necessary, fmt.Sprintf() remains a powerful tool to handle more intricate string construction.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1485 +/- ##
==========================================
- Coverage 48.72% 48.40% -0.32%
==========================================
Files 224 226 +2
Lines 19845 19976 +131
==========================================
+ Hits 9669 9670 +1
- Misses 9668 9798 +130
Partials 508 508
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
| @@ -92,6 +93,7 @@ func SetupControllers(logger logr.Logger, mgr manager.Manager, options SetupOpti | |||
| } | |||
|
|
|||
| if versionInfo != nil { | |||
| mf.OperatorMetadata.KubernetesVersion = versionInfo.String() | |||
There was a problem hiding this comment.
maybe we could define discoveryClient one level up and pass it to SetupControllers, instead of passing mf? (since mf isn't related to controller setup.) then we could declare versionInfo one level up too and pass it to setupMetadataForwarder
| Path: defaultURLPath, | ||
| } | ||
| // prioritize URL set in env var | ||
| if mdURLFromEnvVar := os.Getenv("METADATA_URL"); mdURLFromEnvVar != "" { |
There was a problem hiding this comment.
do we want this to be configurable?
|
|
||
| func getTickerInterval(logger logr.Logger) time.Duration { | ||
| interval := defaultInterval | ||
| if s := os.Getenv("METADATA_INTERVAL"); s != "" { |
There was a problem hiding this comment.
i wonder if we need this to be configurable as well
| now := time.Now().Unix() | ||
|
|
||
| // prioritize custom payload from env var | ||
| if payloadFromEnvVar := os.Getenv("PAYLOAD"); payloadFromEnvVar != "" { |
There was a problem hiding this comment.
i assume this was for testing - do you plan to keep it?
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
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?
Describe your test plan
Write there any instructions and details you may have to test your PR.
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel