From 7a0b40059a3e042c1d49e3b8b67bece1f8b18bfd Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 16 Feb 2021 16:40:15 -0800 Subject: [PATCH 1/5] Document SDK span ending stops recording --- sdk/trace/span.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index b09b4fa2b1b..32a63eda4fe 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -144,6 +144,8 @@ func (s *span) SpanContext() trace.SpanContext { return s.spanContext } +// IsRecording returns if this span is being recorded. If this span has ended +// this will return false. func (s *span) IsRecording() bool { if s == nil { return false @@ -212,6 +214,7 @@ func (s *span) End(options ...trace.SpanOption) { config := trace.NewSpanConfig(options...) s.mu.Lock() + // Setting endTime to non-zero marks the span as ended and not recording. if config.Timestamp.IsZero() { s.endTime = et } else { From 6728de64a7f40b7a29437f7502d49f238c89dfa3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 16 Feb 2021 16:47:26 -0800 Subject: [PATCH 2/5] Guard SDK span set methods with record check --- sdk/trace/span.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 32a63eda4fe..1040a06fa43 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -156,9 +156,6 @@ func (s *span) IsRecording() bool { } func (s *span) SetStatus(code codes.Code, msg string) { - if s == nil { - return - } if !s.IsRecording() { return } @@ -175,7 +172,8 @@ func (s *span) SetAttributes(attributes ...label.KeyValue) { s.copyToCappedAttributes(attributes...) } -// End ends the span. +// End ends the span. This method does nothing if the span is already ended or +// is not being recorded. // // The only SpanOption currently supported is WithTimestamp which will set the // end time for a Span's life-cycle. @@ -183,7 +181,7 @@ func (s *span) SetAttributes(attributes ...label.KeyValue) { // If this method is called while panicking an error event is added to the // Span before ending it and the panic is continued. func (s *span) End(options ...trace.SpanOption) { - if s == nil { + if !s.IsRecording() { return } @@ -277,6 +275,10 @@ func (s *span) addEvent(name string, o ...trace.EventOption) { } func (s *span) SetName(name string) { + if !s.IsRecording() { + return + } + s.mu.Lock() defer s.mu.Unlock() From 6df8b64dd07b1355f51f296dc5b1d7df9a16ed3c Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 16 Feb 2021 17:04:37 -0800 Subject: [PATCH 3/5] Document SDK span exported API Make note of methods that do nothing if span is not recording. --- sdk/trace/span.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 1040a06fa43..4c84c7d5866 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -137,6 +137,7 @@ type span struct { var _ trace.Span = &span{} +// SpanContext returns the SpanContext of this span. func (s *span) SpanContext() trace.SpanContext { if s == nil { return trace.SpanContext{} @@ -155,6 +156,9 @@ func (s *span) IsRecording() bool { return s.endTime.IsZero() } +// SetStatus sets the status of this span in the form of a code and a +// message. This overrides the existing value of this span's status if one +// exists. If this span is not being recorded than this method does nothing. func (s *span) SetStatus(code codes.Code, msg string) { if !s.IsRecording() { return @@ -165,6 +169,12 @@ func (s *span) SetStatus(code codes.Code, msg string) { s.mu.Unlock() } +// SetAttributes sets attributes of this span. +// +// If a key from attributes already exists the value associated with that key +// will be overwritten with the value contained in attributes. +// +// If this span is not being recorded than this method does nothing. func (s *span) SetAttributes(attributes ...label.KeyValue) { if !s.IsRecording() { return @@ -229,6 +239,8 @@ func (s *span) End(options ...trace.SpanOption) { } } +// RecordError will record err as a span event for this span. If this span is +// not being recorded or err is nil than this method does nothing. func (s *span) RecordError(err error, opts ...trace.EventOption) { if s == nil || err == nil || !s.IsRecording() { return @@ -251,10 +263,13 @@ func typeStr(i interface{}) string { return fmt.Sprintf("%s.%s", t.PkgPath(), t.Name()) } +// Tracer returns the Tracer that created this span. func (s *span) Tracer() trace.Tracer { return s.tracer } +// AddEvent adds an event with the provided name and options. If this span is +// not being recorded than this method does nothing. func (s *span) AddEvent(name string, o ...trace.EventOption) { if !s.IsRecording() { return @@ -312,36 +327,43 @@ func (s *span) SetName(name string) { } } +// Name returns the name of this span. func (s *span) Name() string { s.mu.Lock() defer s.mu.Unlock() return s.name } +// Name returns the SpanContext of this span's parent span. func (s *span) Parent() trace.SpanContext { s.mu.Lock() defer s.mu.Unlock() return s.parent } +// SpanKind returns the SpanKind of this span. func (s *span) SpanKind() trace.SpanKind { s.mu.Lock() defer s.mu.Unlock() return s.spanKind } +// StartTime returns the time this span started. func (s *span) StartTime() time.Time { s.mu.Lock() defer s.mu.Unlock() return s.startTime } +// EndTime returns the time this span ended. For spans that have not yet +// ended, the returned value will be the zero value of time.Time. func (s *span) EndTime() time.Time { s.mu.Lock() defer s.mu.Unlock() return s.endTime } +// Attributes returns the attributes of this span. func (s *span) Attributes() []label.KeyValue { s.mu.Lock() defer s.mu.Unlock() @@ -351,6 +373,7 @@ func (s *span) Attributes() []label.KeyValue { return s.attributes.toKeyValue() } +// Events returns the links of this span. func (s *span) Links() []trace.Link { s.mu.Lock() defer s.mu.Unlock() @@ -360,6 +383,7 @@ func (s *span) Links() []trace.Link { return s.interfaceArrayToLinksArray() } +// Events returns the events of this span. func (s *span) Events() []trace.Event { s.mu.Lock() defer s.mu.Unlock() @@ -369,24 +393,30 @@ func (s *span) Events() []trace.Event { return s.interfaceArrayToMessageEventArray() } +// StatusCode returns the status code of this span. func (s *span) StatusCode() codes.Code { s.mu.Lock() defer s.mu.Unlock() return s.statusCode } +// StatusMessage returns the status message of this span. func (s *span) StatusMessage() string { s.mu.Lock() defer s.mu.Unlock() return s.statusMessage } +// InstrumentationLibrary returns the instrumentation.Library associated with +// the Tracer that created this span. func (s *span) InstrumentationLibrary() instrumentation.Library { s.mu.Lock() defer s.mu.Unlock() return s.instrumentationLibrary } +// Resource returns the Resource associated with the Tracer that created this +// span. func (s *span) Resource() *resource.Resource { s.mu.Lock() defer s.mu.Unlock() From 2f4e7b0bbf229b5a85c122846698a37c1afaf8d4 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 16 Feb 2021 17:20:08 -0800 Subject: [PATCH 4/5] Document SDK span End recording check --- sdk/trace/span.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 4c84c7d5866..67acb916008 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -191,7 +191,9 @@ func (s *span) SetAttributes(attributes ...label.KeyValue) { // If this method is called while panicking an error event is added to the // Span before ending it and the panic is continued. func (s *span) End(options ...trace.SpanOption) { - if !s.IsRecording() { + // Do not start by checking if the span is being recorded which requires + // acquiring a lock. Make a minimal check that the span is not nil. + if s == nil { return } @@ -199,6 +201,12 @@ func (s *span) End(options ...trace.SpanOption) { // the span's duration in case some operation below takes a while. et := internal.MonotonicEndTime(s.startTime) + // Do relative expensive check now that we have an end time and see if we + // need to do any more processing. + if !s.IsRecording() { + return + } + if recovered := recover(); recovered != nil { // Record but don't stop the panic. defer panic(recovered) @@ -215,10 +223,6 @@ func (s *span) End(options ...trace.SpanOption) { s.executionTracerTaskEnd() } - if !s.IsRecording() { - return - } - config := trace.NewSpanConfig(options...) s.mu.Lock() From 287e7636ccca2f52588a9d2385d2daf21ae03d7a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 16 Feb 2021 17:23:55 -0800 Subject: [PATCH 5/5] Document the SetName method --- sdk/trace/span.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 67acb916008..4fe4785d146 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -293,6 +293,8 @@ func (s *span) addEvent(name string, o ...trace.EventOption) { }) } +// SetName sets the name of this span. If this span is not being recorded than +// this method does nothing. func (s *span) SetName(name string) { if !s.IsRecording() { return