Bugfix/duplicate tags

[andrewstuart]

What does this PR do?

This PR demonstrates one possible solution to an issue we've observed in production, where env tags can be merged with the official deployment.environment resource-level "tag," resulting in confusing behavior if that attribute is not also overridden (or otherwise does not match the env tag).

Motivation

This PR is inspired by a particularly nasty bug we have encountered recently with OTLP metrics being sent to DataDog. When setting env attributes via the attributeprocessor, we were inadvertently creating situations where the Resource level tags did not match the metric series level tags. This caused double entries in the JSON being sent to datadog, and very confusing results in dashboards.

Possible Drawbacks / Trade-offs

The current solution is not configurable and may result in tag values that don't make sense, depending on the tag source that created the original tag.

Describe how to test/QA your changes

go test

You could also set up an otel-contrib-collector pipeline with a DD exporter, and different upsert for env in resourceprocessor and attributeprocessor like so:

  exporters:
    datadog:
      api:
        key: SET_VIA_ENV
  processors:
    resource:
      attributes:
        - key: deployment.environment # technically we override this and env on both processors, now, just to be safe, but IIRC this should replicate
          action: upsert
          value: prod
    attributes:
      actions:
        - key: env
          action: upsert
          value: dev
  service:
    pipelines:
      metrics:
        exporters:
          - datadog
        processors:
          - resource
          - attributes
        receivers:
          - prometheus # or whatever

Reviewer's Checklist

• If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
• Use the major_change label 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.
• A release note has been added or the changelog/no-changelog label has been applied.
• Changed code has automated tests for its functionality.
• Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
• At least one team/.. label has been applied, indicating the team(s) that should QA this change.
• If applicable, docs team has been notified or an issue has been opened on the documentation repo.
• If applicable, the need-change/operator and need-change/helm labels have been applied.
• If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
• If applicable, the config template has been updated.

[bits-bot]

All committers have signed the CLA.

[andrewstuart]

I wanted to get this out there to discuss the solution, and whether or not it is viable or if you'd like some more changes or design to happen before proceeding, but we've been bitten by this and have a workaround currently, but it seems ideal that others would not have to figure out the underlying conversions in play, quite to the same level that we've had to debug.

Please let me know if there's anything you'd like to change here, or discuss better alternatives that I may have missed, not knowing the codebase quite as well as I'm sure the team does.

[dineshg13]

@andrewstuart Thanks for finding & fixing this bug. This might an issue with traces or even logs. I will get back to you if we want to fix in all places or there is some central place we can do it .

[gbbr]

Why are we assuming that any duplicate that is a resource attribute and prefixing it as such? If we want to prefix all resource attributes like this shouldn't it be done at a different level where we are actually aware of this fact? It's a bit of a wild assumption.

[andrewstuart]

I mentioned that explicitly in the tradeoffs above. I just wanted to get this out there, and figured a better solution (at the very least, configurable) would surface as part of the discussions.

[gbbr]

I don't think such a tradeoff is acceptable. I think we should instead simply ensure that the environment is taken from the right tag.

[andrewstuart]

Feel free to fix this however you would like. I put a few solid days of discovery into this issue and fix, so I'm tapped out if you decide to go a different direction, but I won't be offended if you do. Either way, just know that the potential for duplicate tags will still be a fundamental issue with the way the DataDog otlp adapter, and thereby the otel DD exporter, is currently designed.

[diguardiag]

Any update to this? we are affected by this issue and i'm sure other people as well