feat(origin detection): send both container ID and Entity ID

wdhif commented 2 months ago

What does this PR do?

We would like customers to be able to retrieve container tags even when DD_ENTITY_ID is set. Currently the library does not send the container ID if the Entity ID is set.

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.

Description of the Change

Remove checks for DD_ENTITY_ID when resolving container ID.

Possible Drawbacks

Verification Process

To make sure that this is working as intended, create a new pod with the following manifest:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: dummy-dogstatsd-udp-app
spec:
  replicas: 1
  selector:
    matchLabels:
      app: dummy-dogstatsd-udp-app
  template:
    metadata:
      labels:
        app: dummy-dogstatsd-udp-app
    spec:
      containers:
      - name: dummy-dogstatsd-udp-app
        image: wdhifdatadog/dummy_dogstatsd_udp
        imagePullPolicy: Always
        env:
        - name: DD_AGENT_HOST
          valueFrom:
            fieldRef:
              fieldPath: status.hostIP
        - name: DD_ENTITY_ID
          valueFrom:
            fieldRef:
              fieldPath: metadata.uid

You should still be able to get the following metrics with container tags such as container_id:

dummy_dsd_udp_python.increment
dummy_dsd_udp_python.decrement

We can see that, with an agent cardinality set as high:

With the old behaviour, we don't get container tags.
With the old behaviour and dd.internal.card:low, we don't get container tags (only pod tags).
With the new behaviour, we get container tags.
With the new behaviour and dd.internal.card:low, we don't get container tags (only pod tags).

Review checklist (to be filled by reviewers)

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

wdhif commented 2 months ago

Links to https://github.com/DataDog/datadog-go/pull/300

wdhif commented 2 months ago

I've added a unit test showing both Entity ID and container ID in the same metric.

wdhif commented 2 months ago

/merge

dd-devflow[bot] commented 2 months ago

:steam_locomotive: 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[bot] commented 2 months ago

:rotating_light: 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 commented 2 months ago

/merge

dd-devflow[bot] commented 2 months ago

:steam_locomotive: 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[bot] commented 2 months ago

:rotating_light: 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!

DataDog / datadogpy