From 1f527a52ab7264ed6bea0ea7f4ebd2573c796a60 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 1 Sep 2021 12:44:13 -0700 Subject: [PATCH] Update trace API config creation functions (#2212) * Update trace API config creation funcs Follow our style guide and return the config struct instead of pointers. * Update changelog with changes Co-authored-by: Anthony Mirabella --- CHANGELOG.md | 1 + bridge/opencensus/internal/span_test.go | 12 ++++--- bridge/opentracing/internal/mock.go | 4 +-- internal/global/trace.go | 3 +- sdk/trace/tracer.go | 2 +- trace/config.go | 24 ++++++------- trace/config_test.go | 46 ++++++++++++------------- 7 files changed, 49 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a6c4748c4a..af7e2cae67a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Metric SDK/API implementation type `InstrumentKind` moves into `sdkapi` sub-package. (#2091) - The Metrics SDK export record no longer contains a Resource pointer, the SDK `"go.opentelemetry.io/otel/sdk/trace/export/metric".Exporter.Export()` function for push-based exporters now takes a single Resource argument, pull-based exporters use `"go.opentelemetry.io/otel/sdk/metric/controller/basic".Controller.Resource()`. (#2120) - The JSON output of the `go.opentelemetry.io/otel/exporters/stdout/stdouttrace` is harmonized now such that the output is "plain" JSON objects after each other of the form `{ ... } { ... } { ... }`. Earlier the JSON objects describing a span were wrapped in a slice for each `Exporter.ExportSpans` call, like `[ { ... } ][ { ... } { ... } ]`. Outputting JSON object directly after each other is consistent with JSON loggers, and a bit easier to parse and read. (#2196) +- Update the `NewTracerConfig`, `NewSpanStartConfig`, `NewSpanEndConfig`, and `NewEventConfig` function in the `go.opentelemetry.io/otel/trace` package to return their respective configurations as structs instead of pointers to the struct. (#2212) ### Deprecated diff --git a/bridge/opencensus/internal/span_test.go b/bridge/opencensus/internal/span_test.go index b0771aad63c..7aa023e71c2 100644 --- a/bridge/opencensus/internal/span_test.go +++ b/bridge/opencensus/internal/span_test.go @@ -151,7 +151,8 @@ func TestSpanAnnotate(t *testing.T) { t.Error("span.Annotate did not set event name") } - got := trace.NewEventConfig(s.eOpts...).Attributes() + config := trace.NewEventConfig(s.eOpts...) + got := config.Attributes() if len(want) != len(got) || want[0] != got[0] { t.Error("span.Annotate did not set event options") } @@ -174,7 +175,8 @@ func TestSpanAnnotatef(t *testing.T) { t.Error("span.Annotatef did not set event name") } - got := trace.NewEventConfig(s.eOpts...).Attributes() + config := trace.NewEventConfig(s.eOpts...) + got := config.Attributes() if len(want) != len(got) || want[0] != got[0] { t.Error("span.Annotatef did not set event options") } @@ -192,7 +194,8 @@ func TestSpanAddMessageSendEvent(t *testing.T) { t.Error("span.AddMessageSendEvent did not set event name") } - got := trace.NewEventConfig(s.eOpts...).Attributes() + config := trace.NewEventConfig(s.eOpts...) + got := config.Attributes() if len(got) != 2 { t.Fatalf("span.AddMessageSendEvent set %d attributes, want 2", len(got)) } @@ -220,7 +223,8 @@ func TestSpanAddMessageReceiveEvent(t *testing.T) { t.Error("span.AddMessageReceiveEvent did not set event name") } - got := trace.NewEventConfig(s.eOpts...).Attributes() + config := trace.NewEventConfig(s.eOpts...) + got := config.Attributes() if len(got) != 2 { t.Fatalf("span.AddMessageReceiveEvent set %d attributes, want 2", len(got)) } diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index 57a1651ec30..ecb6320762c 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -75,7 +75,7 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanS startTime = time.Now() } spanContext := trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: t.getTraceID(ctx, config), + TraceID: t.getTraceID(ctx, &config), SpanID: t.getSpanID(), TraceFlags: 0, }) @@ -86,7 +86,7 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanS Attributes: config.Attributes(), StartTime: startTime, EndTime: time.Time{}, - ParentSpanID: t.getParentSpanID(ctx, config), + ParentSpanID: t.getParentSpanID(ctx, &config), Events: nil, SpanKind: trace.ValidateSpanKind(config.SpanKind()), } diff --git a/internal/global/trace.go b/internal/global/trace.go index 24fd2a6e4cd..11c7c556875 100644 --- a/internal/global/trace.go +++ b/internal/global/trace.go @@ -90,9 +90,10 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T // At this moment it is guaranteed that no sdk is installed, save the tracer in the tracers map. + c := trace.NewTracerConfig(opts...) key := il{ name: name, - version: trace.NewTracerConfig(opts...).InstrumentationVersion(), + version: c.InstrumentationVersion(), } if p.tracers == nil { diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 0c692fc815a..621ca8258e7 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -46,7 +46,7 @@ func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanS } } - span := startSpanInternal(ctx, tr, name, config) + span := startSpanInternal(ctx, tr, name, &config) for _, l := range config.Links() { span.addLink(l) } diff --git a/trace/config.go b/trace/config.go index 3bfb2fff64c..b1b74cb722d 100644 --- a/trace/config.go +++ b/trace/config.go @@ -38,10 +38,10 @@ func (t *TracerConfig) SchemaURL() string { } // NewTracerConfig applies all the options to a returned TracerConfig. -func NewTracerConfig(options ...TracerOption) *TracerConfig { - config := new(TracerConfig) +func NewTracerConfig(options ...TracerOption) TracerConfig { + var config TracerConfig for _, option := range options { - option.apply(config) + option.apply(&config) } return config } @@ -103,10 +103,10 @@ func (cfg *SpanConfig) SpanKind() SpanKind { // 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 NewSpanStartConfig(options ...SpanStartOption) *SpanConfig { - c := new(SpanConfig) +func NewSpanStartConfig(options ...SpanStartOption) SpanConfig { + var c SpanConfig for _, option := range options { - option.applySpanStart(c) + option.applySpanStart(&c) } return c } @@ -115,10 +115,10 @@ func NewSpanStartConfig(options ...SpanStartOption) *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) +func NewSpanEndConfig(options ...SpanEndOption) SpanConfig { + var c SpanConfig for _, option := range options { - option.applySpanEnd(c) + option.applySpanEnd(&c) } return c } @@ -167,10 +167,10 @@ func (cfg *EventConfig) StackTrace() bool { // timestamp option is passed, the returned EventConfig will have a Timestamp // set to the call time, otherwise no validation is performed on the returned // EventConfig. -func NewEventConfig(options ...EventOption) *EventConfig { - c := new(EventConfig) +func NewEventConfig(options ...EventOption) EventConfig { + var c EventConfig for _, option := range options { - option.applyEvent(c) + option.applyEvent(&c) } if c.timestamp.IsZero() { c.timestamp = time.Now() diff --git a/trace/config_test.go b/trace/config_test.go index 2b4ab965ed3..877bd024eeb 100644 --- a/trace/config_test.go +++ b/trace/config_test.go @@ -42,18 +42,18 @@ func TestNewSpanConfig(t *testing.T) { tests := []struct { options []SpanStartOption - expected *SpanConfig + expected SpanConfig }{ { // No non-zero-values should be set. []SpanStartOption{}, - new(SpanConfig), + SpanConfig{}, }, { []SpanStartOption{ WithAttributes(k1v1), }, - &SpanConfig{ + SpanConfig{ attributes: []attribute.KeyValue{k1v1}, }, }, @@ -64,7 +64,7 @@ func TestNewSpanConfig(t *testing.T) { WithAttributes(k1v2), WithAttributes(k2v2), }, - &SpanConfig{ + SpanConfig{ // No uniqueness is guaranteed by the API. attributes: []attribute.KeyValue{k1v1, k1v2, k2v2}, }, @@ -73,7 +73,7 @@ func TestNewSpanConfig(t *testing.T) { []SpanStartOption{ WithAttributes(k1v1, k1v2, k2v2), }, - &SpanConfig{ + SpanConfig{ // No uniqueness is guaranteed by the API. attributes: []attribute.KeyValue{k1v1, k1v2, k2v2}, }, @@ -82,7 +82,7 @@ func TestNewSpanConfig(t *testing.T) { []SpanStartOption{ WithTimestamp(timestamp0), }, - &SpanConfig{ + SpanConfig{ timestamp: timestamp0, }, }, @@ -92,7 +92,7 @@ func TestNewSpanConfig(t *testing.T) { WithTimestamp(timestamp0), WithTimestamp(timestamp1), }, - &SpanConfig{ + SpanConfig{ timestamp: timestamp1, }, }, @@ -100,7 +100,7 @@ func TestNewSpanConfig(t *testing.T) { []SpanStartOption{ WithLinks(link1), }, - &SpanConfig{ + SpanConfig{ links: []Link{link1}, }, }, @@ -110,7 +110,7 @@ func TestNewSpanConfig(t *testing.T) { WithLinks(link1), WithLinks(link1, link2), }, - &SpanConfig{ + SpanConfig{ // No uniqueness is guaranteed by the API. links: []Link{link1, link1, link2}, }, @@ -119,7 +119,7 @@ func TestNewSpanConfig(t *testing.T) { []SpanStartOption{ WithNewRoot(), }, - &SpanConfig{ + SpanConfig{ newRoot: true, }, }, @@ -129,7 +129,7 @@ func TestNewSpanConfig(t *testing.T) { WithNewRoot(), WithNewRoot(), }, - &SpanConfig{ + SpanConfig{ newRoot: true, }, }, @@ -137,7 +137,7 @@ func TestNewSpanConfig(t *testing.T) { []SpanStartOption{ WithSpanKind(SpanKindConsumer), }, - &SpanConfig{ + SpanConfig{ spanKind: SpanKindConsumer, }, }, @@ -147,7 +147,7 @@ func TestNewSpanConfig(t *testing.T) { WithSpanKind(SpanKindClient), WithSpanKind(SpanKindConsumer), }, - &SpanConfig{ + SpanConfig{ spanKind: SpanKindConsumer, }, }, @@ -160,7 +160,7 @@ func TestNewSpanConfig(t *testing.T) { WithNewRoot(), WithSpanKind(SpanKindConsumer), }, - &SpanConfig{ + SpanConfig{ attributes: []attribute.KeyValue{k1v1}, timestamp: timestamp0, links: []Link{link1, link2}, @@ -179,17 +179,17 @@ func TestEndSpanConfig(t *testing.T) { tests := []struct { options []SpanEndOption - expected *SpanConfig + expected SpanConfig }{ { []SpanEndOption{}, - new(SpanConfig), + SpanConfig{}, }, { []SpanEndOption{ WithStackTrace(true), }, - &SpanConfig{ + SpanConfig{ stackTrace: true, }, }, @@ -197,7 +197,7 @@ func TestEndSpanConfig(t *testing.T) { []SpanEndOption{ WithTimestamp(timestamp), }, - &SpanConfig{ + SpanConfig{ timestamp: timestamp, }, }, @@ -213,18 +213,18 @@ func TestTracerConfig(t *testing.T) { schemaURL := "https://opentelemetry.io/schemas/1.2.0" tests := []struct { options []TracerOption - expected *TracerConfig + expected TracerConfig }{ { // No non-zero-values should be set. []TracerOption{}, - new(TracerConfig), + TracerConfig{}, }, { []TracerOption{ WithInstrumentationVersion(v1), }, - &TracerConfig{ + TracerConfig{ instrumentationVersion: v1, }, }, @@ -234,7 +234,7 @@ func TestTracerConfig(t *testing.T) { WithInstrumentationVersion(v1), WithInstrumentationVersion(v2), }, - &TracerConfig{ + TracerConfig{ instrumentationVersion: v2, }, }, @@ -242,7 +242,7 @@ func TestTracerConfig(t *testing.T) { []TracerOption{ WithSchemaURL(schemaURL), }, - &TracerConfig{ + TracerConfig{ schemaURL: schemaURL, }, },