AO-16209 otel tracer #174
AO-16209 otel tracer #174
Conversation
Codecov Report
@@ Coverage Diff @@
## master #174 +/- ##
==========================================
- Coverage 81.98% 81.65% -0.34%
==========================================
Files 40 40
Lines 4292 4322 +30
==========================================
+ Hits 3519 3529 +10
- Misses 638 654 +16
- Partials 135 139 +4
Continue to review full report at Codecov.
|
| // SpanOptions defines the options of creating a span | ||
| type SpanOptions struct { | ||
| // WithBackTrace indicates whether to include the backtrace in BeginSpan | ||
| // Keep in mind that the cost of this option may be high as it calls | ||
| // `debug.Stack()` internally to gather the stack trace. Please consider | ||
| // the impact on performance/memory footprint carefully. | ||
| WithBackTrace bool | ||
|
|
||
| StartTime time.Time | ||
| EndTime time.Time |
There was a problem hiding this comment.
It's not used in the OT tracer, the Timestamp in OT's SpanConfig is used as timestamp override instead.
| // This is based from the OpenTelemetry sdk code. | ||
| func GetSpanContextAndLinks(ctx context.Context, | ||
| ignoreContext bool) (trace.Span, []trace.Link) { | ||
| local := trace.SpanFromContext(ctx) |
There was a problem hiding this comment.
Wondering if this would work if the root span is an ao.Span? For example the root span is created by our instrumentation (hence an ao.Span), then a dev uses our tracer to create a nested span, would the context.Context returns ot span which is the wrapped ao.Span root span?
There was a problem hiding this comment.
No, as the span needs to be attached to the context with the key currentSpanKey (defined in OT). It also checks if the span implements OT's span before returned to the caller. The users should always use one set of APIs to create spans (either OT or AO, but not mixed)
| local := trace.SpanFromContext(ctx) | ||
|
|
||
| if ignoreContext { | ||
| links := addLinkIfValid(nil, local.SpanContext(), "current") |
There was a problem hiding this comment.
Is this trying to keep some info of current context (if exist), even if the config tells creating a new root? :)
There was a problem hiding this comment.
Yes, this produces a loose link to the current span context with an attribute indicating it's "ignored-on-demand".
|
Will we provide text map propagator for our tracer? https://github.com/open-telemetry/opentelemetry-go/blob/d616df61f5d163589228c5ff3be4aa5415f5a884/oteltest/text_map_propagator.go#L164 If a dev creates their own instrumentation , and then uses OT context injection, it will be nice ao's x-trace header is injected too such that it can interacts with other ao agent? :) |
Codecov Report
@@ Coverage Diff @@
## master #174 +/- ##
==========================================
- Coverage 81.98% 81.62% -0.37%
==========================================
Files 40 40
Lines 4292 4322 +30
==========================================
+ Hits 3519 3528 +9
- Misses 638 655 +17
- Partials 135 139 +4
Continue to review full report at Codecov.
|
Good point. An AO context text map propagator is added. |
An OT tracer implementation which works as an think wrapper to the existing AO Go agent.
See https://swicloud.atlassian.net/browse/AO-16209?focusedCommentId=236389 for more details.