-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[statsdreceiver] support simple tags #29012
[statsdreceiver] support simple tags #29012
Conversation
re: attributes, the otel spec says:
explicit null is however not allowed. Go is a bit awkward here b/c the nil value is distinct from what |
At the root of your checkout, run |
ed67ff4
to
33ce62a
Compare
i have now put this behind a config flag as well just to be safe. the changelog includes a note about this. i'm open to it being default; we use dogstatsd so it makes sense to be on, but i'm not sure what other popular implementations of statsd are in use by others. |
6468e97
to
b27d979
Compare
dogstatsd supports two types of tags on metrics: simple and dimensional tags[^1]. the former is just a key, the latter is a key and a value. with the assumption that many users of the statsdreceiver are enabling ingest of dogstatsd metrics, this makes the statsd parser more optimistic, so it can handle tags w/o a value. when this happens, we set an attribute that has a zero value. so far as i know, this is allowed in the opentelemetry spec. the decision of how to handle attributes w/ zero values is best left to configuration w/in the pipeline itself, as different users may have different opinions or approaches that work best with their systems. [^1]: https://www.datadoghq.com/blog/the-power-of-tagged-metrics/#whats-a-metric-tag
b27d979
to
e24f785
Compare
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Are you interested in making the new behavior on by default? I understand being conservative, but the new functionality is not likely to break any existing functionality, right? Most users who come to this component are likely coming from a dogstatsd or similar agent. |
thanks!
I'm for doing this in another PR now that this is merged. I agree that leaving it off by default is perhaps too conservative, but I opted for the minimal-disruption path in order to add confidence to the patch. |
dogstatsd supports two types of tags on metrics: simple and dimensional tags[^1]. the former is just a key, the latter is a key and a value. with the assumption that many users of the statsdreceiver are enabling ingest of dogstatsd metrics, this makes the statsd parser more optimistic, so it can handle tags w/o a value. this functionality is gated behind a new `enable_simple_tags` flag. when this happens, we set an attribute that has a zero value. so far as i know, this is allowed in the opentelemetry spec. the decision of how to handle attributes w/ zero values is best left to configuration w/in the pipeline itself, as different users may have different opinions or approaches that work best with their systems. [^1]: https://www.datadoghq.com/blog/the-power-of-tagged-metrics/#whats-a-metric-tag **Testing:** added coverage to unit tests to enable parsing simple tags. --------- Co-authored-by: Alex Boten <[email protected]>
@diurnalist thanks for this contribution! I'm seeing errors like these from our
Unfortunately I don't have access to the full line, but based on your experience does this error message indicate use of a "simple tag"? Due to the trailing |
Description:
dogstatsd supports two types of tags on metrics: simple and dimensional tags1. the former is just a key, the latter is a key and a value.
with the assumption that many users of the statsdreceiver are enabling ingest of dogstatsd metrics, this makes the statsd parser more optimistic, so it can handle tags w/o a value. this functionality is gated behind a new
enable_simple_tags
flag.when this happens, we set an attribute that has a zero value. so far as i know, this is allowed in the opentelemetry spec. the decision of how to handle attributes w/ zero values is best left to configuration w/in the pipeline itself, as different users may have different opinions or approaches that work best with their systems.
Testing:
added coverage to unit tests to enable parsing simple tags.
Footnotes
https://www.datadoghq.com/blog/the-power-of-tagged-metrics/#whats-a-metric-tag ↩