From c1f460e097d395fa7cd02acc4a6096d6c6f14b08 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 27 May 2021 09:53:56 -0500 Subject: [PATCH] Update API configs. (#1921) * Added Reason to Contributing and Updated TracerConfig * PR comment fixup * Changed how span Options work. * Fix Markdown linting * Added meter configs. * Fixes from PR comments * fix for missing instrument Co-authored-by: Tyler Yahn --- CHANGELOG.md | 5 + CONTRIBUTING.md | 12 +- bridge/opencensus/bridge.go | 4 +- bridge/opentracing/bridge.go | 2 +- bridge/opentracing/internal/mock.go | 22 +-- bridge/opentracing/wrapper.go | 2 +- internal/global/meter.go | 2 +- internal/global/trace.go | 6 +- internal/global/trace_test.go | 9 +- metric/config.go | 105 +++++++----- metric/metric.go | 16 +- metric/metric_sdkapi.go | 2 +- metric/metric_test.go | 140 ++++++++++++--- oteltest/provider.go | 4 +- oteltest/span.go | 12 +- oteltest/tracer.go | 14 +- sdk/trace/provider.go | 2 +- sdk/trace/span.go | 32 ++-- sdk/trace/trace_test.go | 6 +- sdk/trace/tracer.go | 8 +- trace/config.go | 255 +++++++++++++++++----------- trace/config_test.go | 66 +++---- trace/noop.go | 4 +- trace/trace.go | 4 +- 24 files changed, 452 insertions(+), 282 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 905eb249b43..77cf87b1343 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Spans created by the global `Tracer` obtained from `go.opentelemetry.io/otel`, prior to a functioning `TracerProvider` being set, now propagate the span context from their parent if one exists. (#1901) - The `"go.opentelemetry.io/otel".Tracer` function now accepts tracer options. (#1902) - Move the `go.opentelemetry.io/otel/unit` package to `go.opentelemetry.io/otel/metric/unit`. (#1903) +- Changed `go.opentelemetry.io/otel/trace.TracerConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config.) (#1921) +- Changed `go.opentelemetry.io/otel/trace.SpanConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config). (#1921) +- Changed `span.End()` now only accepts Options that are allowed at `end()`. (#1921) +- Changed `go.opentelemetry.io/otel/metric.InstrumentConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config). (#1921) +- Changed `go.opentelemetry.io/otel/metric.MeterConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config). (#1921) - Refactor option types according to the contribution style guide. (#1882) - Move the `go.opentelemetry.io/otel/trace.TraceStateFromKeyValues` function to the `go.opentelemetry.io/otel/oteltest` package. This function is preserved for testing purposes where it may be useful to create a `TraceState` from `attribute.KeyValue`s, but it is not intended for production use. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ded63f87121..eec0975eb5a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -190,8 +190,16 @@ will likely want to build custom options for the configuration, the `config` should be exported. Please, include in the documentation for the `config` how the user can extend the configuration. -It is important that `config` are not shared across package boundaries. -Meaning a `config` from one package should not be directly used by another. +It is important that internal `config` are not shared across package boundaries. +Meaning a `config` from one package should not be directly used by another. The +one exception is the API packages. The configs from the base API, eg. +`go.opentelemetry.io/otel/trace.TracerConfig` and +`go.opentelemetry.io/otel/metric.InstrumentConfig`, are intended to be consumed +by the SDK therefor it is expected that these are exported. + +When a config is exported we want to maintain forward and backward +compatibility, to achieve this no fields should be exported but should +instead be accessed by methods. Optionally, it is common to include a `newConfig` function (with the same naming scheme). This function wraps any defaults setting and looping over diff --git a/bridge/opencensus/bridge.go b/bridge/opencensus/bridge.go index b63fb94ec2e..ecf1d5181ca 100644 --- a/bridge/opencensus/bridge.go +++ b/bridge/opencensus/bridge.go @@ -45,12 +45,12 @@ func (o *otelTracer) StartSpan(ctx context.Context, name string, s ...octrace.St return ctx, octrace.NewSpan(&span{otSpan: sp}) } -func convertStartOptions(optFns []octrace.StartOption, name string) []trace.SpanOption { +func convertStartOptions(optFns []octrace.StartOption, name string) []trace.SpanStartOption { var ocOpts octrace.StartOptions for _, fn := range optFns { fn(&ocOpts) } - otOpts := []trace.SpanOption{} + otOpts := []trace.SpanStartOption{} switch ocOpts.SpanKind { case octrace.SpanKindClient: otOpts = append(otOpts, trace.WithSpanKind(trace.SpanKindClient)) diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index a6af1fdf1aa..316ddd8764e 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -98,7 +98,7 @@ func (s *bridgeSpan) Finish() { } func (s *bridgeSpan) FinishWithOptions(opts ot.FinishOptions) { - var otelOpts []trace.SpanOption + var otelOpts []trace.SpanEndOption if !opts.FinishTime.IsZero() { otelOpts = append(otelOpts, trace.WithTimestamp(opts.FinishTime)) diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index e57d9129b85..11198805063 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -70,9 +70,9 @@ func NewMockTracer() *MockTracer { } } -func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) { - config := trace.NewSpanConfig(opts...) - startTime := config.Timestamp +func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { + config := trace.NewSpanStartConfig(opts...) + startTime := config.Timestamp() if startTime.IsZero() { startTime = time.Now() } @@ -86,13 +86,13 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanO officialTracer: t, spanContext: spanContext, Attributes: baggage.NewMap(baggage.MapUpdate{ - MultiKV: config.Attributes, + MultiKV: config.Attributes(), }), StartTime: startTime, EndTime: time.Time{}, ParentSpanID: t.getParentSpanID(ctx, config), Events: nil, - SpanKind: trace.ValidateSpanKind(config.SpanKind), + SpanKind: trace.ValidateSpanKind(config.SpanKind()), } if !migration.SkipContextSetup(ctx) { ctx = trace.ContextWithSpan(ctx, span) @@ -137,7 +137,7 @@ func (t *MockTracer) getParentSpanID(ctx context.Context, config *trace.SpanConf } func (t *MockTracer) getParentSpanContext(ctx context.Context, config *trace.SpanConfig) trace.SpanContext { - if !config.NewRoot { + if !config.NewRoot() { return trace.SpanContextFromContext(ctx) } return trace.SpanContext{} @@ -232,12 +232,12 @@ func (s *MockSpan) applyUpdate(update baggage.MapUpdate) { s.Attributes = s.Attributes.Apply(update) } -func (s *MockSpan) End(options ...trace.SpanOption) { +func (s *MockSpan) End(options ...trace.SpanEndOption) { if !s.EndTime.IsZero() { return // already finished } - config := trace.NewSpanConfig(options...) - endTime := config.Timestamp + config := trace.NewSpanEndConfig(options...) + endTime := config.Timestamp() if endTime.IsZero() { endTime = time.Now() } @@ -269,10 +269,10 @@ func (s *MockSpan) Tracer() trace.Tracer { func (s *MockSpan) AddEvent(name string, o ...trace.EventOption) { c := trace.NewEventConfig(o...) s.Events = append(s.Events, MockEvent{ - Timestamp: c.Timestamp, + Timestamp: c.Timestamp(), Name: name, Attributes: baggage.NewMap(baggage.MapUpdate{ - MultiKV: c.Attributes, + MultiKV: c.Attributes(), }), }) } diff --git a/bridge/opentracing/wrapper.go b/bridge/opentracing/wrapper.go index 149e520a634..d199a22adfd 100644 --- a/bridge/opentracing/wrapper.go +++ b/bridge/opentracing/wrapper.go @@ -74,7 +74,7 @@ func (t *WrapperTracer) otelTracer() trace.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 ...trace.SpanOption) (context.Context, trace.Span) { +func (t *WrapperTracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { ctx, span := t.otelTracer().Start(ctx, name, opts...) if spanWithExtension, ok := span.(migration.OverrideTracerSpanExtension); ok { spanWithExtension.OverrideTracer(t) diff --git a/internal/global/meter.go b/internal/global/meter.go index 36f9edeb8c6..d91c0b8a234 100644 --- a/internal/global/meter.go +++ b/internal/global/meter.go @@ -153,7 +153,7 @@ func (p *meterProvider) Meter(instrumentationName string, opts ...metric.MeterOp key := meterKey{ Name: instrumentationName, - Version: metric.NewMeterConfig(opts...).InstrumentationVersion, + Version: metric.NewMeterConfig(opts...).InstrumentationVersion(), } entry, ok := p.meters[key] if !ok { diff --git a/internal/global/trace.go b/internal/global/trace.go index aedf1168804..c08facd3dfe 100644 --- a/internal/global/trace.go +++ b/internal/global/trace.go @@ -92,7 +92,7 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T key := il{ name: name, - version: trace.NewTracerConfig(opts...).InstrumentationVersion, + version: trace.NewTracerConfig(opts...).InstrumentationVersion(), } if p.tracers == nil { @@ -139,7 +139,7 @@ func (t *tracer) setDelegate(provider trace.TracerProvider) { // 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.SpanOption) (context.Context, trace.Span) { +func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { delegate := t.delegate.Load() if delegate != nil { return delegate.(trace.Tracer).Start(ctx, name, opts...) @@ -175,7 +175,7 @@ func (nonRecordingSpan) SetError(bool) {} func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {} // End does nothing. -func (nonRecordingSpan) End(...trace.SpanOption) {} +func (nonRecordingSpan) End(...trace.SpanEndOption) {} // RecordError does nothing. func (nonRecordingSpan) RecordError(error, ...trace.EventOption) {} diff --git a/internal/global/trace_test.go b/internal/global/trace_test.go index 4ca53c35984..e084ebb23c7 100644 --- a/internal/global/trace_test.go +++ b/internal/global/trace_test.go @@ -74,10 +74,10 @@ func (fn fnTracerProvider) Tracer(instrumentationName string, opts ...trace.Trac } type fnTracer struct { - start func(ctx context.Context, spanName string, opts ...trace.SpanOption) (context.Context, trace.Span) + start func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) } -func (fn fnTracer) Start(ctx context.Context, spanName string, opts ...trace.SpanOption) (context.Context, trace.Span) { +func (fn fnTracer) Start(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { return fn.start(ctx, spanName, opts...) } @@ -93,7 +93,6 @@ func TestTraceProviderDelegates(t *testing.T) { tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { called = true assert.Equal(t, "abc", name) - assert.Equal(t, []trace.TracerOption{trace.WithInstrumentationVersion("xyz")}, opts) return trace.NewNoopTracerProvider().Tracer("") }, }) @@ -131,7 +130,6 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) { tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { newVal := atomic.AddInt32(&called, 1) assert.Equal(t, "abc", name) - assert.Equal(t, []trace.TracerOption{trace.WithInstrumentationVersion("xyz")}, opts) if newVal == 10 { // Signal the goroutine to finish. close(quit) @@ -175,9 +173,8 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) { otel.SetTracerProvider(fnTracerProvider{ tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { assert.Equal(t, "abc", name) - assert.Equal(t, []trace.TracerOption{trace.WithInstrumentationVersion("xyz")}, opts) return fnTracer{ - start: func(ctx context.Context, spanName string, opts ...trace.SpanOption) (context.Context, trace.Span) { + start: func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { newVal := atomic.AddInt32(&called, 1) assert.Equal(t, "name", spanName) if newVal == 10 { diff --git a/metric/config.go b/metric/config.go index 8877ae78021..053d0699b7a 100644 --- a/metric/config.go +++ b/metric/config.go @@ -20,23 +20,39 @@ import ( // InstrumentConfig contains options for metric instrument descriptors. type InstrumentConfig struct { - // Description describes the instrument in human-readable terms. - Description string - // Unit describes the measurement unit for a instrument. - Unit unit.Unit - // InstrumentationName is the name of the library providing - // instrumentation. - InstrumentationName string - // InstrumentationVersion is the version of the library providing - // instrumentation. - InstrumentationVersion string + description string + unit unit.Unit + instrumentationName string + instrumentationVersion string +} + +// Description describes the instrument in human-readable terms. +func (cfg InstrumentConfig) Description() string { + return cfg.description +} + +// Unit describes the measurement unit for a instrument. +func (cfg InstrumentConfig) Unit() unit.Unit { + return cfg.unit +} + +// InstrumentationName is the name of the library providing +// instrumentation. +func (cfg InstrumentConfig) InstrumentationName() string { + return cfg.instrumentationName +} + +// InstrumentationVersion is the version of the library providing +// instrumentation. +func (cfg InstrumentConfig) InstrumentationVersion() string { + return cfg.instrumentationVersion } // InstrumentOption is an interface for applying metric instrument options. type InstrumentOption interface { // ApplyMeter is used to set a InstrumentOption value of a // InstrumentConfig. - ApplyInstrument(*InstrumentConfig) + applyInstrument(*InstrumentConfig) } // NewInstrumentConfig creates a new InstrumentConfig @@ -44,55 +60,52 @@ type InstrumentOption interface { func NewInstrumentConfig(opts ...InstrumentOption) InstrumentConfig { var config InstrumentConfig for _, o := range opts { - o.ApplyInstrument(&config) + o.applyInstrument(&config) } return config } -// WithDescription applies provided description. -func WithDescription(desc string) InstrumentOption { - return descriptionOption(desc) -} +type instrumentOptionFunc func(*InstrumentConfig) -type descriptionOption string +func (fn instrumentOptionFunc) applyInstrument(cfg *InstrumentConfig) { + fn(cfg) +} -func (d descriptionOption) ApplyInstrument(config *InstrumentConfig) { - config.Description = string(d) +// WithDescription applies provided description. +func WithDescription(desc string) InstrumentOption { + return instrumentOptionFunc(func(cfg *InstrumentConfig) { + cfg.description = desc + }) } // WithUnit applies provided unit. func WithUnit(unit unit.Unit) InstrumentOption { - return unitOption(unit) -} - -type unitOption unit.Unit - -func (u unitOption) ApplyInstrument(config *InstrumentConfig) { - config.Unit = unit.Unit(u) + return instrumentOptionFunc(func(cfg *InstrumentConfig) { + cfg.unit = unit + }) } // WithInstrumentationName sets the instrumentation name. func WithInstrumentationName(name string) InstrumentOption { - return instrumentationNameOption(name) -} - -type instrumentationNameOption string - -func (i instrumentationNameOption) ApplyInstrument(config *InstrumentConfig) { - config.InstrumentationName = string(i) + return instrumentOptionFunc(func(cfg *InstrumentConfig) { + cfg.instrumentationName = name + }) } // MeterConfig contains options for Meters. type MeterConfig struct { - // InstrumentationVersion is the version of the library providing - // instrumentation. - InstrumentationVersion string + instrumentationVersion string +} + +// InstrumentationVersion is the version of the library providing instrumentation. +func (cfg MeterConfig) InstrumentationVersion() string { + return cfg.instrumentationVersion } // MeterOption is an interface for applying Meter options. type MeterOption interface { // ApplyMeter is used to set a MeterOption value of a MeterConfig. - ApplyMeter(*MeterConfig) + applyMeter(*MeterConfig) } // NewMeterConfig creates a new MeterConfig and applies @@ -100,29 +113,29 @@ type MeterOption interface { func NewMeterConfig(opts ...MeterOption) MeterConfig { var config MeterConfig for _, o := range opts { - o.ApplyMeter(&config) + o.applyMeter(&config) } return config } -// InstrumentationOption is an interface for applying instrumentation specific -// options. -type InstrumentationOption interface { +// InstrumentMeterOption are options that can be used as both an InstrumentOption +// and MeterOption +type InstrumentMeterOption interface { InstrumentOption MeterOption } // WithInstrumentationVersion sets the instrumentation version. -func WithInstrumentationVersion(version string) InstrumentationOption { +func WithInstrumentationVersion(version string) InstrumentMeterOption { return instrumentationVersionOption(version) } type instrumentationVersionOption string -func (i instrumentationVersionOption) ApplyMeter(config *MeterConfig) { - config.InstrumentationVersion = string(i) +func (i instrumentationVersionOption) applyMeter(config *MeterConfig) { + config.instrumentationVersion = string(i) } -func (i instrumentationVersionOption) ApplyInstrument(config *InstrumentConfig) { - config.InstrumentationVersion = string(i) +func (i instrumentationVersionOption) applyInstrument(config *InstrumentConfig) { + config.instrumentationVersion = string(i) } diff --git a/metric/metric.go b/metric/metric.go index a5d0dcd04f2..d8489aef07e 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -285,8 +285,8 @@ func (m Meter) newAsync( return NoopAsync{}, nil } desc := NewDescriptor(name, mkind, nkind, opts...) - desc.config.InstrumentationName = m.name - desc.config.InstrumentationVersion = m.version + desc.config.instrumentationName = m.name + desc.config.instrumentationVersion = m.version return m.impl.NewAsyncInstrument(desc, runner) } @@ -304,8 +304,8 @@ func (m Meter) newSync( return NoopSync{}, nil } desc := NewDescriptor(name, metricKind, numberKind, opts...) - desc.config.InstrumentationName = m.name - desc.config.InstrumentationVersion = m.version + desc.config.instrumentationName = m.name + desc.config.instrumentationVersion = m.version return m.impl.NewSyncInstrument(desc) } @@ -549,13 +549,13 @@ func (d Descriptor) InstrumentKind() InstrumentKind { // Description provides a human-readable description of the metric // instrument. func (d Descriptor) Description() string { - return d.config.Description + return d.config.Description() } // Unit describes the units of the metric instrument. Unitless // metrics return the empty string. func (d Descriptor) Unit() unit.Unit { - return d.config.Unit + return d.config.Unit() } // NumberKind returns whether this instrument is declared over int64, @@ -567,11 +567,11 @@ func (d Descriptor) NumberKind() number.Kind { // InstrumentationName returns the name of the library that provided // instrumentation for this instrument. func (d Descriptor) InstrumentationName() string { - return d.config.InstrumentationName + return d.config.InstrumentationName() } // InstrumentationVersion returns the version of the library that provided // instrumentation for this instrument. func (d Descriptor) InstrumentationVersion() string { - return d.config.InstrumentationVersion + return d.config.InstrumentationVersion() } diff --git a/metric/metric_sdkapi.go b/metric/metric_sdkapi.go index 94164f7b485..eb5ac4f52a6 100644 --- a/metric/metric_sdkapi.go +++ b/metric/metric_sdkapi.go @@ -90,6 +90,6 @@ func WrapMeterImpl(impl MeterImpl, instrumentationName string, opts ...MeterOpti return Meter{ impl: impl, name: instrumentationName, - version: NewMeterConfig(opts...).InstrumentationVersion, + version: NewMeterConfig(opts...).InstrumentationVersion(), } } diff --git a/metric/metric_test.go b/metric/metric_test.go index f01d7a1ebd2..899cc273714 100644 --- a/metric/metric_test.go +++ b/metric/metric_test.go @@ -148,25 +148,31 @@ func checkSyncBatches(ctx context.Context, t *testing.T, labels []attribute.KeyV func TestOptions(t *testing.T) { type testcase struct { - name string - opts []metric.InstrumentOption - desc string - unit unit.Unit + name string + opts []metric.InstrumentOption + desc string + unit unit.Unit + iName string + iVer string } testcases := []testcase{ { - name: "no opts", - opts: nil, - desc: "", - unit: "", + name: "no opts", + opts: nil, + desc: "", + unit: "", + iName: "", + iVer: "", }, { name: "description", opts: []metric.InstrumentOption{ metric.WithDescription("stuff"), }, - desc: "stuff", - unit: "", + desc: "stuff", + unit: "", + iName: "", + iVer: "", }, { name: "description override", @@ -174,34 +180,126 @@ func TestOptions(t *testing.T) { metric.WithDescription("stuff"), metric.WithDescription("things"), }, - desc: "things", - unit: "", + desc: "things", + unit: "", + iName: "", + iVer: "", }, { name: "unit", opts: []metric.InstrumentOption{ metric.WithUnit("s"), }, - desc: "", - unit: "s", + desc: "", + unit: "s", + iName: "", + iVer: "", }, + { + name: "description override", + opts: []metric.InstrumentOption{ + metric.WithDescription("stuff"), + metric.WithDescription("things"), + }, + desc: "things", + unit: "", + iName: "", + iVer: "", + }, + { + name: "unit", + opts: []metric.InstrumentOption{ + metric.WithUnit("s"), + }, + desc: "", + unit: "s", + iName: "", + iVer: "", + }, + { name: "unit override", opts: []metric.InstrumentOption{ metric.WithUnit("s"), metric.WithUnit("h"), }, - desc: "", - unit: "h", + desc: "", + unit: "h", + iName: "", + iVer: "", + }, + { + name: "name", + opts: []metric.InstrumentOption{ + metric.WithInstrumentationName("n"), + }, + desc: "", + unit: "", + iName: "n", + iVer: "", + }, + + { + name: "name override", + opts: []metric.InstrumentOption{ + metric.WithInstrumentationName("n"), + metric.WithInstrumentationName("o"), + }, + desc: "", + unit: "", + iName: "o", + iVer: "", + }, + { + name: "version", + opts: []metric.InstrumentOption{ + metric.WithInstrumentationVersion("v"), + }, + desc: "", + unit: "", + iName: "", + iVer: "v", + }, + + { + name: "version override", + opts: []metric.InstrumentOption{ + metric.WithInstrumentationVersion("v"), + metric.WithInstrumentationVersion("q"), + }, + desc: "", + unit: "", + iName: "", + iVer: "q", + }, + { + name: "all", + opts: []metric.InstrumentOption{ + metric.WithDescription("stuff"), + metric.WithUnit("s"), + metric.WithInstrumentationName("n"), + metric.WithInstrumentationVersion("v"), + }, + desc: "stuff", + unit: "s", + iName: "n", + iVer: "v", }, } for idx, tt := range testcases { t.Logf("Testing counter case %s (%d)", tt.name, idx) - if diff := cmp.Diff(metric.NewInstrumentConfig(tt.opts...), metric.InstrumentConfig{ - Description: tt.desc, - Unit: tt.unit, - }); diff != "" { - t.Errorf("Compare options: -got +want %s", diff) + cfg := metric.NewInstrumentConfig(tt.opts...) + if diff := cmp.Diff(cfg.Description(), tt.desc); diff != "" { + t.Errorf("Compare Description: -got +want %s", diff) + } + if diff := cmp.Diff(cfg.Unit(), tt.unit); diff != "" { + t.Errorf("Compare Unit: -got +want %s", diff) + } + if diff := cmp.Diff(cfg.InstrumentationName(), tt.iName); diff != "" { + t.Errorf("Compare InstrumentationNam: -got +want %s", diff) + } + if diff := cmp.Diff(cfg.InstrumentationVersion(), tt.iVer); diff != "" { + t.Errorf("Compare InstrumentationVersion: -got +want %s", diff) } } } diff --git a/oteltest/provider.go b/oteltest/provider.go index cf6aea17880..0b63c84497c 100644 --- a/oteltest/provider.go +++ b/oteltest/provider.go @@ -51,7 +51,7 @@ func (p *TracerProvider) Tracer(instName string, opts ...trace.TracerOption) tra inst := instrumentation{ Name: instName, - Version: conf.InstrumentationVersion, + Version: conf.InstrumentationVersion(), } p.tracersMu.Lock() defer p.tracersMu.Unlock() @@ -59,7 +59,7 @@ func (p *TracerProvider) Tracer(instName string, opts ...trace.TracerOption) tra if !ok { t = &Tracer{ Name: instName, - Version: conf.InstrumentationVersion, + Version: conf.InstrumentationVersion(), config: &p.config, } p.tracers[inst] = t diff --git a/oteltest/span.go b/oteltest/span.go index 2d7edd15c01..d243e9f8c68 100644 --- a/oteltest/span.go +++ b/oteltest/span.go @@ -49,7 +49,7 @@ type Span struct { // End ends s. If the Tracer that created s was configured with a // SpanRecorder, that recorder's OnEnd method is called as the final part of // this method. -func (s *Span) End(opts ...trace.SpanOption) { +func (s *Span) End(opts ...trace.SpanEndOption) { s.lock.Lock() defer s.lock.Unlock() @@ -57,9 +57,9 @@ func (s *Span) End(opts ...trace.SpanOption) { return } - c := trace.NewSpanConfig(opts...) + c := trace.NewSpanEndConfig(opts...) s.endTime = time.Now() - if endTime := c.Timestamp; !endTime.IsZero() { + if endTime := c.Timestamp(); !endTime.IsZero() { s.endTime = endTime } @@ -101,15 +101,15 @@ func (s *Span) AddEvent(name string, o ...trace.EventOption) { c := trace.NewEventConfig(o...) var attributes map[attribute.Key]attribute.Value - if l := len(c.Attributes); l > 0 { + if l := len(c.Attributes()); l > 0 { attributes = make(map[attribute.Key]attribute.Value, l) - for _, attr := range c.Attributes { + for _, attr := range c.Attributes() { attributes[attr.Key] = attr.Value } } s.events = append(s.events, Event{ - Timestamp: c.Timestamp, + Timestamp: c.Timestamp(), Name: name, Attributes: attributes, }) diff --git a/oteltest/tracer.go b/oteltest/tracer.go index 8b04f17f41b..b5e7367d0d1 100644 --- a/oteltest/tracer.go +++ b/oteltest/tracer.go @@ -36,10 +36,10 @@ type Tracer struct { // Start creates a span. If t is configured with a SpanRecorder its OnStart // method will be called after the created Span has been initialized. -func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) { - c := trace.NewSpanConfig(opts...) +func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { + c := trace.NewSpanStartConfig(opts...) startTime := time.Now() - if st := c.Timestamp; !st.IsZero() { + if st := c.Timestamp(); !st.IsZero() { startTime = st } @@ -48,10 +48,10 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio startTime: startTime, attributes: make(map[attribute.Key]attribute.Value), links: []trace.Link{}, - spanKind: c.SpanKind, + spanKind: c.SpanKind(), } - if c.NewRoot { + if c.NewRoot() { span.spanContext = trace.SpanContext{} } else { span.spanContext = t.config.SpanContextFunc(ctx) @@ -61,7 +61,7 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio } } - for _, link := range c.Links { + for _, link := range c.Links() { for i, sl := range span.links { if sl.SpanContext.SpanID() == link.SpanContext.SpanID() && sl.SpanContext.TraceID() == link.SpanContext.TraceID() && @@ -75,7 +75,7 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio } span.SetName(name) - span.SetAttributes(c.Attributes...) + span.SetAttributes(c.Attributes()...) if t.config.SpanRecorder != nil { t.config.SpanRecorder.OnStart(span) diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 083c0b9946a..bc4ba5db34f 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -116,7 +116,7 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T } il := instrumentation.Library{ Name: name, - Version: c.InstrumentationVersion, + Version: c.InstrumentationVersion(), } t, ok := p.namedTracer[il] if !ok { diff --git a/sdk/trace/span.go b/sdk/trace/span.go index ea61e9c6b34..f40cf1afec2 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -214,7 +214,7 @@ func (s *span) SetAttributes(attributes ...attribute.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) { +func (s *span) End(options ...trace.SpanEndOption) { // 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 { @@ -247,14 +247,14 @@ func (s *span) End(options ...trace.SpanOption) { s.executionTracerTaskEnd() } - config := trace.NewSpanConfig(options...) + config := trace.NewSpanEndConfig(options...) s.mu.Lock() // Setting endTime to non-zero marks the span as ended and not recording. - if config.Timestamp.IsZero() { + if config.Timestamp().IsZero() { s.endTime = et } else { - s.endTime = config.Timestamp + s.endTime = config.Timestamp() } s.mu.Unlock() @@ -305,19 +305,19 @@ func (s *span) addEvent(name string, o ...trace.EventOption) { c := trace.NewEventConfig(o...) // Discard over limited attributes + attributes := c.Attributes() var discarded int - if len(c.Attributes) > s.spanLimits.AttributePerEventCountLimit { - discarded = len(c.Attributes) - s.spanLimits.AttributePerEventCountLimit - c.Attributes = c.Attributes[:s.spanLimits.AttributePerEventCountLimit] + if len(attributes) > s.spanLimits.AttributePerEventCountLimit { + discarded = len(attributes) - s.spanLimits.AttributePerEventCountLimit + attributes = attributes[:s.spanLimits.AttributePerEventCountLimit] } - s.mu.Lock() defer s.mu.Unlock() s.events.add(Event{ Name: name, - Attributes: c.Attributes, + Attributes: attributes, DroppedAttributeCount: discarded, - Time: c.Timestamp, + Time: c.Timestamp(), }) } @@ -549,7 +549,7 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, o *trace.Sp // If told explicitly to make this a new root use a zero value SpanContext // as a parent which contains an invalid trace ID and is not remote. var psc trace.SpanContext - if !o.NewRoot { + if !o.NewRoot() { psc = trace.SpanContextFromContext(ctx) } @@ -575,9 +575,9 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, o *trace.Sp ParentContext: ctx, TraceID: tid, Name: name, - Kind: o.SpanKind, - Attributes: o.Attributes, - Links: o.Links, + Kind: o.SpanKind(), + Attributes: o.Attributes(), + Links: o.Links(), }) scc := trace.SpanContextConfig{ @@ -596,13 +596,13 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, o *trace.Sp return span } - startTime := o.Timestamp + startTime := o.Timestamp() if startTime.IsZero() { startTime = time.Now() } span.startTime = startTime - span.spanKind = trace.ValidateSpanKind(o.SpanKind) + span.spanKind = trace.ValidateSpanKind(o.SpanKind()) span.name = name span.parent = psc span.resource = provider.resource diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 46454e8f019..b18a9d77b62 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -813,7 +813,7 @@ func checkChild(t *testing.T, p trace.SpanContext, apiSpan trace.Span) error { // startSpan starts a span with a name "span0". See startNamedSpan for // details. -func startSpan(tp *TracerProvider, trName string, args ...trace.SpanOption) trace.Span { +func startSpan(tp *TracerProvider, trName string, args ...trace.SpanStartOption) trace.Span { return startNamedSpan(tp, trName, "span0", args...) } @@ -821,7 +821,7 @@ func startSpan(tp *TracerProvider, trName string, args ...trace.SpanOption) trac // 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 *TracerProvider, trName, name string, args ...trace.SpanOption) trace.Span { +func startNamedSpan(tp *TracerProvider, trName, name string, args ...trace.SpanStartOption) trace.Span { _, span := tp.Tracer(trName).Start( trace.ContextWithRemoteSpanContext(context.Background(), sc), name, @@ -834,7 +834,7 @@ func startNamedSpan(tp *TracerProvider, trName, name string, args ...trace.SpanO // passed name and with the passed context. The context is returned // along with the span so this parent can be used to create child // spans. -func startLocalSpan(tp *TracerProvider, ctx context.Context, trName, name string, args ...trace.SpanOption) (context.Context, trace.Span) { +func startLocalSpan(tp *TracerProvider, ctx context.Context, trName, name string, args ...trace.SpanStartOption) (context.Context, trace.Span) { ctx, span := tp.Tracer(trName).Start( ctx, name, diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index b7869eddd2e..0c692fc815a 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -36,8 +36,8 @@ var _ trace.Tracer = &tracer{} // 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 ...trace.SpanOption) (context.Context, trace.Span) { - config := trace.NewSpanConfig(options...) +func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanStartOption) (context.Context, trace.Span) { + config := trace.NewSpanStartConfig(options...) // For local spans created by this SDK, track child span count. if p := trace.SpanFromContext(ctx); p != nil { @@ -47,10 +47,10 @@ func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanO } span := startSpanInternal(ctx, tr, name, config) - for _, l := range config.Links { + for _, l := range config.Links() { span.addLink(l) } - span.SetAttributes(config.Attributes...) + span.SetAttributes(config.Attributes()...) span.tracer = tr diff --git a/trace/config.go b/trace/config.go index ea30ee35f15..4485fb9be07 100644 --- a/trace/config.go +++ b/trace/config.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package trace +package trace // import "go.opentelemetry.io/otel/trace" import ( "time" @@ -22,109 +22,171 @@ import ( // TracerConfig is a group of options for a Tracer. type TracerConfig struct { - // InstrumentationVersion is the version of the library providing - // instrumentation. - InstrumentationVersion string + instrumentationVersion string +} + +// InstrumentationVersion returns the version of the library providing instrumentation. +func (t *TracerConfig) InstrumentationVersion() string { + return t.instrumentationVersion } // NewTracerConfig applies all the options to a returned TracerConfig. func NewTracerConfig(options ...TracerOption) *TracerConfig { config := new(TracerConfig) for _, option := range options { - option.ApplyTracer(config) + option.apply(config) } return config } // TracerOption applies an option to a TracerConfig. type TracerOption interface { - ApplyTracer(*TracerConfig) + apply(*TracerConfig) +} + +type tracerOptionFunc func(*TracerConfig) - // A private method to prevent users implementing the - // interface and so future additions to it will not - // violate compatibility. - private() +func (fn tracerOptionFunc) apply(cfg *TracerConfig) { + fn(cfg) } // SpanConfig is a group of options for a Span. type SpanConfig struct { - // Attributes describe the associated qualities of a Span. - Attributes []attribute.KeyValue - // Timestamp is a time in a Span life-cycle. - Timestamp time.Time - // Links are the associations a Span has with other Spans. - Links []Link - // 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 role a Span has in a trace. - SpanKind SpanKind -} - -// NewSpanConfig applies all the options to a returned SpanConfig. + attributes []attribute.KeyValue + timestamp time.Time + links []Link + newRoot bool + spanKind SpanKind +} + +// Attributes describe the associated qualities of a Span. +func (cfg *SpanConfig) Attributes() []attribute.KeyValue { + return cfg.attributes +} + +// Timestamp is a time in a Span life-cycle. +func (cfg *SpanConfig) Timestamp() time.Time { + return cfg.timestamp +} + +// Links are the associations a Span has with other Spans. +func (cfg *SpanConfig) Links() []Link { + return cfg.links +} + +// 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. +func (cfg *SpanConfig) NewRoot() bool { + return cfg.newRoot +} + +// SpanKind is the role a Span has in a trace. +func (cfg *SpanConfig) SpanKind() SpanKind { + return cfg.spanKind +} + +// NewSpanStartConfig applies all the options to a returned SpanConfig. // No validation is performed on the returned SpanConfig (e.g. no uniqueness // checking or bounding of data), it is left to the SDK to perform this // action. -func NewSpanConfig(options ...SpanOption) *SpanConfig { +func NewSpanStartConfig(options ...SpanStartOption) *SpanConfig { c := new(SpanConfig) for _, option := range options { - option.ApplySpan(c) + option.applySpanStart(c) } return c } -// SpanOption applies an option to a SpanConfig. -type SpanOption interface { - ApplySpan(*SpanConfig) +// NewSpanEndConfig applies all the options to a returned SpanConfig. +// No validation is performed on the returned SpanConfig (e.g. no uniqueness +// checking or bounding of data), it is left to the SDK to perform this +// action. +func NewSpanEndConfig(options ...SpanEndOption) *SpanConfig { + c := new(SpanConfig) + for _, option := range options { + option.applySpanEnd(c) + } + return c +} - // A private method to prevent users implementing the - // interface and so future additions to it will not - // violate compatibility. - private() +// SpanStartOption applies an option to a SpanConfig. These options are applicable +// only when the span is created +type SpanStartOption interface { + applySpanStart(*SpanConfig) +} + +type spanOptionFunc func(*SpanConfig) + +func (fn spanOptionFunc) applySpanStart(cfg *SpanConfig) { + fn(cfg) +} + +// SpanEndOptions applies an option to a SpanConfig. These options are applicable +// only when the span is ended. +type SpanEndOption interface { + applySpanEnd(*SpanConfig) +} + +// EventConfig is a group of options for an Event. +type EventConfig struct { + attributes []attribute.KeyValue + timestamp time.Time +} + +// Attributes describe the associated qualities of an Event. +func (cfg *EventConfig) Attributes() []attribute.KeyValue { + return cfg.attributes +} + +// Timestamp is a time in an Event life-cycle. +func (cfg *EventConfig) Timestamp() time.Time { + return cfg.timestamp } // NewEventConfig applies all the EventOptions to a returned SpanConfig. If no // timestamp option is passed, the returned SpanConfig will have a Timestamp // set to the call time, otherwise no validation is performed on the returned // SpanConfig. -func NewEventConfig(options ...EventOption) *SpanConfig { - c := new(SpanConfig) +func NewEventConfig(options ...EventOption) *EventConfig { + c := new(EventConfig) for _, option := range options { - option.ApplyEvent(c) + option.applyEvent(c) } - if c.Timestamp.IsZero() { - c.Timestamp = time.Now() + if c.timestamp.IsZero() { + c.timestamp = time.Now() } return c } -// EventOption applies span event options to a SpanConfig. +// EventOption applies span event options to an EventConfig. type EventOption interface { - ApplyEvent(*SpanConfig) + applyEvent(*EventConfig) +} - // A private method to prevent users implementing the - // interface and so future additions to it will not - // violate compatibility. - private() +// SpanOption are options that can be used at both the beginning and end of a span. +type SpanOption interface { + SpanStartOption + SpanEndOption } -// LifeCycleOption applies span life-cycle options to a SpanConfig. These -// options set values releated to events in a spans life-cycle like starting, -// ending, experiencing an error and other user defined notable events. -type LifeCycleOption interface { - SpanOption +// SpanStartEventOption are options that can be used at the start of a span, or with an event. +type SpanStartEventOption interface { + SpanStartOption EventOption } -type attributeSpanOption []attribute.KeyValue +type attributeOption []attribute.KeyValue -func (o attributeSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) } -func (o attributeSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) } -func (attributeSpanOption) private() {} -func (o attributeSpanOption) apply(c *SpanConfig) { - c.Attributes = append(c.Attributes, []attribute.KeyValue(o)...) +func (o attributeOption) applySpan(c *SpanConfig) { + c.attributes = append(c.attributes, []attribute.KeyValue(o)...) } +func (o attributeOption) applySpanStart(c *SpanConfig) { o.applySpan(c) } +func (o attributeOption) applyEvent(c *EventConfig) { + c.attributes = append(c.attributes, []attribute.KeyValue(o)...) +} + +var _ SpanStartEventOption = attributeOption{} // WithAttributes adds the attributes related to a span life-cycle event. // These attributes are used to describe the work a Span represents when this @@ -135,71 +197,58 @@ func (o attributeSpanOption) apply(c *SpanConfig) { // If multiple of these options are passed the attributes of each successive // option will extend the attributes instead of overwriting. There is no // guarantee of uniqueness in the resulting attributes. -func WithAttributes(attributes ...attribute.KeyValue) LifeCycleOption { - return attributeSpanOption(attributes) +func WithAttributes(attributes ...attribute.KeyValue) SpanStartEventOption { + return attributeOption(attributes) } -type timestampSpanOption time.Time +// SpanEventOption are options that can be used with an event or a span. +type SpanEventOption interface { + SpanOption + EventOption +} -func (o timestampSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) } -func (o timestampSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) } -func (timestampSpanOption) private() {} -func (o timestampSpanOption) apply(c *SpanConfig) { c.Timestamp = time.Time(o) } +type timestampOption time.Time -// WithTimestamp sets the time of a Span life-cycle moment (e.g. started, -// stopped, errored). -func WithTimestamp(t time.Time) LifeCycleOption { - return timestampSpanOption(t) -} +func (o timestampOption) applySpan(c *SpanConfig) { c.timestamp = time.Time(o) } +func (o timestampOption) applySpanStart(c *SpanConfig) { o.applySpan(c) } +func (o timestampOption) applySpanEnd(c *SpanConfig) { o.applySpan(c) } +func (o timestampOption) applyEvent(c *EventConfig) { c.timestamp = time.Time(o) } -type linksSpanOption []Link +var _ SpanEventOption = timestampOption{} -func (o linksSpanOption) ApplySpan(c *SpanConfig) { c.Links = append(c.Links, []Link(o)...) } -func (linksSpanOption) private() {} +// WithTimestamp sets the time of a Span or Event life-cycle moment (e.g. +// started, stopped, errored). +func WithTimestamp(t time.Time) SpanEventOption { + return timestampOption(t) +} // WithLinks adds links to a Span. The links are added to the existing Span // links, i.e. this does not overwrite. -func WithLinks(links ...Link) SpanOption { - return linksSpanOption(links) +func WithLinks(links ...Link) SpanStartOption { + return spanOptionFunc(func(cfg *SpanConfig) { + cfg.links = append(cfg.links, links...) + }) } -type newRootSpanOption bool - -func (o newRootSpanOption) ApplySpan(c *SpanConfig) { c.NewRoot = bool(o) } -func (newRootSpanOption) private() {} - // 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) +func WithNewRoot() SpanStartOption { + return spanOptionFunc(func(cfg *SpanConfig) { + cfg.newRoot = true + }) } -type spanKindSpanOption SpanKind - -func (o spanKindSpanOption) ApplySpan(c *SpanConfig) { c.SpanKind = SpanKind(o) } -func (o spanKindSpanOption) private() {} - // WithSpanKind sets the SpanKind of a Span. -func WithSpanKind(kind SpanKind) SpanOption { - return spanKindSpanOption(kind) -} - -// InstrumentationOption is an interface for applying instrumentation specific -// options. -type InstrumentationOption interface { - TracerOption +func WithSpanKind(kind SpanKind) SpanStartOption { + return spanOptionFunc(func(cfg *SpanConfig) { + cfg.spanKind = kind + }) } // WithInstrumentationVersion sets the instrumentation version. -func WithInstrumentationVersion(version string) InstrumentationOption { - return instrumentationVersionOption(version) +func WithInstrumentationVersion(version string) TracerOption { + return tracerOptionFunc(func(cfg *TracerConfig) { + cfg.instrumentationVersion = version + }) } - -type instrumentationVersionOption string - -func (i instrumentationVersionOption) ApplyTracer(config *TracerConfig) { - config.InstrumentationVersion = string(i) -} - -func (instrumentationVersionOption) private() {} diff --git a/trace/config_test.go b/trace/config_test.go index 9375191c40a..074aa303ebb 100644 --- a/trace/config_test.go +++ b/trace/config_test.go @@ -41,119 +41,119 @@ func TestNewSpanConfig(t *testing.T) { } tests := []struct { - options []SpanOption + options []SpanStartOption expected *SpanConfig }{ { // No non-zero-values should be set. - []SpanOption{}, + []SpanStartOption{}, new(SpanConfig), }, { - []SpanOption{ + []SpanStartOption{ WithAttributes(k1v1), }, &SpanConfig{ - Attributes: []attribute.KeyValue{k1v1}, + attributes: []attribute.KeyValue{k1v1}, }, }, { // Multiple calls should append not overwrite. - []SpanOption{ + []SpanStartOption{ WithAttributes(k1v1), WithAttributes(k1v2), WithAttributes(k2v2), }, &SpanConfig{ // No uniqueness is guaranteed by the API. - Attributes: []attribute.KeyValue{k1v1, k1v2, k2v2}, + attributes: []attribute.KeyValue{k1v1, k1v2, k2v2}, }, }, { - []SpanOption{ + []SpanStartOption{ WithAttributes(k1v1, k1v2, k2v2), }, &SpanConfig{ // No uniqueness is guaranteed by the API. - Attributes: []attribute.KeyValue{k1v1, k1v2, k2v2}, + attributes: []attribute.KeyValue{k1v1, k1v2, k2v2}, }, }, { - []SpanOption{ + []SpanStartOption{ WithTimestamp(timestamp0), }, &SpanConfig{ - Timestamp: timestamp0, + timestamp: timestamp0, }, }, { - []SpanOption{ + []SpanStartOption{ // Multiple calls overwrites with last-one-wins. WithTimestamp(timestamp0), WithTimestamp(timestamp1), }, &SpanConfig{ - Timestamp: timestamp1, + timestamp: timestamp1, }, }, { - []SpanOption{ + []SpanStartOption{ WithLinks(link1), }, &SpanConfig{ - Links: []Link{link1}, + links: []Link{link1}, }, }, { - []SpanOption{ + []SpanStartOption{ // Multiple calls should append not overwrite. WithLinks(link1), WithLinks(link1, link2), }, &SpanConfig{ // No uniqueness is guaranteed by the API. - Links: []Link{link1, link1, link2}, + links: []Link{link1, link1, link2}, }, }, { - []SpanOption{ + []SpanStartOption{ WithNewRoot(), }, &SpanConfig{ - NewRoot: true, + newRoot: true, }, }, { - []SpanOption{ + []SpanStartOption{ // Multiple calls should not change NewRoot state. WithNewRoot(), WithNewRoot(), }, &SpanConfig{ - NewRoot: true, + newRoot: true, }, }, { - []SpanOption{ + []SpanStartOption{ WithSpanKind(SpanKindConsumer), }, &SpanConfig{ - SpanKind: SpanKindConsumer, + spanKind: SpanKindConsumer, }, }, { - []SpanOption{ + []SpanStartOption{ // Multiple calls overwrites with last-one-wins. WithSpanKind(SpanKindClient), WithSpanKind(SpanKindConsumer), }, &SpanConfig{ - SpanKind: SpanKindConsumer, + spanKind: SpanKindConsumer, }, }, { // Everything should work together. - []SpanOption{ + []SpanStartOption{ WithAttributes(k1v1), WithTimestamp(timestamp0), WithLinks(link1, link2), @@ -161,16 +161,16 @@ func TestNewSpanConfig(t *testing.T) { WithSpanKind(SpanKindConsumer), }, &SpanConfig{ - Attributes: []attribute.KeyValue{k1v1}, - Timestamp: timestamp0, - Links: []Link{link1, link2}, - NewRoot: true, - SpanKind: SpanKindConsumer, + attributes: []attribute.KeyValue{k1v1}, + timestamp: timestamp0, + links: []Link{link1, link2}, + newRoot: true, + spanKind: SpanKindConsumer, }, }, } for _, test := range tests { - assert.Equal(t, test.expected, NewSpanConfig(test.options...)) + assert.Equal(t, test.expected, NewSpanStartConfig(test.options...)) } } @@ -191,7 +191,7 @@ func TestTracerConfig(t *testing.T) { WithInstrumentationVersion(v1), }, &TracerConfig{ - InstrumentationVersion: v1, + instrumentationVersion: v1, }, }, { @@ -201,7 +201,7 @@ func TestTracerConfig(t *testing.T) { WithInstrumentationVersion(v2), }, &TracerConfig{ - InstrumentationVersion: v2, + instrumentationVersion: v2, }, }, } diff --git a/trace/noop.go b/trace/noop.go index d3691d1c62b..5d5bd7af68e 100644 --- a/trace/noop.go +++ b/trace/noop.go @@ -43,7 +43,7 @@ type noopTracer struct{} var _ Tracer = noopTracer{} // Start starts a noop span. -func (t noopTracer) Start(ctx context.Context, name string, _ ...SpanOption) (context.Context, Span) { +func (t noopTracer) Start(ctx context.Context, name string, _ ...SpanStartOption) (context.Context, Span) { span := noopSpan{} return ContextWithSpan(ctx, span), span } @@ -69,7 +69,7 @@ func (noopSpan) SetError(bool) {} func (noopSpan) SetAttributes(...attribute.KeyValue) {} // End does nothing. -func (noopSpan) End(...SpanOption) {} +func (noopSpan) End(...SpanEndOption) {} // RecordError does nothing. func (noopSpan) RecordError(error, ...EventOption) {} diff --git a/trace/trace.go b/trace/trace.go index 62dc4d2c3ee..cd96a756561 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -342,7 +342,7 @@ type Span interface { // delivered through the rest of the telemetry pipeline after this method // is called. Therefore, updates to the Span are not allowed after this // method has been called. - End(options ...SpanOption) + End(options ...SpanEndOption) // AddEvent adds an event with the provided name and options. AddEvent(name string, options ...EventOption) @@ -488,7 +488,7 @@ type Tracer interface { // // Any Span that is created MUST also be ended. This is the responsibility of the user. // Implementations of this API may leak memory or other resources if Spans are not ended. - Start(ctx context.Context, spanName string, opts ...SpanOption) (context.Context, Span) + Start(ctx context.Context, spanName string, opts ...SpanStartOption) (context.Context, Span) } // TracerProvider provides access to instrumentation Tracers.