-
-
Notifications
You must be signed in to change notification settings - Fork 7
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 / Span Correlations #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, makes incremental changes super easy.
text/xxxx-metrics-correlation.md
Outdated
|
||
# Open Questions | ||
|
||
* Metric vs span tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should hopefully be addressed by https://github.com/getsentry/rfcs/blob/main/text/0116-sentry-semantic-conventions.md - let's just go all in on OpenTelemetry conventions
text/xxxx-metrics-correlation.md
Outdated
processor = Processor() | ||
|
||
# This creates a span with op `timing_measurement` | ||
with metrics.timing("processor.process_batch"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the naming scheme of span name/metric name instead of using op
(aligns with otel, removes educating about op types, aligns with other metrics products).
Op already has a meaning to sentry today (db.X
, resource.Y
), and that concept generally has caused us issues, let's not introduce it for any metrics API (we can always make op an optional thing people can set and map it to the same semantics as span op).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly wrote this up in the light of what we have today. I agree that the op/description situation is not great so ideally this can recommend something else instead.
text/xxxx-metrics-correlation.md
Outdated
with metrics.timing("foo"): | ||
pass | ||
|
||
with start_span(op="timing_measurement", metric="foo"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does this work when we are using an otel API under the hood? We'll have to add logic in otel span processors I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is an open question I'm not fully sure about yet. OTel Metrics themselves do not suggest any span based metrics changes. They are doing the correlations OOB with exemplars: https://opentelemetry.io/docs/specs/otel/metrics/data-model/#exemplars
But I do think the case of span + timed metric is very common and ideally we can find a way to funnel that out with otel span APIs somehow.
This RFC proposes a way in which metrics and spans can be correlated. This is a work in progress.
Rendered RFC