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

metrics tag missing from datadog exporter v3 #791

Closed
rex-remind101 opened this issue May 4, 2022 · 13 comments
Closed

metrics tag missing from datadog exporter v3 #791

rex-remind101 opened this issue May 4, 2022 · 13 comments
Labels
A-trace Area: issues related to tracing enhancement New feature or request

Comments

@rex-remind101
Copy link
Contributor

Looking at dd-trace we see that "sampling.priority" is added as a tag implicitly https://github.com/DataDog/dd-trace-js/blob/3419a4c09584a3109952b7c7da7d84924c36b866/packages/dd-trace/src/format.js#L98

Given that "sampling.priority" is represented by a number it should be exported as part of the metrics part of the span data as shown here https://github.com/DataDog/dd-trace-js/blob/3419a4c09584a3109952b7c7da7d84924c36b866/packages/dd-trace/src/format.js#L146

However, looking at the datadog exporter v3, there does not appear to be any mention of forwarding "sampling.priority" nor mention of metrics https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-datadog/src/exporter/model/v03.rs

Is this inserted elsewhere? If not what would be required to add it?

@rex-remind101
Copy link
Contributor Author

pr #792

@TommyCpp TommyCpp added enhancement New feature or request A-trace Area: issues related to tracing labels May 11, 2022
@TommyCpp
Copy link
Contributor

Thanks for bringing it up. I think maybe we can just drop the span when exporting instead of encode it and send out.

Have you tested it against a datadog endpoint? I lost my access to datadog a while ago so I can't test it myself.

@rex-remind101
Copy link
Contributor Author

rex-remind101 commented May 11, 2022

We need to export even in cases where spans are not sampled because the agent needs to be aware of metrics to properly calculate request/error statistics on datadog's end. The agent will then perform sampling on its end anyway afaiu.

@rex-remind101
Copy link
Contributor Author

Actually not sure if i'm understanding you, what do you mean by "drop the span when exporting"?

@TommyCpp
Copy link
Contributor

For unsampled spans, we don't have to pass it to the span exporter as per the spec.

Sampled flag in TraceFlags on SpanContext. This flag is propagated via the SpanContext to child Spans. For more details see the W3C Trace Context specification. This flag indicates that the Span has been sampled and will be exported. Span Exporters MUST receive those spans which have Sampled flag set to true and they SHOULD NOT receive the ones that do not.

@rex-remind101
Copy link
Contributor Author

Yet judging from everything I've looked at, datadog's agent requires receiving metrics for every span or it will fail to correctly calculate statistics. So it seems like datadog does not precisely follow the spec. Also I believe datadog allows the user to setup their own ingestion controls through their website for how we wish to sample.

I think this means we'll need to force every span to be sent to datadog and make sure that metrics appropriately sets the sampling priority as I did in that pr for datadog's metrics to work correctly.

Does this make sense?

@TommyCpp
Copy link
Contributor

Yeah that make sense. Let me take a deep look on the datadog document but generally I think your argument is valid

@rex-remind101
Copy link
Contributor Author

rex-remind101 commented May 26, 2022

Have you had time to take a deep look yet? We are hoping this will resolve the described issue with our telemetry and would like to get it in.

@TommyCpp
Copy link
Contributor

Sorry I have been traveling this week so cannot spend much time. I will try to get a review for the PR this weekend

@rex-remind101
Copy link
Contributor Author

looks like we can close this out now, how often do new versions come out so we may upgrade?

@rex-remind101
Copy link
Contributor Author

@TommyCpp ?

@TommyCpp
Copy link
Contributor

looks like we can close this out now, how often do new versions come out so we may upgrade?

We are planning to release 0.18 on #779. I'd say probably some time next month?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trace Area: issues related to tracing enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants