From d3b58b6a5e9270e513bb6b3c9aada3ee4ea2a256 Mon Sep 17 00:00:00 2001 From: lastchiliarch Date: Sat, 17 Apr 2021 23:18:07 +0800 Subject: [PATCH] Add WithStatus option to set Span status at the same time (#1677) Signed-off-by: lastchiliarch --- sdk/trace/span.go | 6 ++-- sdk/trace/trace_test.go | 70 +++++++++++++++++++++++------------------ trace/config_test.go | 2 +- 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 6683a275e9a8..56a8bb608005 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -245,9 +245,9 @@ func (s *span) End(options ...trace.SpanOption) { } // RecordError will record err as a span event for this span. -// this method does not change the Span status in default. -// If you want to change Span status, pass WithStatus as opts. -// this metod does nothing If this span is not being recorded or err is nil. +// This method does not change the Span status in default +// To change Span status, pass `WithStatus` as options. +// This method does nothing If this span is not being recorded or err is nil. func (s *span) RecordError(err error, opts ...trace.EventOption) { if s == nil || err == nil || !s.IsRecording() { return diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 9ba05f699cb5..315a4fa94a53 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1165,15 +1165,48 @@ func TestRecordErrorNil(t *testing.T) { } func TestRecordErrorWithStatus(t *testing.T) { + errTime := time.Now() scenarios := []struct { - err error - typ string - msg string + err error + want *SpanSnapshot }{ { err: ottest.NewTestError("test error"), - typ: "go.opentelemetry.io/otel/internal/internaltest.TestError", - msg: "test error", + want: &SpanSnapshot{ + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: tid, + TraceFlags: 0x1, + }), + Parent: sc.WithRemote(true), + Name: "span0", + StatusCode: codes.Error, + SpanKind: trace.SpanKindInternal, + MessageEvents: []trace.Event{ + { + Name: semconv.ExceptionEventName, + Time: errTime, + Attributes: []attribute.KeyValue{ + semconv.ExceptionTypeKey.String("go.opentelemetry.io/otel/internal/internaltest.TestError"), + semconv.ExceptionMessageKey.String("test error"), + }, + }, + }, + InstrumentationLibrary: instrumentation.Library{Name: "RecordError"}, + }, + }, + { + err: nil, + want: &SpanSnapshot{ + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: tid, + TraceFlags: 0x1, + }), + Parent: sc.WithRemote(true), + Name: "span0", + StatusCode: codes.Unset, + SpanKind: trace.SpanKindInternal, + InstrumentationLibrary: instrumentation.Library{Name: "RecordError"}, + }, }, } @@ -1181,37 +1214,12 @@ func TestRecordErrorWithStatus(t *testing.T) { te := NewTestExporter() tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty())) span := startSpan(tp, "RecordError") - - errTime := time.Now() span.RecordError(s.err, trace.WithTimestamp(errTime), trace.WithStatus(true)) - got, err := endSpan(te, span) if err != nil { t.Fatal(err) } - - want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: tid, - TraceFlags: 0x1, - }), - Parent: sc.WithRemote(true), - Name: "span0", - StatusCode: codes.Error, - SpanKind: trace.SpanKindInternal, - MessageEvents: []trace.Event{ - { - Name: semconv.ExceptionEventName, - Time: errTime, - Attributes: []attribute.KeyValue{ - semconv.ExceptionTypeKey.String(s.typ), - semconv.ExceptionMessageKey.String(s.msg), - }, - }, - }, - InstrumentationLibrary: instrumentation.Library{Name: "RecordError"}, - } - if diff := cmpDiff(got, want); diff != "" { + if diff := cmpDiff(got, s.want); diff != "" { t.Errorf("SpanErrorOptions: -got +want %s", diff) } } diff --git a/trace/config_test.go b/trace/config_test.go index 703119b278a5..f24422f8eb95 100644 --- a/trace/config_test.go +++ b/trace/config_test.go @@ -41,7 +41,7 @@ func TestNewSpanConfig(t *testing.T) { } withStatusOpt := WithStatus(true) - //just for coverage + // For coverage withStatusOpt.private() tests := []struct {