feat: sdk/trace: span processed metric for simple span processor #7162
Conversation
…le_span_processor.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7162 +/- ##
=====================================
Coverage 82.8% 82.8%
=====================================
Files 265 265
Lines 24896 24944 +48
=====================================
+ Hits 20631 20677 +46
- Misses 3885 3887 +2
Partials 380 380
🚀 New features to boost your workflow:
|
…is already being used in batch span processor
mahendrabishnoi2
requested review from
MrAlias,
XSAM,
dashpole,
dmathieu,
flc1125 and
pellared
as code owners
|
For some reason codecov comment is not updated but new code is covered by tests. (https://app.codecov.io/github/open-telemetry/opentelemetry-go/pull/7162) |
sdk/trace/simple_span_processor.go
Outdated
| } | ||
|
|
||
| // configureSelfObservability configures metrics for the simple span processor. | ||
| func (ssp *simpleSpanProcessor) configureSelfObservability() { |
There was a problem hiding this comment.
Please flatten this into the span processor creation function or refactor it to not produce side-effects.
Related:
- separate changelog entry - flatten self observability initialization - reset simpleProcessorIDCounter so tests can be run in parallel and order doesn't matter
- use sync.Pool to amortize allocation for metric attrs
- use sync.Pool to amortize allocation for metric attrs - pass context with span to ensure metric is recorded with correct span context
|
@MrAlias Addressed the comments. Please do another round of review when you can. Thanks. |
| if ssp.selfObservabilityEnabled { | ||
| // Add the span to the context to ensure the metric is recorded | ||
| // with the correct span context. | ||
| ctx := trace.ContextWithSpanContext(context.Background(), s.SpanContext()) |
There was a problem hiding this comment.
Is this the correct way to add span to context given we have ReadOnlySpan in this method instead of Span?
|
@mahendrabishnoi2 PTAL #7272 |
|
Hi, since this process involves specification adjustments and historical review records (which may contain invalid review suggestions), it’s impossible to tell which items need attention amid the large volume of information. Could we create a new PR based on the current branch before preparing for the review, so that subsequent reviews can proceed more smoothly? Thanks~ |
|
@mahendrabishnoi2 please take a look at #7004. I have updated the issue with a check list of items from our project Observability guidelines that need to be completed in this PR. As @flc1125 pointed out, you may find it easier to open a new PR. But feel free to update this PR as well. If you plan to update this PR, please mark it as a draft while you are working to address the items listed in #7004 and the Observability guidelines. |
|
Closed in favor of #7374 |
4 participants
Fixes: #7004
This PR adds support for experimental
otel.sdk.processor.span.processedmetric in simple span processor.Definition of metric at: https://github.com/open-telemetry/semantic-conventions/blob/v1.36.0/docs/otel/sdk-metrics.md
Experimental metrics are behind a feature flag:
OTEL_GO_X_SELF_OBSERVABILITY