Skip to content

feat(otel): add otel tracing for standalone activities. #2302

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.

Sign up for GitHub

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

Open
cconstable wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(otel): add otel tracing for standalone activities. #2302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions contrib/opentelemetry/tracing_interceptor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import (
"go.opentelemetry.io/otel/sdk/trace/tracetest"
"go.opentelemetry.io/otel/trace"

"go.temporal.io/sdk/client"
"go.temporal.io/sdk/contrib/opentelemetry"
"go.temporal.io/sdk/interceptor"
"go.temporal.io/sdk/internal"
"go.temporal.io/sdk/internal/interceptortest"
"go.temporal.io/sdk/temporal"
"go.temporal.io/sdk/testsuite"
Expand All @@ -35,6 +37,34 @@ func TestSpanPropagation(t *testing.T) {
interceptortest.AssertSpanPropagation(t, testTracer)
}

func TestStandaloneActivitySpanCreation(t *testing.T) {
Copy link
Copy Markdown
Contributor Author

@cconstable cconstable Apr 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally named this TestStandaloneActivitySpanPropagation but I realized there is another test, TestSpanPropagation, that actually tests the propagation. We just care about the span creation here: an SAA should create a span with the correct name and temporalActivityID.

rec := tracetest.NewSpanRecorder()
tracer, err := opentelemetry.NewTracer(opentelemetry.TracerOptions{
Tracer: sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(rec)).Tracer(""),
})
require.NoError(t, err)

ctx := internal.NewHeaderContext(context.Background())
Copy link
Copy Markdown
Contributor Author

@cconstable cconstable Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interceptor below writes to a header map expected to be on the context. If the map is missing it panics. To work around this, I went through a number of options:

  1. Add internal.NewHeaderContext wrapper around contextWithNewHeader. Additive change, reachable from the contrib package. This is what I did. I think it's simple.
  2. Rename contextWithNewHeader to ContextWithNewHeader. Requires touching every internal call site and didn't feel right.
  3. Promote it to "public" api as interceptor.HeaderContext(ctx). Don't like exposing test scaffolding like this.
  4. Make writeSpanToHeader a noop for null headers. Don't like silently failing. Seems bad.
  5. Spin up a real client. Too heavy, I just want to unit test span creation.

outbound := interceptor.NewTracingInterceptor(tracer).InterceptClient(&testNoopClientOutbound{})
_, _ = outbound.ExecuteActivity(ctx, &interceptor.ClientExecuteActivityInput{
ActivityType: "test-saa",
Options: &client.StartActivityOptions{ID: "test-saa-123"},
})

spans := rec.Ended()
require.Len(t, spans, 1)
assert.Equal(t, "StartActivity:test-saa", spans[0].Name())

var foundActivityID bool
for _, attr := range spans[0].Attributes() {
if string(attr.Key) == "temporalActivityID" {
foundActivityID = true
assert.Equal(t, "test-saa-123", attr.Value.AsString())
}
}
require.True(t, foundActivityID, "expected activity ID span attribute")
}

type testTracer struct {
interceptor.Tracer
rec *tracetest.SpanRecorder
Expand Down Expand Up @@ -235,3 +265,11 @@ func TestSpanFromWorkflowContextNoOpSpan(t *testing.T) {
require.True(t, env.IsWorkflowCompleted())
require.NoError(t, env.GetWorkflowError())
}

type testNoopClientOutbound struct {
interceptor.ClientOutboundInterceptorBase
}

func (n *testNoopClientOutbound) ExecuteActivity(_ context.Context, _ *interceptor.ClientExecuteActivityInput) (client.ActivityHandle, error) {
return nil, nil
}
1 change: 1 addition & 0 deletions interceptor/interceptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ type NexusCancelOperationInput = internal.NexusCancelOperationInput
//
// This returns a non-nil map only for contexts inside
// ActivityInboundInterceptor.ExecuteActivity,
// ClientOutboundInterceptor.ExecuteActivity,
// ClientOutboundInterceptor.ExecuteWorkflow, and
// ClientOutboundInterceptor.SignalWithStartWorkflow.
func Header(ctx context.Context) map[string]*commonpb.Payload {
Expand Down
22 changes: 22 additions & 0 deletions interceptor/tracing_interceptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,28 @@ func (t *tracingClientOutboundInterceptor) UpdateWithStartWorkflow(
return val, err
}

func (t *tracingClientOutboundInterceptor) ExecuteActivity(
ctx context.Context,
in *ClientExecuteActivityInput,
) (client.ActivityHandle, error) {
span, ctx, err := t.root.startSpanFromContext(ctx, &TracerStartSpanOptions{
Operation: "StartActivity",
Name: in.ActivityType,
Tags: map[string]string{activityIDTagKey: in.Options.ID},
ToHeader: true,
Time: time.Now(),
}, t.root.headerReader(ctx), t.root.headerWriter(ctx))
if err != nil {
return nil, err
}
var finishOpts TracerFinishSpanOptions
defer span.Finish(&finishOpts)

handle, err := t.Next.ExecuteActivity(ctx, in)
finishOpts.Error = err
return handle, err
}

type tracingActivityOutboundInterceptor struct {
ActivityOutboundInterceptorBase
root *tracingInterceptor
Expand Down
37 changes: 19 additions & 18 deletions internal/cmd/tools/doclink/doclink.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,25 @@ func processInternal(cfg config, file *os.File, pairs map[string]map[string]stri
trimmedNextLine = nextLine
}

// Check for new doc links to add
// Track whether nextLine is inside a function or interface block.
// We update this first so the first line inside a block is not treated
// as a top-level definition.
if strings.HasPrefix(trimmedLine, "func ") {
funcSpaces = indentSize
inFunc = true
} else if inFunc && trimmedLine == "}" && funcSpaces == indentSize {
funcSpaces = -1
inFunc = false
}
if strings.HasSuffix(trimmedLine, "interface {") {
interfaceSpaces = indentSize
inInterface = true
} else if inInterface && trimmedLine == "}" && interfaceSpaces == indentSize {
interfaceSpaces = -1
inInterface = false
}
Copy link
Copy Markdown
Contributor Author

@cconstable cconstable Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial commits failed the doclink check because doclink was erroneously interpreting the first line of a top-level function as a top-level line.

Without the fix:

go run ./internal/cmd/tools/doclink/doclink.go
Missing doc in internal/client_testsuite.go for internal:ClientOutboundInterceptor to interceptor:ClientOutboundInterceptor
2026/04/24 17:28:05 Changes needed, see previous stdout for which objects. Re-run command with -fix to auto-generate new docs.

func (s *ClientTestSuite) NewTestClientEnvironment(interceptors ...ClientInterceptor) *TestClientEnvironment {
	var chain ClientOutboundInterceptor = &noopClientOutboundInterceptor{}
              ^^^^^^^^^^^^^^^^^^^^^^^^^

and running go run ./internal/cmd/tools/doclink/doclink.go -fix would add this:

func (s *ClientTestSuite) NewTestClientEnvironment(interceptors ...ClientInterceptor) *TestClientEnvironment {
	//
	// Exposed as: [go.temporal.io/sdk/interceptor.ClientOutboundInterceptor]
	var chain ClientOutboundInterceptor = &noopClientOutboundInterceptor{}

which is not correct. It read var chain ClientOutboundInterceptor as a top-level line.


// Check for new doc links to add on top-level definitions only.
if !inFunc && !inInterface && isValidDefinition(trimmedNextLine, &inGroup, &inStruct) {
// Find the "Exposed As" line in the doc comment
var existingDoclink string
Expand Down Expand Up @@ -451,23 +469,6 @@ func processInternal(cfg config, file *os.File, pairs map[string]map[string]stri
}
}

// update inFunc after we actually check for doclinks to allow us to check
// a function's definition, without checking anything inside the function
if strings.HasPrefix(trimmedLine, "func ") {
funcSpaces = indentSize
inFunc = true
} else if inFunc && trimmedLine == "}" && funcSpaces == indentSize {
funcSpaces = -1
inFunc = false
}
if strings.HasSuffix(trimmedLine, "interface {") {
interfaceSpaces = indentSize
inInterface = true
} else if inInterface && trimmedLine == "}" && interfaceSpaces == indentSize {
interfaceSpaces = -1
inInterface = false
}

newFile += line + "\n"
}

Expand Down
1 change: 1 addition & 0 deletions internal/interceptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ type ClientOutboundInterceptor interface {
DescribeWorkflow(context.Context, *ClientDescribeWorkflowInput) (*ClientDescribeWorkflowOutput, error)

// ExecuteActivity intercepts client.Client.ExecuteActivity.
// interceptor.Header will return a non-nil map for this context.
Copy link
Copy Markdown
Contributor Author

@cconstable cconstable Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consistent with other interceptor methods

//
// NOTE: Experimental
ExecuteActivity(context.Context, *ClientExecuteActivityInput) (ClientActivityHandle, error)
Expand Down
4 changes: 4 additions & 0 deletions internal/interceptor_header.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ func contextWithNewHeader(ctx context.Context) context.Context {
return context.WithValue(ctx, headerKey{}, map[string]*commonpb.Payload{})
}

func NewHeaderContext(ctx context.Context) context.Context {
return contextWithNewHeader(ctx)
}

func contextWithoutHeader(ctx context.Context) context.Context {
return context.WithValue(ctx, headerKey{}, nil)
}
Expand Down
Loading