Render agent status using status component #22115
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Go Package Import DifferencesBaseline: b0f3ede
|
TCheruy
reviewed
Jan 25, 2024
There was a problem hiding this comment.
Traps status is not rendering anymore on my side, working fine in the GUI but not in the CLI 🤔
Conf to reproduce:
network_devices:
namespace: test
snmp_traps:
enabled: true
port: 9162
community_strings:
- public
bind_host: 0.0.0.0
There was a problem hiding this comment.
Thank you so much for reporting the issue.
An issue existed when dynamically accessing the config object inside an fx.Supply function. I changed the way we dynamically provide the different status providers. Here is the commit 6a69662
This is an output using the configuration you provided
TCheruy
approved these changes
Jan 25, 2024
GustavoCaso
force-pushed
the
render-agent-status-using-status-component
branch
from
2b20ac3 to
6a69662
Compare
|
/merge |
|
🚂 MergeQueue Pull request added to the queue. There are 4 builds ahead! (estimated merge in less than 1h) Use |
dd-mergequeue
bot
deleted the
render-agent-status-using-status-component
branch
dd-devflow
bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
This was referenced Jan 26, 2024
10 tasks
This was referenced Feb 26, 2024
This was referenced Mar 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Motivation
Create the different status providers to render the
agent statusoutput, and configure the run command FX dependencies.Logic to decide where the status provider should live
If a component is responsible for that particular section of the status output, create the status provider using FX value groups. It would require adding or changing that particular component return value.
Example: Adding the status provider to the host metadata component
Example: Changing the return value of the forwarder component to include the status provider
If there is no component, we move the status provider to the
pkg/status/*or inside the original package.Example: Move SNMP provider to the pkg/snmp
Example: Create JMX status provider in pkg/status/jmx
Addition to the status component
Added support for Noop providers. These providers would be entirely ignored by the status component and would not render any information.
We needed to propagate the
verboseargument information to the different providers.Important considerations
For components or packages that expose the status provider, I have created a
status.gowithin their folder. Once the pattern of how to add new status provider is fully established, I will add new documentation in thedocs/componentfolderThe majority of the additions are because of the status templates. Those are similar to those in the
pkg/status/render. To help with the review, filter those outOnly the
agent statusis using the status component. Process agent, apm agent, cluster agent and the GUI status are using the old way. We do this to minimize regressions. Since I'm migrating onestatuscommand at a time, I need to keep the other templates for now. So, the existing templates atpkg/status/render/templates/*can not be deleted yet, as I want the other status command to work in isolation from theagent statuscommand. I will remove those once I migrate allstatuscommands.I've added some basics for all the new status providers. The test makes sure no unwanted panic or errors are returned. It would be ideal to have a test validating the result of each provider. I can only write some of the tests myself; it would take too much time. I would leave that exercise to the code owners of those files.
Describe how to test/QA your changes
PLEASE TAKE INTO CONSIDERATION THAT THE ORDER OF THE STATUS OUTPUT HAS CHANGED. When doing the QA, we need to focus on the same information being displayed and not too much on order.
Build the agent binary
invoke agent.build --build-exclude=systemdTest that the agent status CLI command returns the right information.
agent status -jagent status -pagent statusagent status -vThe
agent statusdisplays information about other agent processes if they are running. For testing those permutations, you can build the other agent and run them + modify the agent configuration.To build and run the process agent, follow this guide
To build the APM agent, run:
inv -e trace-agent.build. To run the apm agent./bin/trace-agent/trace-agent --config dev/dist/datadog.yamlHere is an example of a configuration with logs, apm and process agent enabled:
Reviewer'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, with required eitherqa/doneorqa/no-code-changelabels, are 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.