From 65cf87cc4f495dcc6988cbec97f6956b6a000132 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 27 Aug 2020 19:00:59 -0700 Subject: [PATCH 01/12] Unify API Span Start/End Options Replace both with `SpanOption`. Add a unified `SpanConfig` to match and a `SpanConfigure` function to parse a `SpanConfig` from `SpanOption`s. Update all the related options to use new `SpanOption`s. --- api/trace/api.go | 179 ++++++++++++++++++++++++++--------------------- 1 file changed, 98 insertions(+), 81 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index 19528d43bda..ab08a17c37b 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -52,23 +52,7 @@ func WithInstrumentationVersion(version string) TracerOption { type Tracer interface { // Start a span. - Start(ctx context.Context, spanName string, opts ...StartOption) (context.Context, Span) -} - -// EndConfig provides options to set properties of span at the time of ending -// the span. -type EndConfig struct { - EndTime time.Time -} - -// EndOption applies changes to EndConfig that sets options when the span is ended. -type EndOption func(*EndConfig) - -// WithEndTime sets the end time of the span to provided time t, when it is ended. -func WithEndTime(t time.Time) EndOption { - return func(c *EndConfig) { - c.EndTime = t - } + Start(ctx context.Context, spanName string, opts ...SpanOption) (context.Context, Span) } // ErrorConfig provides options to set properties of an error event at the time it is recorded. @@ -100,7 +84,7 @@ type Span interface { // End completes the span. No updates are allowed to span after it // ends. The only exception is setting status of the span. - End(options ...EndOption) + End(options ...SpanOption) // AddEvent adds an event to the span. AddEvent(ctx context.Context, name string, attrs ...label.KeyValue) @@ -137,18 +121,103 @@ type Span interface { SetAttribute(string, interface{}) } -// StartOption applies changes to StartConfig that sets options at span start time. -type StartOption func(*StartConfig) - -// StartConfig provides options to set properties of span at the time of starting -// a new span. -type StartConfig struct { +// SpanConfig is a group of options for a Span. +type SpanConfig struct { + // Attributes describe the associated qualities of a Span. Attributes []label.KeyValue - StartTime time.Time - Links []Link - Record bool - NewRoot bool - SpanKind SpanKind + // Timestamp is a time in a Span life-cycle. + Timestamp time.Time + // Links are the associations a Span has with other Spans. + Links []Link + // Record is the recording state of a Span. + Record bool + // NewRoot identifies a Span as the root Span for a new trace. This is + // commonly used when an existing trace crosses trust boundaries and the + // remote parent span context should be ignored for security. + NewRoot bool + // SpanKind is the SpanKind a Span is. + SpanKind SpanKind +} + +// SpanConfigure applies all the options to a returned SpanConfig. This sets a +// default value for the following SpanConfig fields if they are not set, +// otherwise a field can be assumed to be the zero-value: +// +// * Timestamp: set to the time this function is called at. +func SpanConfigure(options []SpanOption) *SpanConfig { + config := new(SpanConfig) + for _, option := range options { + option.Apply(config) + } + if config.Timestamp.IsZero() { + config.Timestamp = time.Now() + } + return config +} + +// SpanOption applies an options to a SpanConfig. +type SpanOption interface { + Apply(*SpanConfig) +} + +type attributeSpanOption []label.KeyValue + +func (o attributeSpanOption) Apply(c *SpanConfig) { c.Attributes = []label.KeyValue(o) } + +// WithAttributes sets the attributes of a span. These attributes are meant to +// provide additional information about the work the Span represents. +func WithAttributes(attrs ...label.KeyValue) SpanOption { + return attributeSpanOption(attrs) +} + +type timestampSpanOption time.Time + +func (o timestampSpanOption) Apply(c *SpanConfig) { c.Timestamp = time.Time(o) } + +// WithTimestamp sets the time of a Span life-cycle moment (e.g. started or +// stopped). +func WithTimestamp(t time.Time) SpanOption { + return timestampSpanOption(t) +} + +type linksSpanOption []Link + +func (o linksSpanOption) Apply(c *SpanConfig) { c.Links = append(c.Links, []Link(o)...) } + +// WithLinks adds links to a Span. +func WithLinks(links ...Link) SpanOption { + return linksSpanOption(links) +} + +type recordSpanOption bool + +func (o recordSpanOption) Apply(c *SpanConfig) { c.Record = bool(o) } + +// WithRecord specifies that the span should be recorded. It is important to +// note that implementations may override this option, i.e. if the span is a +// child of an un-sampled trace. +func WithRecord() SpanOption { + return recordSpanOption(true) +} + +type newRootSpanOption bool + +func (o newRootSpanOption) Apply(c *SpanConfig) { c.NewRoot = bool(o) } + +// WithNewRoot specifies that the Span should be treated as a root Span. The +// parent span context should be ignored when defining the Span's trace +// identifiers. +func WithNewRoot() SpanOption { + return newRootSpanOption(true) +} + +type spanKindSpanOption SpanKind + +func (o spanKindSpanOption) Apply(c *SpanConfig) { c.SpanKind = SpanKind(o) } + +// WithSpanKind sets the SpanKind of a Span. +func WithSpanKind(kind SpanKind) SpanOption { + return spanKindSpanOption(kind) } // Link is used to establish relationship between two spans within the same Trace or @@ -219,55 +288,3 @@ func (sk SpanKind) String() string { return "unspecified" } } - -// WithStartTime sets the start time of the span to provided time t, when it is started. -// In absence of this option, wall clock time is used as start time. -// This option is typically used when starting of the span is delayed. -func WithStartTime(t time.Time) StartOption { - return func(c *StartConfig) { - c.StartTime = t - } -} - -// WithAttributes sets attributes to span. These attributes provides additional -// data about the span. -// Multiple `WithAttributes` options appends the attributes preserving the order. -func WithAttributes(attrs ...label.KeyValue) StartOption { - return func(c *StartConfig) { - c.Attributes = append(c.Attributes, attrs...) - } -} - -// WithRecord specifies that the span should be recorded. -// Note that the implementation may still override this preference, -// e.g., if the span is a child in an unsampled trace. -func WithRecord() StartOption { - return func(c *StartConfig) { - c.Record = true - } -} - -// WithNewRoot specifies that the current span or remote span context -// in context passed to `Start` should be ignored when deciding about -// a parent, which effectively means creating a span with new trace -// ID. The current span and the remote span context may be added as -// links to the span by the implementation. -func WithNewRoot() StartOption { - return func(c *StartConfig) { - c.NewRoot = true - } -} - -// LinkedTo allows instantiating a Span with initial Links. -func LinkedTo(sc SpanContext, attrs ...label.KeyValue) StartOption { - return func(c *StartConfig) { - c.Links = append(c.Links, Link{sc, attrs}) - } -} - -// WithSpanKind specifies the role a Span on a Trace. -func WithSpanKind(sk SpanKind) StartOption { - return func(c *StartConfig) { - c.SpanKind = sk - } -} From fc21674367f0366c6540cc2f92bed865d94c682d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 27 Aug 2020 20:21:25 -0700 Subject: [PATCH 02/12] No non-zero SpanConfig defaults The SDK uses an internal clock for the current time that cannot be use if it does not know the time has not been set. --- api/trace/api.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index ab08a17c37b..1fac413ee61 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -139,19 +139,12 @@ type SpanConfig struct { SpanKind SpanKind } -// SpanConfigure applies all the options to a returned SpanConfig. This sets a -// default value for the following SpanConfig fields if they are not set, -// otherwise a field can be assumed to be the zero-value: -// -// * Timestamp: set to the time this function is called at. +// SpanConfigure applies all the options to a returned SpanConfig. func SpanConfigure(options []SpanOption) *SpanConfig { config := new(SpanConfig) for _, option := range options { option.Apply(config) } - if config.Timestamp.IsZero() { - config.Timestamp = time.Now() - } return config } From 66a46bc8428ad90de85486d3f32eef49902568d6 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 27 Aug 2020 20:22:40 -0700 Subject: [PATCH 03/12] Append attributes for WithAttributes This preserves existing behavior. --- api/trace/api.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index 1fac413ee61..b0e18a34b87 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -155,12 +155,14 @@ type SpanOption interface { type attributeSpanOption []label.KeyValue -func (o attributeSpanOption) Apply(c *SpanConfig) { c.Attributes = []label.KeyValue(o) } +func (o attributeSpanOption) Apply(c *SpanConfig) { + c.Attributes = append(c.Attributes, []label.KeyValue(o)...) +} -// WithAttributes sets the attributes of a span. These attributes are meant to +// WithAttributes adds the attributes to a span. These attributes are meant to // provide additional information about the work the Span represents. -func WithAttributes(attrs ...label.KeyValue) SpanOption { - return attributeSpanOption(attrs) +func WithAttributes(attributes ...label.KeyValue) SpanOption { + return attributeSpanOption(attributes) } type timestampSpanOption time.Time From aa68e3f51fd2e4925f8ba8bbcdc6759c36b7d238 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 27 Aug 2020 20:23:19 -0700 Subject: [PATCH 04/12] Add unit test for SpanConfigure --- api/trace/span_test.go | 195 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 api/trace/span_test.go diff --git a/api/trace/span_test.go b/api/trace/span_test.go new file mode 100644 index 00000000000..a97a37e3066 --- /dev/null +++ b/api/trace/span_test.go @@ -0,0 +1,195 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package trace + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/label" +) + +func TestSpanConfigure(t *testing.T) { + k1v1 := label.String("key1", "value1") + k1v2 := label.String("key1", "value2") + k2v2 := label.String("key2", "value2") + + timestamp0 := time.Unix(0, 0) + timestamp1 := time.Unix(0, 0) + + link1 := Link{ + SpanContext: SpanContext{TraceID: ID([16]byte{1, 1}), SpanID: SpanID{3}}, + Attributes: []label.KeyValue{k1v1}, + } + link2 := Link{ + SpanContext: SpanContext{TraceID: ID([16]byte{1, 1}), SpanID: SpanID{3}}, + Attributes: []label.KeyValue{k1v2, k2v2}, + } + + tests := []struct { + options []SpanOption + expected *SpanConfig + }{ + { + // No non-zero-values should be set. + []SpanOption{}, + new(SpanConfig), + }, + { + []SpanOption{ + WithAttributes(k1v1), + }, + &SpanConfig{ + Attributes: []label.KeyValue{k1v1}, + }, + }, + { + // Multiple calls should append not overwrite. + []SpanOption{ + WithAttributes(k1v1), + WithAttributes(k1v2), + WithAttributes(k2v2), + }, + &SpanConfig{ + // No uniqueness is guaranteed by the API. + Attributes: []label.KeyValue{k1v1, k1v2, k2v2}, + }, + }, + { + []SpanOption{ + WithAttributes(k1v1, k1v2, k2v2), + }, + &SpanConfig{ + // No uniqueness is guaranteed by the API. + Attributes: []label.KeyValue{k1v1, k1v2, k2v2}, + }, + }, + { + []SpanOption{ + WithTimestamp(timestamp0), + }, + &SpanConfig{ + Timestamp: timestamp0, + }, + }, + { + []SpanOption{ + // Multiple calls overwrites with last-one-wins. + WithTimestamp(timestamp0), + WithTimestamp(timestamp1), + }, + &SpanConfig{ + Timestamp: timestamp1, + }, + }, + { + []SpanOption{ + WithLinks(link1), + }, + &SpanConfig{ + Links: []Link{link1}, + }, + }, + { + []SpanOption{ + // Multiple calls should append not overwrite. + WithLinks(link1), + WithLinks(link1, link2), + }, + &SpanConfig{ + // No uniqueness is guaranteed by the API. + Links: []Link{link1, link1, link2}, + }, + }, + { + []SpanOption{ + WithRecord(), + }, + &SpanConfig{ + Record: true, + }, + }, + { + []SpanOption{ + // Multiple calls should not change record state. + WithRecord(), + WithRecord(), + }, + &SpanConfig{ + Record: true, + }, + }, + { + []SpanOption{ + WithNewRoot(), + }, + &SpanConfig{ + NewRoot: true, + }, + }, + { + []SpanOption{ + // Multiple calls should not change record state. + WithNewRoot(), + WithNewRoot(), + }, + &SpanConfig{ + NewRoot: true, + }, + }, + { + []SpanOption{ + WithSpanKind(SpanKindConsumer), + }, + &SpanConfig{ + SpanKind: SpanKindConsumer, + }, + }, + { + []SpanOption{ + // Multiple calls overwrites with last-one-wins. + WithSpanKind(SpanKindClient), + WithSpanKind(SpanKindConsumer), + }, + &SpanConfig{ + SpanKind: SpanKindConsumer, + }, + }, + { + // Everything should work together. + []SpanOption{ + WithAttributes(k1v1), + WithTimestamp(timestamp0), + WithLinks(link1, link2), + WithRecord(), + WithNewRoot(), + WithSpanKind(SpanKindConsumer), + }, + &SpanConfig{ + Attributes: []label.KeyValue{k1v1}, + Timestamp: timestamp0, + Links: []Link{link1, link2}, + Record: true, + NewRoot: true, + SpanKind: SpanKindConsumer, + }, + }, + } + for _, test := range tests { + assert.Equal(t, test.expected, SpanConfigure(test.options)) + } +} From c138f9e33b6f9d96592677e020ed926b43fa6c66 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 27 Aug 2020 20:23:34 -0700 Subject: [PATCH 05/12] Propagate changes --- api/global/internal/trace.go | 2 +- api/trace/context_test.go | 2 +- api/trace/noop_span.go | 2 +- api/trace/noop_trace.go | 2 +- api/trace/tracetest/span.go | 11 +++----- api/trace/tracetest/span_test.go | 4 +-- api/trace/tracetest/tracer.go | 10 +++---- api/trace/tracetest/tracer_test.go | 8 +++--- bridge/opentracing/bridge.go | 21 ++++++++------- bridge/opentracing/internal/mock.go | 42 ++++++++++++----------------- bridge/opentracing/wrapper.go | 2 +- internal/trace/mock_span.go | 2 +- internal/trace/mock_tracer.go | 9 +++---- sdk/trace/span.go | 15 +++++------ sdk/trace/trace_test.go | 39 +++++++++++++-------------- sdk/trace/tracer.go | 16 +++++------ 16 files changed, 79 insertions(+), 108 deletions(-) diff --git a/api/global/internal/trace.go b/api/global/internal/trace.go index 69024b66769..bd17dddf4ff 100644 --- a/api/global/internal/trace.go +++ b/api/global/internal/trace.go @@ -116,7 +116,7 @@ func (t *tracer) setDelegate(provider trace.Provider) { // Start implements trace.Tracer by forwarding the call to t.delegate if // set, otherwise it forwards the call to a NoopTracer. -func (t *tracer) Start(ctx context.Context, name string, opts ...trace.StartOption) (context.Context, trace.Span) { +func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) { if t.delegate != nil { return t.delegate.Start(ctx, name, opts...) } diff --git a/api/trace/context_test.go b/api/trace/context_test.go index 57c1b0a04d0..4af89292641 100644 --- a/api/trace/context_test.go +++ b/api/trace/context_test.go @@ -101,7 +101,7 @@ func (mockSpan) SetAttribute(k string, v interface{}) { } // End does nothing. -func (mockSpan) End(options ...trace.EndOption) { +func (mockSpan) End(options ...trace.SpanOption) { } // RecordError does nothing. diff --git a/api/trace/noop_span.go b/api/trace/noop_span.go index 27f0d319f3d..d205daf887c 100644 --- a/api/trace/noop_span.go +++ b/api/trace/noop_span.go @@ -54,7 +54,7 @@ func (NoopSpan) SetAttribute(k string, v interface{}) { } // End does nothing. -func (NoopSpan) End(options ...EndOption) { +func (NoopSpan) End(options ...SpanOption) { } // RecordError does nothing. diff --git a/api/trace/noop_trace.go b/api/trace/noop_trace.go index ffdb913185e..9b67ea3d884 100644 --- a/api/trace/noop_trace.go +++ b/api/trace/noop_trace.go @@ -23,7 +23,7 @@ type NoopTracer struct{} var _ Tracer = NoopTracer{} // Start starts a noop span. -func (NoopTracer) Start(ctx context.Context, name string, opts ...StartOption) (context.Context, Span) { +func (NoopTracer) Start(ctx context.Context, name string, opts ...SpanOption) (context.Context, Span) { span := NoopSpan{} return ContextWithSpan(ctx, span), span } diff --git a/api/trace/tracetest/span.go b/api/trace/tracetest/span.go index c60e8525c83..8ea113497b8 100644 --- a/api/trace/tracetest/span.go +++ b/api/trace/tracetest/span.go @@ -55,7 +55,7 @@ func (s *Span) Tracer() trace.Tracer { return s.tracer } -func (s *Span) End(opts ...trace.EndOption) { +func (s *Span) End(opts ...trace.SpanOption) { s.lock.Lock() defer s.lock.Unlock() @@ -63,14 +63,9 @@ func (s *Span) End(opts ...trace.EndOption) { return } - var c trace.EndConfig - for _, opt := range opts { - opt(&c) - } - + c := trace.SpanConfigure(opts) s.endTime = time.Now() - - if endTime := c.EndTime; !endTime.IsZero() { + if endTime := c.Timestamp; !endTime.IsZero() { s.endTime = endTime } diff --git a/api/trace/tracetest/span_test.go b/api/trace/tracetest/span_test.go index 11fa68b988e..a0f8642fa4c 100644 --- a/api/trace/tracetest/span_test.go +++ b/api/trace/tracetest/span_test.go @@ -101,7 +101,7 @@ func TestSpan(t *testing.T) { e.Expect(endTime).ToEqual(expectedEndTime) }) - t.Run("uses the time from WithEndTime", func(t *testing.T) { + t.Run("uses the time from WithTimestamp", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) @@ -113,7 +113,7 @@ func TestSpan(t *testing.T) { e.Expect(ok).ToBeTrue() expectedEndTime := time.Now().AddDate(5, 0, 0) - subject.End(trace.WithEndTime(expectedEndTime)) + subject.End(trace.WithTimestamp(expectedEndTime)) e.Expect(subject.Ended()).ToBeTrue() diff --git a/api/trace/tracetest/tracer.go b/api/trace/tracetest/tracer.go index 9662c106898..5b25a110225 100644 --- a/api/trace/tracetest/tracer.go +++ b/api/trace/tracetest/tracer.go @@ -34,14 +34,10 @@ type Tracer struct { config *config } -func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.StartOption) (context.Context, trace.Span) { - var c trace.StartConfig - for _, opt := range opts { - opt(&c) - } - +func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) { + c := trace.SpanConfigure(opts) startTime := time.Now() - if st := c.StartTime; !st.IsZero() { + if st := c.Timestamp; !st.IsZero() { startTime = st } diff --git a/api/trace/tracetest/tracer_test.go b/api/trace/tracetest/tracer_test.go index f2999e2c515..43e246e9201 100644 --- a/api/trace/tracetest/tracer_test.go +++ b/api/trace/tracetest/tracer_test.go @@ -47,7 +47,7 @@ func TestTracer(t *testing.T) { return span, nil }) - t.Run("uses the start time from WithStartTime", func(t *testing.T) { + t.Run("uses the start time from WithTimestamp", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) @@ -55,7 +55,7 @@ func TestTracer(t *testing.T) { expectedStartTime := time.Now().AddDate(5, 0, 0) subject := tp.Tracer(t.Name()) - _, span := subject.Start(context.Background(), "test", trace.WithStartTime(expectedStartTime)) + _, span := subject.Start(context.Background(), "test", trace.WithTimestamp(expectedStartTime)) testSpan, ok := span.(*tracetest.Span) e.Expect(ok).ToBeTrue() @@ -223,7 +223,7 @@ func TestTracer(t *testing.T) { e.Expect(gotLinks).ToMatchInAnyOrder(expectedLinks) }) - t.Run("uses the links provided through LinkedTo", func(t *testing.T) { + t.Run("uses the links provided through WithLinks", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) @@ -246,7 +246,7 @@ func TestTracer(t *testing.T) { }, } - _, span = subject.Start(context.Background(), "test", trace.LinkedTo(link1.SpanContext, link1.Attributes...), trace.LinkedTo(link2.SpanContext, link2.Attributes...)) + _, span = subject.Start(context.Background(), "test", trace.WithLinks(link1, link2)) testSpan, ok := span.(*tracetest.Span) e.Expect(ok).ToBeTrue() diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 97d6786ee33..6fbe9bbac54 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -99,10 +99,10 @@ func (s *bridgeSpan) Finish() { } func (s *bridgeSpan) FinishWithOptions(opts ot.FinishOptions) { - var otelOpts []oteltrace.EndOption + var otelOpts []oteltrace.SpanOption if !opts.FinishTime.IsZero() { - otelOpts = append(otelOpts, oteltrace.WithEndTime(opts.FinishTime)) + otelOpts = append(otelOpts, oteltrace.WithTimestamp(opts.FinishTime)) } for _, record := range opts.LogRecords { s.logRecord(record) @@ -389,14 +389,15 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio if parentBridgeSC != nil { checkCtx = oteltrace.ContextWithRemoteSpanContext(checkCtx, parentBridgeSC.otelSpanContext) } - checkCtx2, otelSpan := t.setTracer.tracer().Start(checkCtx, operationName, func(opts *oteltrace.StartConfig) { - opts.Attributes = attributes - opts.StartTime = sso.StartTime - opts.Links = links - opts.Record = true - opts.NewRoot = false - opts.SpanKind = kind - }) + checkCtx2, otelSpan := t.setTracer.tracer().Start( + checkCtx, + operationName, + oteltrace.WithAttributes(attributes...), + oteltrace.WithTimestamp(sso.StartTime), + oteltrace.WithLinks(links...), + oteltrace.WithRecord(), + oteltrace.WithSpanKind(kind), + ) if checkCtx != checkCtx2 { t.warnOnce.Do(func() { t.warningHandler("SDK should have deferred the context setup, see the documentation of go.opentelemetry.io/otel/bridge/opentracing/migration\n") diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index f346ffa33ac..59861ea09d2 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -70,17 +70,14 @@ func NewMockTracer() *MockTracer { } } -func (t *MockTracer) Start(ctx context.Context, name string, opts ...oteltrace.StartOption) (context.Context, oteltrace.Span) { - spanOpts := oteltrace.StartConfig{} - for _, opt := range opts { - opt(&spanOpts) - } - startTime := spanOpts.StartTime +func (t *MockTracer) Start(ctx context.Context, name string, opts ...oteltrace.SpanOption) (context.Context, oteltrace.Span) { + config := oteltrace.SpanConfigure(opts) + startTime := config.Timestamp if startTime.IsZero() { startTime = time.Now() } spanContext := oteltrace.SpanContext{ - TraceID: t.getTraceID(ctx, &spanOpts), + TraceID: t.getTraceID(ctx, config), SpanID: t.getSpanID(), TraceFlags: 0, } @@ -88,15 +85,15 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...oteltrace.S mockTracer: t, officialTracer: t, spanContext: spanContext, - recording: spanOpts.Record, + recording: config.Record, Attributes: otelcorrelation.NewMap(otelcorrelation.MapUpdate{ - MultiKV: spanOpts.Attributes, + MultiKV: config.Attributes, }), StartTime: startTime, EndTime: time.Time{}, - ParentSpanID: t.getParentSpanID(ctx, &spanOpts), + ParentSpanID: t.getParentSpanID(ctx, config), Events: nil, - SpanKind: oteltrace.ValidateSpanKind(spanOpts.SpanKind), + SpanKind: oteltrace.ValidateSpanKind(config.SpanKind), } if !migration.SkipContextSetup(ctx) { ctx = oteltrace.ContextWithSpan(ctx, span) @@ -118,8 +115,8 @@ func (t *MockTracer) addSpareContextValue(ctx context.Context) context.Context { return ctx } -func (t *MockTracer) getTraceID(ctx context.Context, spanOpts *oteltrace.StartConfig) oteltrace.ID { - if parent := t.getParentSpanContext(ctx, spanOpts); parent.IsValid() { +func (t *MockTracer) getTraceID(ctx context.Context, config *oteltrace.SpanConfig) oteltrace.ID { + if parent := t.getParentSpanContext(ctx, config); parent.IsValid() { return parent.TraceID } if len(t.SpareTraceIDs) > 0 { @@ -133,15 +130,15 @@ func (t *MockTracer) getTraceID(ctx context.Context, spanOpts *oteltrace.StartCo return t.getRandTraceID() } -func (t *MockTracer) getParentSpanID(ctx context.Context, spanOpts *oteltrace.StartConfig) oteltrace.SpanID { - if parent := t.getParentSpanContext(ctx, spanOpts); parent.IsValid() { +func (t *MockTracer) getParentSpanID(ctx context.Context, config *oteltrace.SpanConfig) oteltrace.SpanID { + if parent := t.getParentSpanContext(ctx, config); parent.IsValid() { return parent.SpanID } return oteltrace.SpanID{} } -func (t *MockTracer) getParentSpanContext(ctx context.Context, spanOpts *oteltrace.StartConfig) oteltrace.SpanContext { - spanCtx, _, _ := otelparent.GetSpanContextAndLinks(ctx, spanOpts.NewRoot) +func (t *MockTracer) getParentSpanContext(ctx context.Context, config *oteltrace.SpanConfig) oteltrace.SpanContext { + spanCtx, _, _ := otelparent.GetSpanContextAndLinks(ctx, config.NewRoot) return spanCtx } @@ -239,17 +236,12 @@ func (s *MockSpan) applyUpdate(update otelcorrelation.MapUpdate) { s.Attributes = s.Attributes.Apply(update) } -func (s *MockSpan) End(options ...oteltrace.EndOption) { +func (s *MockSpan) End(options ...oteltrace.SpanOption) { if !s.EndTime.IsZero() { return // already finished } - endOpts := oteltrace.EndConfig{} - - for _, opt := range options { - opt(&endOpts) - } - - endTime := endOpts.EndTime + config := oteltrace.SpanConfigure(options) + endTime := config.Timestamp if endTime.IsZero() { endTime = time.Now() } diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index f133c116edc..f5070695bdc 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -74,7 +74,7 @@ func (t *WrapperTracer) otelTracer() oteltrace.Tracer { // Start forwards the call to the wrapped tracer. It also tries to // override the tracer of the returned span if the span implements the // OverrideTracerSpanExtension interface. -func (t *WrapperTracer) Start(ctx context.Context, name string, opts ...oteltrace.StartOption) (context.Context, oteltrace.Span) { +func (t *WrapperTracer) Start(ctx context.Context, name string, opts ...oteltrace.SpanOption) (context.Context, oteltrace.Span) { ctx, span := t.otelTracer().Start(ctx, name, opts...) if spanWithExtension, ok := span.(migration.OverrideTracerSpanExtension); ok { spanWithExtension.OverrideTracer(t) diff --git a/internal/trace/mock_span.go b/internal/trace/mock_span.go index fe9146e562e..212f53d779d 100644 --- a/internal/trace/mock_span.go +++ b/internal/trace/mock_span.go @@ -67,7 +67,7 @@ func (ms *MockSpan) SetAttribute(k string, v interface{}) { } // End does nothing. -func (ms *MockSpan) End(options ...apitrace.EndOption) { +func (ms *MockSpan) End(options ...apitrace.SpanOption) { } // RecordError does nothing. diff --git a/internal/trace/mock_tracer.go b/internal/trace/mock_tracer.go index 312516ebb1e..763d82ec456 100644 --- a/internal/trace/mock_tracer.go +++ b/internal/trace/mock_tracer.go @@ -47,15 +47,12 @@ var _ apitrace.Tracer = (*MockTracer)(nil) // TracdID is used from Parent Span Context and SpanID is assigned. // If Parent SpanContext option is not specified then random TraceID is used. // No other options are supported. -func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.StartOption) (context.Context, apitrace.Span) { - var opts apitrace.StartConfig - for _, op := range o { - op(&opts) - } +func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.SpanOption) (context.Context, apitrace.Span) { + config := apitrace.SpanConfigure(o) var span *MockSpan var sc apitrace.SpanContext - parentSpanContext, _, _ := parent.GetSpanContextAndLinks(ctx, opts.NewRoot) + parentSpanContext, _, _ := parent.GetSpanContextAndLinks(ctx, config.NewRoot) if !parentSpanContext.IsValid() { sc = apitrace.SpanContext{} diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 9e93bfe215b..b86fb3d1b27 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -109,7 +109,7 @@ func (s *span) SetAttribute(k string, v interface{}) { } // End ends the span adding an error event if it was called while panicking. -func (s *span) End(options ...apitrace.EndOption) { +func (s *span) End(options ...apitrace.SpanOption) { if s == nil { return } @@ -131,19 +131,16 @@ func (s *span) End(options ...apitrace.EndOption) { if !s.IsRecording() { return } - opts := apitrace.EndConfig{} - for _, opt := range options { - opt(&opts) - } + config := apitrace.SpanConfigure(options) s.endOnce.Do(func() { sps, _ := s.tracer.provider.spanProcessors.Load().(spanProcessorMap) mustExportOrProcess := len(sps) > 0 if mustExportOrProcess { sd := s.makeSpanData() - if opts.EndTime.IsZero() { + if config.Timestamp.IsZero() { sd.EndTime = internal.MonotonicEndTime(sd.StartTime) } else { - sd.EndTime = opts.EndTime + sd.EndTime = config.Timestamp } for sp := range sps { sp.OnEnd(sd) @@ -324,7 +321,7 @@ func (s *span) addChild() { s.mu.Unlock() } -func startSpanInternal(tr *tracer, name string, parent apitrace.SpanContext, remoteParent bool, o apitrace.StartConfig) *span { +func startSpanInternal(tr *tracer, name string, parent apitrace.SpanContext, remoteParent bool, o *apitrace.SpanConfig) *span { var noParent bool span := &span{} span.spanContext = parent @@ -355,7 +352,7 @@ func startSpanInternal(tr *tracer, name string, parent apitrace.SpanContext, rem return span } - startTime := o.StartTime + startTime := o.Timestamp if startTime.IsZero() { startTime = time.Now() } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 8a35b2930cc..2851a33b91e 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -488,13 +488,11 @@ func TestLinks(t *testing.T) { sc1 := apitrace.SpanContext{TraceID: apitrace.ID([16]byte{1, 1}), SpanID: apitrace.SpanID{3}} sc2 := apitrace.SpanContext{TraceID: apitrace.ID([16]byte{1, 1}), SpanID: apitrace.SpanID{3}} - span := startSpan(tp, "Links", - apitrace.LinkedTo(sc1, label.String("key1", "value1")), - apitrace.LinkedTo(sc2, - label.String("key2", "value2"), - label.String("key3", "value3"), - ), - ) + links := []apitrace.Link{ + {SpanContext: sc1, Attributes: []label.KeyValue{k1v1}}, + {SpanContext: sc2, Attributes: []label.KeyValue{k2v2, k3v3}}, + } + span := startSpan(tp, "Links", apitrace.WithLinks(links...)) got, err := endSpan(te, span) if err != nil { @@ -506,13 +504,10 @@ func TestLinks(t *testing.T) { TraceID: tid, TraceFlags: 0x1, }, - ParentSpanID: sid, - Name: "span0", - HasRemoteParent: true, - Links: []apitrace.Link{ - {SpanContext: sc1, Attributes: []label.KeyValue{k1v1}}, - {SpanContext: sc2, Attributes: []label.KeyValue{k2v2, k3v3}}, - }, + ParentSpanID: sid, + Name: "span0", + HasRemoteParent: true, + Links: links, SpanKind: apitrace.SpanKindInternal, InstrumentationLibrary: instrumentation.Library{Name: "Links"}, } @@ -532,9 +527,11 @@ func TestLinksOverLimit(t *testing.T) { tp, _ := NewProvider(WithConfig(cfg), WithSyncer(te)) span := startSpan(tp, "LinksOverLimit", - apitrace.LinkedTo(sc1, label.String("key1", "value1")), - apitrace.LinkedTo(sc2, label.String("key2", "value2")), - apitrace.LinkedTo(sc3, label.String("key3", "value3")), + apitrace.WithLinks( + apitrace.Link{SpanContext: sc1, Attributes: []label.KeyValue{label.String("key1", "value1")}}, + apitrace.Link{SpanContext: sc2, Attributes: []label.KeyValue{label.String("key2", "value2")}}, + apitrace.Link{SpanContext: sc3, Attributes: []label.KeyValue{label.String("key3", "value3")}}, + ), ) k2v2 := label.String("key2", "value2") @@ -656,7 +653,7 @@ func checkChild(p apitrace.SpanContext, apiSpan apitrace.Span) error { // startSpan starts a span with a name "span0". See startNamedSpan for // details. -func startSpan(tp *Provider, trName string, args ...apitrace.StartOption) apitrace.Span { +func startSpan(tp *Provider, trName string, args ...apitrace.SpanOption) apitrace.Span { return startNamedSpan(tp, trName, "span0", args...) } @@ -664,7 +661,7 @@ func startSpan(tp *Provider, trName string, args ...apitrace.StartOption) apitra // passed name and with remote span context as parent. The remote span // context contains TraceFlags with sampled bit set. This allows the // span to be automatically sampled. -func startNamedSpan(tp *Provider, trName, name string, args ...apitrace.StartOption) apitrace.Span { +func startNamedSpan(tp *Provider, trName, name string, args ...apitrace.SpanOption) apitrace.Span { ctx := context.Background() ctx = apitrace.ContextWithRemoteSpanContext(ctx, remoteSpanContext()) args = append(args, apitrace.WithRecord()) @@ -894,9 +891,9 @@ func TestCustomStartEndTime(t *testing.T) { _, span := tp.Tracer("Custom Start and End time").Start( context.Background(), "testspan", - apitrace.WithStartTime(startTime), + apitrace.WithTimestamp(startTime), ) - span.End(apitrace.WithEndTime(endTime)) + span.End(apitrace.WithTimestamp(endTime)) if len(te.spans) != 1 { t.Fatalf("got exported spans %#v, want one span", te.spans) diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 8d395c58efb..97bf122935b 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -29,14 +29,10 @@ type tracer struct { var _ apitrace.Tracer = &tracer{} -func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.StartOption) (context.Context, apitrace.Span) { - var opts apitrace.StartConfig +func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.SpanOption) (context.Context, apitrace.Span) { + config := apitrace.SpanConfigure(o) - for _, op := range o { - op(&opts) - } - - parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, opts.NewRoot) + parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, config.NewRoot) if p := apitrace.SpanFromContext(ctx); p != nil { if sdkSpan, ok := p.(*span); ok { @@ -44,14 +40,14 @@ func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.StartOpt } } - span := startSpanInternal(tr, name, parentSpanContext, remoteParent, opts) + span := startSpanInternal(tr, name, parentSpanContext, remoteParent, config) for _, l := range links { span.addLink(l) } - for _, l := range opts.Links { + for _, l := range config.Links { span.addLink(l) } - span.SetAttributes(opts.Attributes...) + span.SetAttributes(config.Attributes...) span.tracer = tr From 6457d7ff3072f4eb7204e3a0ad3bc5d601f3e95f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 28 Aug 2020 08:58:45 -0700 Subject: [PATCH 06/12] Update append option documentation --- api/trace/api.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index b0e18a34b87..5f749496d46 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -160,7 +160,11 @@ func (o attributeSpanOption) Apply(c *SpanConfig) { } // WithAttributes adds the attributes to a span. These attributes are meant to -// provide additional information about the work the Span represents. +// provide additional information about the work the Span represents. The +// attributes are added to the existing Span attributes, i.e. this does not +// overwrite. There is no guarantee of uniqueness of the final set of +// attributes made by the API, instead it is left to implementations of the +// SDK to determine this. func WithAttributes(attributes ...label.KeyValue) SpanOption { return attributeSpanOption(attributes) } @@ -179,7 +183,10 @@ type linksSpanOption []Link func (o linksSpanOption) Apply(c *SpanConfig) { c.Links = append(c.Links, []Link(o)...) } -// WithLinks adds links to a Span. +// WithLinks adds links to a Span. The links are added to the existing Span +// links, i.e. this does not overwrite. There is no guarantee of uniqueness of +// the final set of links made by the API, instead it is left to the +// implementations of the SDK to determine this. func WithLinks(links ...Link) SpanOption { return linksSpanOption(links) } From fe22f16ebd901f1c9b60349ea38e4d4babff5fa3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 28 Aug 2020 08:59:58 -0700 Subject: [PATCH 07/12] Update testing comments --- api/trace/span_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/trace/span_test.go b/api/trace/span_test.go index a97a37e3066..6cd35528bfc 100644 --- a/api/trace/span_test.go +++ b/api/trace/span_test.go @@ -125,7 +125,7 @@ func TestSpanConfigure(t *testing.T) { }, { []SpanOption{ - // Multiple calls should not change record state. + // Multiple calls should not change Record state. WithRecord(), WithRecord(), }, @@ -143,7 +143,7 @@ func TestSpanConfigure(t *testing.T) { }, { []SpanOption{ - // Multiple calls should not change record state. + // Multiple calls should not change NewRoot state. WithNewRoot(), WithNewRoot(), }, From be303bd26a1d10392cece33657abf752cd601977 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 28 Aug 2020 09:21:45 -0700 Subject: [PATCH 08/12] Move comments on guarantees to appropriate function --- api/trace/api.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index 5f749496d46..043416e9404 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -140,6 +140,11 @@ type SpanConfig struct { } // SpanConfigure applies all the options to a returned SpanConfig. +// The default value for all the fields of the returned SpanConfig are the +// default zero value of the type. Also, this does not perform any validation +// on the returned SpanConfig (e.g. no uniqueness checking or bounding of +// data), instead it is left to the implementations of the SDK to perform this +// action. func SpanConfigure(options []SpanOption) *SpanConfig { config := new(SpanConfig) for _, option := range options { @@ -162,9 +167,7 @@ func (o attributeSpanOption) Apply(c *SpanConfig) { // WithAttributes adds the attributes to a span. These attributes are meant to // provide additional information about the work the Span represents. The // attributes are added to the existing Span attributes, i.e. this does not -// overwrite. There is no guarantee of uniqueness of the final set of -// attributes made by the API, instead it is left to implementations of the -// SDK to determine this. +// overwrite. func WithAttributes(attributes ...label.KeyValue) SpanOption { return attributeSpanOption(attributes) } @@ -184,9 +187,7 @@ type linksSpanOption []Link func (o linksSpanOption) Apply(c *SpanConfig) { c.Links = append(c.Links, []Link(o)...) } // WithLinks adds links to a Span. The links are added to the existing Span -// links, i.e. this does not overwrite. There is no guarantee of uniqueness of -// the final set of links made by the API, instead it is left to the -// implementations of the SDK to determine this. +// links, i.e. this does not overwrite. func WithLinks(links ...Link) SpanOption { return linksSpanOption(links) } From b8f7822730c9bdf87ae51d2f8c38a8b5d5e38299 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 28 Aug 2020 09:54:02 -0700 Subject: [PATCH 09/12] Add documentation for SDK methods Include SDK implementation specific information in the Tracer Start method and Span End method. --- sdk/trace/span.go | 8 +++++++- sdk/trace/tracer.go | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index b86fb3d1b27..46c055c4391 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -108,7 +108,13 @@ func (s *span) SetAttribute(k string, v interface{}) { } } -// End ends the span adding an error event if it was called while panicking. +// End ends the span. +// +// The only SpanOption currently supported is WithTimestamp which will set the +// end time for a Span's life-cycle. +// +// 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 ...apitrace.SpanOption) { if s == nil { return diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 97bf122935b..ec785999e8b 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -29,8 +29,14 @@ type tracer struct { var _ apitrace.Tracer = &tracer{} -func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.SpanOption) (context.Context, apitrace.Span) { - config := apitrace.SpanConfigure(o) +// Start starts a Span and returns it along with a context containing it. +// +// The Span is created with the provided name and as a child of any existing +// span context found in the passed context. The created Span will be +// configured appropriately by any SpanOption passed. Any Timestamp option +// passed will be used as the start time of the Span's life-cycle. +func (tr *tracer) Start(ctx context.Context, name string, options ...apitrace.SpanOption) (context.Context, apitrace.Span) { + config := apitrace.SpanConfigure(options) parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, config.NewRoot) From 231fc0bbeb6ac43b33cda3af10fb2dd070813032 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 28 Aug 2020 10:40:02 -0700 Subject: [PATCH 10/12] Add changes to Changelog --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e553b56ae9c..6659f13d62c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- A `SpanConfigure` function in `go.opentelemetry.io/otel/api/trace` to create a new `SpanConfig` from `SpanOption`s. (#1108) + +### Changed + +- Replace `StartOption` and `EndOption` in `go.opentelemetry.io/otel/api/trace` with `SpanOption`. + This changes is matched by replacing the `StartConfig` and `EndConfig` with a unified `SpanConfig`. (#1108) +- Replace the `LinkedTo` span option in `go.opentelemetry.io/otel/api/trace` with `WithLinks`. + This is be more consistent with our other option patterns, i.e. passing the item to be configured directly instead of its component parts, and a provides a cleaner function signature. (#1108) + ## [0.11.0] - 2020-08-24 ### Added From 8fa6098f61e2926d0ce331302333fdbc78813e50 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 31 Aug 2020 09:07:22 -0700 Subject: [PATCH 11/12] Apply suggestions from code review Co-authored-by: ET --- CHANGELOG.md | 4 ++-- api/trace/api.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6659f13d62c..d9361b48ec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - Replace `StartOption` and `EndOption` in `go.opentelemetry.io/otel/api/trace` with `SpanOption`. - This changes is matched by replacing the `StartConfig` and `EndConfig` with a unified `SpanConfig`. (#1108) + This change is matched by replacing the `StartConfig` and `EndConfig` with a unified `SpanConfig`. (#1108) - Replace the `LinkedTo` span option in `go.opentelemetry.io/otel/api/trace` with `WithLinks`. - This is be more consistent with our other option patterns, i.e. passing the item to be configured directly instead of its component parts, and a provides a cleaner function signature. (#1108) + This is be more consistent with our other option patterns, i.e. passing the item to be configured directly instead of its component parts, and provides a cleaner function signature. (#1108) ## [0.11.0] - 2020-08-24 diff --git a/api/trace/api.go b/api/trace/api.go index 043416e9404..3721dffaa10 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -143,7 +143,7 @@ type SpanConfig struct { // The default value for all the fields of the returned SpanConfig are the // default zero value of the type. Also, this does not perform any validation // on the returned SpanConfig (e.g. no uniqueness checking or bounding of -// data), instead it is left to the implementations of the SDK to perform this +// data). Instead, it is left to the implementations of the SDK to perform this // action. func SpanConfigure(options []SpanOption) *SpanConfig { config := new(SpanConfig) @@ -153,7 +153,7 @@ func SpanConfigure(options []SpanOption) *SpanConfig { return config } -// SpanOption applies an options to a SpanConfig. +// SpanOption applies an option to a SpanConfig. type SpanOption interface { Apply(*SpanConfig) } @@ -207,8 +207,8 @@ type newRootSpanOption bool func (o newRootSpanOption) Apply(c *SpanConfig) { c.NewRoot = bool(o) } -// WithNewRoot specifies that the Span should be treated as a root Span. The -// parent span context should be ignored when defining the Span's trace +// WithNewRoot specifies that the Span should be treated as a root Span. Any +// existing parent span context will be ignored when defining the Span's trace // identifiers. func WithNewRoot() SpanOption { return newRootSpanOption(true) From 3abbae3a1274ab58f7332e7bca191539d29d2f66 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 31 Aug 2020 09:11:57 -0700 Subject: [PATCH 12/12] Update the SpanKind comment in the SpanConfig Try for a less tautological comment. --- api/trace/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/trace/api.go b/api/trace/api.go index 3721dffaa10..aecdbaf8f37 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -135,7 +135,7 @@ type SpanConfig struct { // commonly used when an existing trace crosses trust boundaries and the // remote parent span context should be ignored for security. NewRoot bool - // SpanKind is the SpanKind a Span is. + // SpanKind is the role a Span has in a trace. SpanKind SpanKind }