Skip to content

Commit

Permalink
Update trace API config creation functions (#2212)
Browse files Browse the repository at this point in the history
* 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 <a9@aneurysm9.com>
  • Loading branch information
MrAlias and Aneurysm9 authored Sep 1, 2021
1 parent 361a209 commit 1f527a5
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 8 additions & 4 deletions bridge/opencensus/internal/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down
4 changes: 2 additions & 2 deletions bridge/opentracing/internal/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand All @@ -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()),
}
Expand Down
3 changes: 2 additions & 1 deletion internal/global/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion sdk/trace/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
24 changes: 12 additions & 12 deletions trace/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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()
Expand Down
46 changes: 23 additions & 23 deletions trace/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
},
Expand All @@ -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},
},
Expand All @@ -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},
},
Expand All @@ -82,7 +82,7 @@ func TestNewSpanConfig(t *testing.T) {
[]SpanStartOption{
WithTimestamp(timestamp0),
},
&SpanConfig{
SpanConfig{
timestamp: timestamp0,
},
},
Expand All @@ -92,15 +92,15 @@ func TestNewSpanConfig(t *testing.T) {
WithTimestamp(timestamp0),
WithTimestamp(timestamp1),
},
&SpanConfig{
SpanConfig{
timestamp: timestamp1,
},
},
{
[]SpanStartOption{
WithLinks(link1),
},
&SpanConfig{
SpanConfig{
links: []Link{link1},
},
},
Expand All @@ -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},
},
Expand All @@ -119,7 +119,7 @@ func TestNewSpanConfig(t *testing.T) {
[]SpanStartOption{
WithNewRoot(),
},
&SpanConfig{
SpanConfig{
newRoot: true,
},
},
Expand All @@ -129,15 +129,15 @@ func TestNewSpanConfig(t *testing.T) {
WithNewRoot(),
WithNewRoot(),
},
&SpanConfig{
SpanConfig{
newRoot: true,
},
},
{
[]SpanStartOption{
WithSpanKind(SpanKindConsumer),
},
&SpanConfig{
SpanConfig{
spanKind: SpanKindConsumer,
},
},
Expand All @@ -147,7 +147,7 @@ func TestNewSpanConfig(t *testing.T) {
WithSpanKind(SpanKindClient),
WithSpanKind(SpanKindConsumer),
},
&SpanConfig{
SpanConfig{
spanKind: SpanKindConsumer,
},
},
Expand All @@ -160,7 +160,7 @@ func TestNewSpanConfig(t *testing.T) {
WithNewRoot(),
WithSpanKind(SpanKindConsumer),
},
&SpanConfig{
SpanConfig{
attributes: []attribute.KeyValue{k1v1},
timestamp: timestamp0,
links: []Link{link1, link2},
Expand All @@ -179,25 +179,25 @@ func TestEndSpanConfig(t *testing.T) {

tests := []struct {
options []SpanEndOption
expected *SpanConfig
expected SpanConfig
}{
{
[]SpanEndOption{},
new(SpanConfig),
SpanConfig{},
},
{
[]SpanEndOption{
WithStackTrace(true),
},
&SpanConfig{
SpanConfig{
stackTrace: true,
},
},
{
[]SpanEndOption{
WithTimestamp(timestamp),
},
&SpanConfig{
SpanConfig{
timestamp: timestamp,
},
},
Expand All @@ -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,
},
},
Expand All @@ -234,15 +234,15 @@ func TestTracerConfig(t *testing.T) {
WithInstrumentationVersion(v1),
WithInstrumentationVersion(v2),
},
&TracerConfig{
TracerConfig{
instrumentationVersion: v2,
},
},
{
[]TracerOption{
WithSchemaURL(schemaURL),
},
&TracerConfig{
TracerConfig{
schemaURL: schemaURL,
},
},
Expand Down

0 comments on commit 1f527a5

Please sign in to comment.