Skip to content

[CONTINT-3864] Send both origin the container-id and the ENTITY_ID#300

Merged
wdhif merged 4 commits intomasterfrom
ali/fix-origin-detection
May 7, 2024
Merged

[CONTINT-3864] Send both origin the container-id and the ENTITY_ID#300
wdhif merged 4 commits intomasterfrom
ali/fix-origin-detection

Conversation

@AliDatadog
Copy link
Copy Markdown
Contributor

We would like customers to be able to retrieve container tags even when DD_ENTITY_ID is set to false. The current behavior does not send the container-id if the entity id is set.

Note that the admission controller will set the container-id on Kubernetes.

For those who do not want container-tags, several alternatives are possible:

  • Send the tag dd.internal.card=none
  • Inject DD_ORIGIN_DETECTION_ENABLED=false to the application pod

See this doc for reference

@AliDatadog AliDatadog requested a review from a team January 8, 2024 15:59
@AliDatadog AliDatadog changed the title Send both origin the container-id and the ENTITY_ID [CONTINT-3559] Send both origin the container-id and the ENTITY_ID Jan 8, 2024
func isOriginDetectionEnabled(o *Options) bool {
if !o.originDetection || o.containerID != "" {
// originDetection is explicitly disabled
// or DD_ENTITY_ID was found
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I would have left the rest of the comment but that's not a blocker.

wdhif
wdhif previously approved these changes Feb 14, 2024
@wdhif wdhif changed the title [CONTINT-3559] Send both origin the container-id and the ENTITY_ID [CONTINT-3864] Send both origin the container-id and the ENTITY_ID Mar 4, 2024
@wdhif
Copy link
Copy Markdown
Member

wdhif commented Apr 15, 2024

Links to DataDog/datadogpy#828

gh123man
gh123man previously approved these changes May 6, 2024
Copy link
Copy Markdown
Member

@gh123man gh123man left a comment

Choose a reason for hiding this comment

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

LGTM

@wdhif wdhif dismissed stale reviews from gh123man and themself via 8c18762 May 7, 2024 12:07
@wdhif wdhif force-pushed the ali/fix-origin-detection branch from 642c790 to 8c18762 Compare May 7, 2024 12:07
@wdhif
Copy link
Copy Markdown
Member

wdhif commented May 7, 2024

/merge

@dd-devflow
Copy link
Copy Markdown

dd-devflow bot commented May 7, 2024

🚂 MergeQueue

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@dd-devflow
Copy link
Copy Markdown

dd-devflow bot commented May 7, 2024

🚂 MergeQueue

Pull request added to the queue.

This build is going to start soon! (estimated merge in less than 0s)

Use /merge -c to cancel this operation!

@dd-devflow
Copy link
Copy Markdown

dd-devflow bot commented May 7, 2024

🚨 MergeQueue

Gitlab pipeline didn't start on its own and we were unable to create it... Please retry.

If you need support, contact us on Slack #devflow with those details!

@wdhif
Copy link
Copy Markdown
Member

wdhif commented May 7, 2024

/merge

@dd-devflow
Copy link
Copy Markdown

dd-devflow bot commented May 7, 2024

🚂 MergeQueue

Pull request added to the queue.

This build is going to start soon! (estimated merge in less than 0s)

Use /merge -c to cancel this operation!

@dd-devflow
Copy link
Copy Markdown

dd-devflow bot commented May 7, 2024

🚨 MergeQueue

Gitlab pipeline didn't start on its own and we were unable to create it... Please retry.

If you need support, contact us on Slack #devflow with those details!

@wdhif wdhif merged commit 1139efe into master May 7, 2024
@wdhif wdhif deleted the ali/fix-origin-detection branch May 7, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants