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

[statsd] Add origin detection with container ID field #720

Merged
merged 4 commits into from
Mar 8, 2022

Conversation

ahmed-mez
Copy link
Contributor

@ahmed-mez ahmed-mez commented Mar 7, 2022

What does this PR do?

Adds c:<id> field holding the container ID so the agent can attach container tags.

Description of the Change

  • If DD_ENTITY_ID is set, we ignore the container field
  • If DD_ENTITY_ID is not set and DD_ORIGIN_DETECTION_ENABLED is not explicitly set to false and the origin_detection_enabled param is not explicitly disabled, we try to get the app container ID by parsing /proc/self/cgroup. In case of success, the container field in will be set.

Alternate Designs

Possible Drawbacks

Verification Process

Wrote a small app importing the lib and sending metrics to agent 7.35 with origin detection client enabled - the metrics were tagged with container tags.

Additional Notes

Release Notes

Support adding the container ID field for origin detection (dogstatsd 1.2)

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@ahmed-mez ahmed-mez requested review from a team as code owners March 7, 2022 09:33
@ahmed-mez ahmed-mez added the changelog/Added Added features results into a minor version bump label Mar 7, 2022
@ahmed-mez ahmed-mez force-pushed the ahmed/container-id-field branch 2 times, most recently from f8ee080 to d9b9843 Compare March 7, 2022 09:44
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
Copy link
Contributor

@sgnn7 sgnn7 left a comment

Choose a reason for hiding this comment

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

@ahmed-mez Left some comments but overall looks good

datadog/dogstatsd/base.py Show resolved Hide resolved
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
datadog/dogstatsd/base.py Show resolved Hide resolved
datadog/dogstatsd/container.py Outdated Show resolved Hide resolved
datadog/dogstatsd/container.py Outdated Show resolved Hide resolved
datadog/dogstatsd/container.py Outdated Show resolved Hide resolved
datadog/dogstatsd/container.py Outdated Show resolved Hide resolved
@ahmed-mez ahmed-mez requested a review from sgnn7 March 8, 2022 10:16
@sgnn7
Copy link
Contributor

sgnn7 commented Mar 8, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@sgnn7 sgnn7 merged commit 63d0c01 into master Mar 8, 2022
@sgnn7 sgnn7 deleted the ahmed/container-id-field branch March 8, 2022 17:48
@sgnn7 sgnn7 changed the title feat: origin detection with container ID field [statsd] Add origin detection with container ID field Mar 8, 2022
@sgnn7 sgnn7 added this to the 0.45.0 milestone Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Added Added features results into a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants