feat(trace): implement span start/end metrics #4880
Conversation
|
Hmm - this change may be hard since many contrib tests seem to verify metric count itself, not just the metrics from the instrumentation. Any ideas how to proceed? |
Assuming most of the instrumentations are using UPDATE: I see also some are reading directly from a MetricReader so that should also require local filtering or move to TestBase |
|
Thanks for the hint on TestBase @xrmx - it seems to fix a (very ;) ) few of the tests. I'll take the failures here to find the points we need to update in contrib |
|
PRs for contrib are out |
|
Thanks @xrmx I didn't realize test util would be usable without a release - I have sent open-telemetry/opentelemetry-python-contrib#4150 to pass scope explicitly for these failing tests |
…y-python into spanstartmetrics
|
@xrmx I found I had missed a few spots in contrib behind some flaky CI failures and sent open-telemetry/opentelemetry-python-contrib#4165 This time I verified this PR too by temporarily pointing CI at that PR and it was green so should have caught everything this time |
| self._started_spans.add( | ||
| 1, | ||
| { | ||
| "otel.span.parent.origin": parent_origin(parent_span_context), |
There was a problem hiding this comment.
Please import the attributes names from the semconv package:
from opentelemetry.semconv._incubating.attributes.otel_attributes import OTEL_SPAN_PARENT_ORIGIN, OTEL_SPAN_SAMPLING_RESULT
| self._started_spans = meter.create_counter( | ||
| "otel.sdk.span.started", "{span}", "The number of created spans" | ||
| ) | ||
| self._live_spans = meter.create_up_down_counter( |
There was a problem hiding this comment.
You can import the helper create_otel_sdk_span_live from opentelemetry.semconv._incubating.metrics.otel_metrics
| }, | ||
| ) | ||
|
|
||
| if sampling_decision == Decision.DROP: |
There was a problem hiding this comment.
Surprised by the early return, I see in 1.39.0 semconv that DROP is in the possible values for sampling_result attribute.
| def __init__(self, meter_provider: metrics_api.MeterProvider) -> None: | ||
| meter = meter_provider.get_meter("opentelemetry-sdk") | ||
|
|
||
| self._started_spans = meter.create_counter( |
There was a problem hiding this comment.
You can import create_otel_sdk_span_started from opentelemetry.semconv._incubating.metrics.otel_metrics
| return noop | ||
|
|
||
| live_span_attrs = { | ||
| "otel.span.sampling_result": sampling_decision.name, |
There was a problem hiding this comment.
Can we check that sampling_decision.name is in OtelSpanSamplingResultValues before using it ?
| trace_state=sampling_result.trace_state, | ||
| ) | ||
|
|
||
| record_end_metrics = self._tracer_metrics.start_span( |
There was a problem hiding this comment.
I'm not sure we can assume here that the span is not a NonRecordingSpan (for otel.sdk.span.live) so I think we should pass that information or the span itself.
2 participants
Description
I am helping to implement SDK internal metrics
https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/
I have started with the most basic of them, started and ended spans. This sets up some test infrastructure as well and will be good to confirm before implementing more.
For reference, similar change in
Java: open-telemetry/opentelemetry-java#7895
JS: open-telemetry/opentelemetry-js#6213
/cc @xrmx to help with review
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Contrib Repo Change?
Checklist: