Skip to content
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

datadog: Add tracer option to check error eligibility in the span #1438

Merged
merged 3 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 23 additions & 6 deletions contrib/datadog/tracing/interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ type TracerOptions struct {

// DisableQueryTracing can be set to disable query tracing.
DisableQueryTracing bool

// OnFinish sets finish options.
// If unset, this will use [tracer.WithError]
// in case [interceptor.TracerFinishSpanOptions.Error] is non-nil and not [workflow.IsContinueAsNewError].
OnFinish func(options *interceptor.TracerFinishSpanOptions) []tracer.FinishOption
}

// NewTracingInterceptor convenience method that wraps a NeTracer() with a tracing interceptor
Expand All @@ -57,10 +62,23 @@ func NewTracingInterceptor(opts TracerOptions) interceptor.Interceptor {
// NewTracer creates an interceptor for setting on client options
// that implements Datadog tracing for workflows.
func NewTracer(opts TracerOptions) interceptor.Tracer {
if opts.OnFinish == nil {
opts.OnFinish = func(options *interceptor.TracerFinishSpanOptions) []tracer.FinishOption {
var finishOpts []tracer.FinishOption

if err := options.Error; err != nil && !workflow.IsContinueAsNewError(err) {
finishOpts = append(finishOpts, tracer.WithError(err))
}

return finishOpts
}
}
Comment on lines +65 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of having if-else statement in the Finish function, I've decided to set a default function in the constructor.

Copy link
Member

Choose a reason for hiding this comment

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

👍 I think this is a good approach


return &tracerImpl{
opts: TracerOptions{
DisableSignalTracing: opts.DisableSignalTracing,
DisableQueryTracing: opts.DisableQueryTracing,
OnFinish: opts.OnFinish,
},
}
}
Expand Down Expand Up @@ -114,7 +132,7 @@ func (t *tracerImpl) SpanFromContext(ctx context.Context) interceptor.TracerSpan
if !ok {
return nil
}
return &tracerSpan{Span: span}
return &tracerSpan{OnFinish: t.opts.OnFinish, Span: span}
}

func (t *tracerImpl) ContextWithSpan(ctx context.Context, span interceptor.TracerSpan) context.Context {
Expand Down Expand Up @@ -174,7 +192,7 @@ func (t *tracerImpl) StartSpan(options *interceptor.TracerStartSpanOptions) (int

// Start and return span
s := tracer.StartSpan(t.SpanName(options), startOpts...)
return &tracerSpan{Span: s}, nil
return &tracerSpan{OnFinish: t.opts.OnFinish, Span: s}, nil
}

func (t *tracerImpl) GetLogger(logger log.Logger, ref interceptor.TracerSpanRef) log.Logger {
Expand Down Expand Up @@ -223,6 +241,7 @@ func (r spanContextReader) ForeachKey(handler func(key string, value string) err

type tracerSpan struct {
ddtrace.Span
OnFinish func(options *interceptor.TracerFinishSpanOptions) []tracer.FinishOption
}
type tracerSpanCtx struct {
ddtrace.SpanContext
Expand All @@ -241,9 +260,7 @@ func (t *tracerSpan) ForeachBaggageItem(handler func(k string, v string) bool) {
}

func (t *tracerSpan) Finish(options *interceptor.TracerFinishSpanOptions) {
var opts []tracer.FinishOption
if err := options.Error; err != nil && !workflow.IsContinueAsNewError(err) {
opts = append(opts, tracer.WithError(err))
}
opts := t.OnFinish(options)

t.Span.Finish(opts...)
}
29 changes: 29 additions & 0 deletions contrib/datadog/tracing/interceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
package tracing

import (
"strings"
"testing"

"github.com/stretchr/testify/require"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"

"go.temporal.io/sdk/interceptor"
"go.temporal.io/sdk/internal/interceptortest"
Expand Down Expand Up @@ -110,3 +112,30 @@ func Test_tracerImpl_genSpanID(t1 *testing.T) {
})
}
}
func Test_OnFinishOption(t *testing.T) {
mt := mocktracer.Start()
defer mt.Stop()

onFinish := func(options *interceptor.TracerFinishSpanOptions) []tracer.FinishOption {
var finishOpts []tracer.FinishOption

if err := options.Error; strings.Contains(err.Error(), "ignore me") {
finishOpts = append(finishOpts, tracer.WithError(err))
}

return finishOpts
}

impl := NewTracer(TracerOptions{OnFinish: onFinish})
trc := testTracer{
Tracer: impl,
mt: mt,
}

interceptortest.RunTestWorkflowWithError(t, trc)

spans := trc.FinishedSpans()

require.Len(t, spans, 1)
require.Equal(t, "temporal.RunWorkflow", spans[0].Name)
}
26 changes: 26 additions & 0 deletions internal/interceptortest/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package interceptortest

import (
"context"
"errors"
"fmt"
"testing"
"time"
Expand Down Expand Up @@ -90,6 +91,27 @@ func RunTestWorkflow(t *testing.T, tracer interceptor.Tracer) {
require.Equal(t, "query-response", queryResp)
}

func RunTestWorkflowWithError(t *testing.T, tracer interceptor.Tracer) {
var suite testsuite.WorkflowTestSuite
env := suite.NewTestWorkflowEnvironment()

env.RegisterWorkflow(testWorkflowWithError)

// Set tracer interceptor
env.SetWorkerOptions(worker.Options{
Interceptors: []interceptor.WorkerInterceptor{interceptor.NewTracingInterceptor(tracer)},
})

env.SetStartTime(testWorkflowStartTime)

// Exec
env.ExecuteWorkflow(testWorkflowWithError)

// Confirm result
require.True(t, env.IsWorkflowCompleted())
require.Error(t, env.GetWorkflowError())
}

func AssertSpanPropagation(t *testing.T, tracer TestTracer) {

require.Equal(t, []*SpanInfo{
Expand All @@ -110,6 +132,10 @@ func AssertSpanPropagation(t *testing.T, tracer TestTracer) {
}, tracer.FinishedSpans())
}

func testWorkflowWithError(_ workflow.Context) error {
return errors.New("ignore me")
}

func testWorkflow(ctx workflow.Context) ([]string, error) {
// Run code
ret, err := workflowInternal(ctx, false)
Expand Down
Loading