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
[ASCII-145] Update pkg/status/render to only handle rendering logic
#20824
Conversation
| } | ||
|
|
||
| aggregatorStats := stats["aggregatorStats"] | ||
| s, err := checkstats.TranslateEventPlatformEventTypes(aggregatorStats) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already call this function when fetching the status in the pkg/status package.
datadog-agent/pkg/status/status.go
Line 335 in 30cff43
| s, err := checkstats.TranslateEventPlatformEventTypes(stats["aggregatorStats"]) |
| if forwarderStatsMap, ok := forwarderStats.(map[string]interface{}); ok { | ||
| forwarderStatsMap["config"] = stats["config"] | ||
| } else { | ||
| log.Warn("The Forwarder status format is invalid. Some parts of the `Forwarder` section may be missing.") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I aggregated all forwarded stats within the same function when fetching the status.
datadog-agent/pkg/status/status.go
Lines 320 to 327 in 30cff43
| forwarderStatsJSON := []byte(expvar.Get("forwarder").String()) | |
| forwarderStats := make(map[string]interface{}) | |
| json.Unmarshal(forwarderStatsJSON, &forwarderStats) //nolint:errcheck | |
| forwarderStorageMaxSizeInBytes := config.Datadog.GetInt("forwarder_storage_max_size_in_bytes") | |
| if forwarderStorageMaxSizeInBytes > 0 { | |
| forwarderStats["forwarder_storage_max_size_in_bytes"] = strconv.Itoa(forwarderStorageMaxSizeInBytes) | |
| } | |
| stats["forwarderStats"] = forwarderStats |
| } | ||
|
|
||
| netflowFunc := func() error { | ||
| if server.IsEnabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move that check to pkg/status GetStatus() function
| return nil | ||
| } | ||
| snmpTrapFunc := func() error { | ||
| if traps.IsEnabled(config.Datadog) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move that check to pkg/status GetStatus() function
| return renderStatusTemplate(b, "/remoteconfig.tmpl", stats) | ||
| } | ||
| otlpFunc := func() error { | ||
| if otlp.IsDisplayed() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move that check to pkg/status GetStatus() function
Go Package Import DifferencesBaseline: ad3cf2e
|
d83219a
to
125e8da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comment, but looks good overall
58dec94
to
fe628cc
Compare
125e8da
to
7b13bb5
Compare
5f9922f
to
65ca3d3
Compare
7b13bb5
to
747e70e
Compare
65ca3d3
to
7d61e71
Compare
747e70e
to
1b08647
Compare
pkg/status/render to only handle rendering logic
…ventPlatformEventTypes twice
…d move config check outside of the render package
1b08647
to
6c8847a
Compare
7134c88
to
05d8059
Compare
What does this PR do?
The PR builds on top of the work done on #20776
Ensure the
status/renderpackage mainly deals with the rendering responsibility.The changes within the PR ensure no extra conversation nor check if a feature is enabled with the render package. Also, remove the indirection created of having different key names on the templates than the ones used when fetching the data.
This work will be convenient to continue extracting the different status sub-packages.
The end goal is that each sub-package is responsible for fetching and rendering the information. by making the render package only handle rending behaviour, it is going to be easier to extract this functionality in the future
Motivation
Additional Notes
Possible Drawbacks / Trade-offs
Describe how to test/QA your changes
The output of the status commands works as expected.
Normal agent
agent statusagent launch-guiCluster agent
agent statusProcess agent
/opt/datadog-agent/embedded/bin/process-agent statusSecurity Agent
agent statusReviewer's Checklist
Triagemilestone is set.major_changelabel if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.changelog/no-changeloglabel has been applied.qa/skip-qalabel is not applied.team/..label has been applied, indicating the team(s) that should QA this change.need-change/operatorandneed-change/helmlabels have been applied.k8s/<min-version>label, indicating the lowest Kubernetes version compatible with this feature.