-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[spanmetrics] Need a feature to add Event attributes to dimensions for spanmetrics #27451
Comments
Pinging code owners for connector/spanmetrics: @albertteoh. See Adding Labels via Comments if you do not have permissions to add labels yourself. |
This makes sense to me. WDYT @albertteoh? |
@djaglowski @albertteoh , few things that we need to figure out is how do we handle span metrics for spans that have more than one exception recorded. however, I would agree that being able to generate additional labels to record errors as labels and generate exemplars for the same would make the metrics being generated very very useful. |
I just want to be clear that the ask is to add opinionated support specifically for the If that's the ask, I'm generally supportive of this new feature. Additional questions:
I'm thinking we could assign each set of |
would love to hear your thoughts around the latter. |
or should we create an |
Yeah, great point, @vjsamuel.
Why wouldn't exception spans have a notion of latency, or perhaps I've misunderstood what you meant? I can imagine it would be quite useful to know; say if a request times out. This requirement was also mentioned in the description: "We should also be able to calculate the latency metric for the list of dimensions - service name, operation, span kind, status code, exception.type and exception.message." One alternative is for the spanmetrics connector to only select the first I'm interested to get your thoughts on this @vjsamuel. Also @aishyandapalli, do you have any thoughts around how you'd like to see metrics for multiple exceptions per span in your use case? |
I am admittedly not very tuned in to the details of this connector, but I'd like to encourage that we look first for solutions which are not hardcoded to specific attributes. To that end, is there strictly a need to count by |
@djaglowski https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/exceptions/ records the specification of exceptions. they must have |
@vjsamuel, are there events which are not exceptions, which could use the same functionality but based on different attributes? |
maybe yes? :) @aishyandapalli and i were razor focused on the errors aspect of things but there could be value. |
How will we represent events in metrics if we support the more general case. Presumably, each attribute key-value would be included as part of metric's dimensions which would break this invariant called out by @vjsamuel:
Perhaps we could set this expectation as a condition of enabling event attributes in spanmetrics; or maybe there is a solution to this problem that I haven't thought of? |
lets maybe create two new metrics |
I like the idea of creating new metrics (sorry, you mentioned it earlier in this thread); would we also need What is the benefit of having a separate |
span events dont have any duration attached to them specifically. so we would only do counters. keeping the metric separate is just making the representation prettier. nothing else. also exceptions have dedicated semantic conventions. |
@albertteoh any thoughts on how we should move forward? |
We can go ahead with adding this feature. I propose starting with the following spec:
The following example demonstrates all of the above spec: connectors:
spanmetrics:
dimensions:
- name: http.method
default: GET
...
events:
enabled: true
dimensions:
- name: attr
default: val
- name: exceptions.type
- name: exceptions.message I've considered including However, I'm open to being convinced otherwise. 😄 Please let me know what you think of that proposed spec. |
Wow, this is such a detailed explanation. Thanks for that @albertteoh @vjsamuel for final approval on the above logic |
@aishyandapalli, @albertteoh owns this :) he is the final approver. i think the proposal works and we should build it out quickly and submit a PR. |
@vjsamuel I meant for our use case :) I will start working on this |
@albertteoh is it possible today for |
I think so, I can't see a reason why we shouldn't add exemplars to calls and events. At the time when it was added to spanmetrics processor, the focus was just on histograms. cc @anneelisabethlelievre Thank you very much @vjsamuel @aishyandapalli! 🙏🏼 |
… of configured event attributes for a span (#27811) **Description:** We have an events section for a span. The details for all the exceptions like exception.type and exception.message are recorded as Events for a span. Right now, we don't have a feature to add event attributes to span metrics. The idea of this PR is to develop a feature which adds a new metric `events_total` with a default set of dimensions like `service_name, span_name, span_kind, status_code`. We can configure to add additional set of dimensions like `exception.type` and `exception.message` which will be fetched from the Events section for a span **Link to tracking Issue:** [27451](#27451) --------- Co-authored-by: Albert <26584478+albertteoh@users.noreply.github.com>
… of configured event attributes for a span (open-telemetry#27811) **Description:** We have an events section for a span. The details for all the exceptions like exception.type and exception.message are recorded as Events for a span. Right now, we don't have a feature to add event attributes to span metrics. The idea of this PR is to develop a feature which adds a new metric `events_total` with a default set of dimensions like `service_name, span_name, span_kind, status_code`. We can configure to add additional set of dimensions like `exception.type` and `exception.message` which will be fetched from the Events section for a span **Link to tracking Issue:** [27451](open-telemetry#27451) --------- Co-authored-by: Albert <26584478+albertteoh@users.noreply.github.com>
… of configured event attributes for a span (open-telemetry#27811) **Description:** We have an events section for a span. The details for all the exceptions like exception.type and exception.message are recorded as Events for a span. Right now, we don't have a feature to add event attributes to span metrics. The idea of this PR is to develop a feature which adds a new metric `events_total` with a default set of dimensions like `service_name, span_name, span_kind, status_code`. We can configure to add additional set of dimensions like `exception.type` and `exception.message` which will be fetched from the Events section for a span **Link to tracking Issue:** [27451](open-telemetry#27451) --------- Co-authored-by: Albert <26584478+albertteoh@users.noreply.github.com>
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping |
Closed by #27811 |
Component(s)
No response
Is your feature request related to a problem? Please describe.
Business Justification - We have exception data recorded as span events. There are exception.type and exception.message recorded as span event attributes. There should be a way to count the total number of errors for each service name, operation, span kind, status code, exception.type and exception.message. So basically there should be a way to add exception.type and exception.message dimensions to RED metrics.
We should also be able to calculate the latency metric for the list of dimensions - service name, operation, span kind, status code, exception.type and exception.message
We would love to see a feature where we can add the exception data from Events to spanmetrics dimensions.
Describe the solution you'd like
We want a way to add exception data from span Events to spanmetric dimensions.
Describe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: