-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Add WithStackTrace option for tracer #8094
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,9 @@ type tracerProviderConfig struct { | |
|
|
||
| // resource contains attributes representing an entity that produces telemetry. | ||
| resource *resource.Resource | ||
|
|
||
| // stackTrace configs whether to capture stack trace for all recorded errors and panics. | ||
| stackTrace bool | ||
| } | ||
|
|
||
| // MarshalLog is the marshaling function used by the logging system to represent this Provider. | ||
|
|
@@ -52,12 +55,14 @@ func (cfg tracerProviderConfig) MarshalLog() any { | |
| IDGeneratorType string | ||
| SpanLimits SpanLimits | ||
| Resource *resource.Resource | ||
| StackTrace bool | ||
| }{ | ||
| SpanProcessors: cfg.processors, | ||
| SamplerType: fmt.Sprintf("%T", cfg.sampler), | ||
| IDGeneratorType: fmt.Sprintf("%T", cfg.idGenerator), | ||
| SpanLimits: cfg.spanLimits, | ||
| Resource: cfg.resource, | ||
| StackTrace: cfg.stackTrace, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -78,6 +83,7 @@ type TracerProvider struct { | |
| idGenerator IDGenerator | ||
| spanLimits SpanLimits | ||
| resource *resource.Resource | ||
| stackTrace bool | ||
| } | ||
|
|
||
| var _ trace.TracerProvider = &TracerProvider{} | ||
|
|
@@ -110,6 +116,7 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider { | |
| idGenerator: o.idGenerator, | ||
| spanLimits: o.spanLimits, | ||
| resource: o.resource, | ||
| stackTrace: o.stackTrace, | ||
| } | ||
| global.Info("TracerProvider created", "config", o) | ||
|
|
||
|
|
@@ -384,6 +391,15 @@ func WithIDGenerator(g IDGenerator) TracerProviderOption { | |
| }) | ||
| } | ||
|
|
||
| // WithStackTrace configures the TracerProvider to capture a stack trace | ||
| // for all recorded errors and panics. | ||
| func WithStackTrace(b bool) TracerProviderOption { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking out loud: Do we want to be able to support anything else related to this? NeverStackTrace?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see the benefit of NeverStackTrace. This would not allow the user to set the stackTrace on some important code path 🤔
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some possible use-cases:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added NeverStackTrace |
||
| return traceProviderOptionFunc(func(cfg tracerProviderConfig) tracerProviderConfig { | ||
| cfg.stackTrace = b | ||
| return cfg | ||
| }) | ||
| } | ||
|
|
||
| // WithSampler returns a TracerProviderOption that will configure the Sampler | ||
| // s as a TracerProvider's Sampler. The configured Sampler is used by the | ||
| // Tracers the TracerProvider creates to make their sampling decisions for the | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1346,6 +1346,71 @@ func TestRecordErrorWithStackTrace(t *testing.T) { | |
| ) | ||
| } | ||
|
|
||
| func TestProviderRecordErrorWithStackTrace(t *testing.T) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this and the other test are mostly the same as the tests above them. If it makes sense, make these table-driven instead of separate tests.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried. Let's see. |
||
| err := newTestError("test error") | ||
| typ := "go.opentelemetry.io/otel/sdk/trace.testError" | ||
| msg := "test error" | ||
|
|
||
| te := NewTestExporter() | ||
| tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty()), WithStackTrace(true)) | ||
| span := startSpan(tp, "RecordError") | ||
|
|
||
| errTime := time.Now() | ||
| span.RecordError(err, trace.WithTimestamp(errTime)) | ||
|
|
||
| got, err := endSpan(te, span) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| want := &snapshot{ | ||
| spanContext: trace.NewSpanContext(trace.SpanContextConfig{ | ||
| TraceID: tid, | ||
| TraceFlags: 0x1, | ||
| }), | ||
| parent: sc.WithRemote(true), | ||
| name: "span0", | ||
| status: Status{Code: codes.Unset}, | ||
| spanKind: trace.SpanKindInternal, | ||
| events: []Event{ | ||
| { | ||
| Name: semconv.ExceptionEventName, | ||
| Time: errTime, | ||
| Attributes: []attribute.KeyValue{ | ||
| semconv.ExceptionType(typ), | ||
| semconv.ExceptionMessage(msg), | ||
| }, | ||
| }, | ||
| }, | ||
| instrumentationScope: instrumentation.Scope{Name: "RecordError"}, | ||
| } | ||
|
|
||
| assert.Equal(t, want.spanContext, got.spanContext) | ||
| assert.Equal(t, want.parent, got.parent) | ||
| assert.Equal(t, want.name, got.name) | ||
| assert.Equal(t, want.status, got.status) | ||
| assert.Equal(t, want.spanKind, got.spanKind) | ||
| assert.Equal(t, got.events[0].Attributes[0].Value.AsString(), want.events[0].Attributes[0].Value.AsString()) | ||
| assert.Equal(t, got.events[0].Attributes[1].Value.AsString(), want.events[0].Attributes[1].Value.AsString()) | ||
| gotStackTraceFunctionName := strings.Split(got.events[0].Attributes[2].Value.AsString(), "\n") | ||
|
|
||
| assert.Truef( | ||
| t, | ||
| strings.HasPrefix(gotStackTraceFunctionName[1], "go.opentelemetry.io/otel/sdk/trace.recordStackTrace"), | ||
| "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.recordStackTrace", | ||
| gotStackTraceFunctionName[1], | ||
| ) | ||
| assert.Truef( | ||
| t, | ||
| strings.HasPrefix( | ||
| gotStackTraceFunctionName[3], | ||
| "go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).RecordError", | ||
| ), | ||
| "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).RecordError", | ||
| gotStackTraceFunctionName[3], | ||
| ) | ||
| } | ||
|
|
||
| func TestRecordErrorNil(t *testing.T) { | ||
| te := NewTestExporter() | ||
| tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty())) | ||
|
|
@@ -1603,6 +1668,41 @@ func TestSpanCapturesPanicWithStackTrace(t *testing.T) { | |
| ) | ||
| } | ||
|
|
||
| func TestProviderSpanCapturesPanicWithStackTrace(t *testing.T) { | ||
| te := NewTestExporter() | ||
| tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty()), WithStackTrace(true)) | ||
| _, span := tp.Tracer("CatchPanic").Start( | ||
| t.Context(), | ||
| "span", | ||
| ) | ||
|
|
||
| f := func() { | ||
| defer span.End() | ||
| panic(errors.New("error message")) | ||
| } | ||
| require.PanicsWithError(t, "error message", f) | ||
| spans := te.Spans() | ||
| require.Len(t, spans, 1) | ||
| require.Len(t, spans[0].Events(), 1) | ||
| assert.Equal(t, semconv.ExceptionEventName, spans[0].Events()[0].Name) | ||
| assert.Equal(t, "*errors.errorString", spans[0].Events()[0].Attributes[0].Value.AsString()) | ||
| assert.Equal(t, "error message", spans[0].Events()[0].Attributes[1].Value.AsString()) | ||
|
|
||
| gotStackTraceFunctionName := strings.Split(spans[0].Events()[0].Attributes[2].Value.AsString(), "\n") | ||
| assert.Truef( | ||
| t, | ||
| strings.HasPrefix(gotStackTraceFunctionName[1], "go.opentelemetry.io/otel/sdk/trace.recordStackTrace"), | ||
| "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.recordStackTrace", | ||
| gotStackTraceFunctionName[1], | ||
| ) | ||
| assert.Truef( | ||
| t, | ||
| strings.HasPrefix(gotStackTraceFunctionName[3], "go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End"), | ||
| "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End", | ||
| gotStackTraceFunctionName[3], | ||
| ) | ||
| } | ||
|
|
||
| func TestReadOnlySpan(t *testing.T) { | ||
| kv := attribute.String("foo", "bar") | ||
|
|
||
|
|
||
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.
Maybe something like WithAlwaysStackTrace() or something could make it clearer what this does.
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.
Makes sense. Will do.