-
Notifications
You must be signed in to change notification settings - Fork 831
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
OTEL workflow span attributes #6699
base: main
Are you sure you want to change the base?
Conversation
8a6fd36
to
fa77802
Compare
fx.Provide( | ||
func(r *otelresource.Resource, sps []otelsdktrace.SpanProcessor) []otelsdktrace.TracerProviderOption { | ||
opts := make([]otelsdktrace.TracerProviderOption, 0, len(sps)+1) | ||
opts = append(opts, otelsdktrace.WithResource(r)) | ||
for _, sp := range sps { | ||
opts = append(opts, otelsdktrace.WithSpanProcessor(sp)) | ||
} | ||
return opts | ||
}, | ||
), |
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.
Inlined into the next provider to be able to check if there are span processors at all.
fx.Provide(func(lc fx.Lifecycle, opts []otelsdktrace.TracerProviderOption) trace.TracerProvider { | ||
fx.Provide(func(lc fx.Lifecycle, r *otelresource.Resource, sps []otelsdktrace.SpanProcessor) trace.TracerProvider { | ||
if len(sps) == 0 { | ||
return noop.NewTracerProvider() |
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.
No need to trace anything if there are no processors (since there are no exporters)
// annotate span with workflow tags (same ones the Temporal SDKs use) | ||
for _, tag := range c.tags.Extract(s.Payload, methodName) { | ||
var k string | ||
switch tag.Key() { | ||
case "wf-id": | ||
k = "temporalWorkflowID" | ||
case "wf-run-id": | ||
k = "temporalRunID" | ||
default: | ||
continue | ||
} | ||
span.SetAttributes(attribute.Key(k).String(tag.Value().(string))) | ||
} |
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.
This part is new net behavior. It applies the two attributes, when present.
if !isEnabled(tp) { | ||
return nil | ||
} |
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.
This is new behavior: when tracing is disabled, it returns nil.
if !isEnabled(tp) { | ||
return nil | ||
} |
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.
This is new behavior: when tracing is disabled, it returns nil.
fa77802
to
e527f21
Compare
tp trace.TracerProvider, | ||
tmp propagation.TextMapPropagator, | ||
) ServerTraceInterceptor { | ||
//nolint:staticcheck | ||
otelInterceptor := otelgrpc.UnaryServerInterceptor( |
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.
otelgrpc.UnaryServerInterceptor
is deprecated.
tmp propagation.TextMapPropagator, | ||
) ClientTraceInterceptor { | ||
return ClientTraceInterceptor( | ||
otelgrpc.UnaryClientInterceptor( |
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.
otelgrpc.UnaryClientInterceptor
is deprecated.
} | ||
|
||
// annotate span with workflow tags (same ones the Temporal SDKs use) | ||
for _, tag := range c.tags.Extract(s.Payload, methodName) { |
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.
This is re-using the existing generated code that can extract Workflow tags from gRPC requests. It seems to do exactly what we need, so I'm not inclined to clone/fork that code.
e527f21
to
f2ef687
Compare
f2ef687
to
afcc861
Compare
What changed?
(1) Adds the following OpenTelemetry (OTEL) span attributes:
temporalWorkflowID
temporalRunID
(2) Replaces the deprecated approach of OTEL interceptors with
stats.Handler
s(3) If there are no exporters, disables the
stats.Handlers
completely.Why?
The workflow id allows to correlate traces (which spans are part of) that belong to the same workflow id.
How did you test it?
(1) Added unit tests
(2) Manually
Using
it outputs
Potential risks
Breaking change?
Documentation
Is hotfix candidate?