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

feat(error-tracking): add settings for backend standalone #1625

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

GianlucaBortoli
Copy link
Member

@GianlucaBortoli GianlucaBortoli commented Dec 9, 2024

What this PR does / why we need it:

Exposes new settings available for the agent:

The equivalent for the operator will be done in a separate PR.

Checklist

  • Chart Version bumped
  • Documentation has been updated with helm-docs (run: .github/helm-docs.sh)
  • CHANGELOG.md has been updated
  • Variables are documented in the README.md
  • For Datadog Operator chart or value changes update the test baselines (run: make update-test-baselines)

@github-actions github-actions bot added the chart/datadog This issue or pull request is related to the datadog chart label Dec 9, 2024
@GianlucaBortoli GianlucaBortoli force-pushed the gianluca/ERRORT-4859-add-et-standalone-env-vars branch from 25b074e to 876930c Compare December 9, 2024 11:03
@GianlucaBortoli GianlucaBortoli marked this pull request as ready for review December 9, 2024 12:57
@GianlucaBortoli GianlucaBortoli requested a review from a team as a code owner December 9, 2024 12:57
@GianlucaBortoli GianlucaBortoli requested a review from a team December 9, 2024 12:57
@@ -1519,6 +1524,11 @@ agents:
## get guaranteed delivery of the metrics in Datadog-per-namespace setup!
enabled: true

## Settings for the core agent
coreAgent:
Copy link
Collaborator

@clamoriniere clamoriniere Dec 10, 2024

Choose a reason for hiding this comment

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

It doesn’t seem ideal to base the activation or deactivation of a feature on a container name (coreAgent). While this approach has been used in the past, we are now aiming to define settings names based on the end-user feature rather than internal or technical details.

Additionally, the proposed setting name feels too generic. It would be preferable to use a name that is both clear and self-explanatory. For example, something along the lines of agents.payloadForwarder.enabled or, considering the functionality in the datadog-agent codebase, agents.metricsForwarder.enabled might be more appropriate.

WDYT?

Copy link
Collaborator

@clamoriniere clamoriniere left a comment

Choose a reason for hiding this comment

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

I don’t feel knowledgeable enough yet about this new APM feature to recommend the best approach for enabling the "error tracking standalone" functionality.

However, if the APM team intends to reuse the datadog Helm chart while enabling only this APM feature, simply disabling the metrics forwarder doesn’t seem optimal. This is because several other functionalities currently run by default in the core-agent or process-agent container and would continue to execute within the core-agent process, consuming CPU and memory resources unnecessarily. We also run by default the cluster-agent component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart/datadog This issue or pull request is related to the datadog chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants