Add SpanFromWorkflowContext function for OTel #2118
Conversation
|
|
| return trace.ContextWithSpan(ctx, span.(*tracerSpan).Span) | ||
| } | ||
|
|
||
| func SpanFromWorkflowContext(ctx workflow.Context) (trace.Span, bool) { |
There was a problem hiding this comment.
Please add a godoc to this function explaining the functionality
jessefoster
force-pushed
the
otel/span-context
branch
from
3f9e3d5 to
06ebaba
Compare
|
|
||
| // SpanFromWorkflowContext extracts an OpenTelemetry span from the given | ||
| // workflow context. If no span is found, a no-op span is returned. | ||
| func SpanFromWorkflowContext(ctx workflow.Context) (trace.Span, bool) { |
There was a problem hiding this comment.
Why return a bool that is always true?
There was a problem hiding this comment.
Yeah this look like a bug
|
|
||
| // SpanFromWorkflowContext extracts an OpenTelemetry span from the given | ||
| // workflow context. If no span is found, a no-op span is returned. | ||
| func SpanFromWorkflowContext(ctx workflow.Context) (trace.Span, bool) { |
There was a problem hiding this comment.
May need to be clarified in the godoc that this is unsafe/non-deterministic because it can technically return different values when replaying vs not
There was a problem hiding this comment.
So you'd like a comment warning people they should only use the Span data for observability needs? That's generally the assumption with observability tools, but I can make it explicit.
|
|
||
| // SpanFromWorkflowContext extracts an OpenTelemetry span from the given | ||
| // workflow context. If no span is found, a no-op span is returned. | ||
| func SpanFromWorkflowContext(ctx workflow.Context) (trace.Span, bool) { |
There was a problem hiding this comment.
I wonder if SpanContextFromWorkflowContext is needed/better
There was a problem hiding this comment.
We haven't had any need to use SpanContext, but it is another option. The Datadog interceptor only provides Span as well so I followed it's example since it's already in the codebase.
| } | ||
|
|
||
| // Fallback to OpenTelemetry span extraction behavior | ||
| return trace.SpanFromContext(nil), true |
There was a problem hiding this comment.
This should return false
There was a problem hiding this comment.
Yeah sorry about that, copy/paste mistake as I had to recreate file and run tests on another box. Not allowed to push directly from corporate laptop.
jessefoster
force-pushed
the
otel/span-context
branch
from
06ebaba to
8083b21
Compare
4 participants
What was changed
Added a function which can extract Open Telemetry Span from Workflow Context.
Why?
I saw Datadog had this capability already, see #1711, and my project needs access to Open Telemetry spans.
Checklist
How was this tested:
I've confirmed the feature works within my codebase and added unit tests as well. I've run all automated testing as prescribed here
Any doc updates needed?
None that I'm aware of