From 8bbd971ae5374af1ce68280803673641621e74c6 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 11:15:21 -0700 Subject: [PATCH 01/14] Remove TODO from ReadOnlySpan interface --- sdk/trace/span.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index b7f5296cddf..f45023e4671 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -34,8 +34,6 @@ import ( // ReadOnlySpan allows reading information from the data structure underlying a // trace.Span. It is used in places where reading information from a span is // necessary but changing the span isn't necessary or allowed. -// TODO: Should we make the methods unexported? The purpose of this interface -// is controlling access to `span` fields, not having multiple implementations. type ReadOnlySpan interface { Name() string SpanContext() trace.SpanContext From 268d78a7f6aaa6652e23921a143b15e74d13dbaa Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 11:21:18 -0700 Subject: [PATCH 02/14] Remove the Tracer method from the ReadOnlySpan This is not required by the specification nor the use of this interface. --- sdk/trace/span.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index f45023e4671..e29d6c8595b 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -46,7 +46,6 @@ type ReadOnlySpan interface { Events() []Event StatusCode() codes.Code StatusMessage() string - Tracer() trace.Tracer IsRecording() bool InstrumentationLibrary() instrumentation.Library Resource() *resource.Resource From 72fedf7676ffd92307bbd9f4bf195f5f3feae010 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 11:59:22 -0700 Subject: [PATCH 03/14] Add Status type to SDK Use this type to encapsulate the Span status similar to the Event type encapsulating a Span event --- exporters/otlp/internal/otlptest/data.go | 3 +- exporters/otlp/internal/transform/span.go | 2 +- .../otlp/internal/transform/span_test.go | 8 +- exporters/otlp/otlp_span_test.go | 32 +++++--- exporters/stdout/trace_test.go | 13 ++-- exporters/trace/jaeger/jaeger.go | 10 +-- exporters/trace/jaeger/jaeger_test.go | 34 +++++---- exporters/trace/zipkin/model.go | 8 +- exporters/trace/zipkin/model_test.go | 74 ++++++++++++------- exporters/trace/zipkin/zipkin_test.go | 16 ++-- sdk/trace/span.go | 57 +++++++------- sdk/trace/trace_test.go | 48 ++++++------ trace/trace.go | 6 +- 13 files changed, 177 insertions(+), 134 deletions(-) diff --git a/exporters/otlp/internal/otlptest/data.go b/exporters/otlp/internal/otlptest/data.go index 0a61f101cf7..adfdd6b5814 100644 --- a/exporters/otlp/internal/otlptest/data.go +++ b/exporters/otlp/internal/otlptest/data.go @@ -99,8 +99,7 @@ func SingleSpanSnapshot() []*tracesdk.SpanSnapshot { Attributes: []attribute.KeyValue{}, MessageEvents: []tracesdk.Event{}, Links: []trace.Link{}, - StatusCode: codes.Ok, - StatusMessage: "", + Status: tracesdk.Status{Code: codes.Ok}, DroppedAttributeCount: 0, DroppedMessageEventCount: 0, DroppedLinkCount: 0, diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index 342f349a24f..19801791abd 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -108,7 +108,7 @@ func span(sd *tracesdk.SpanSnapshot) *tracepb.Span { TraceId: tid[:], SpanId: sid[:], TraceState: sd.SpanContext.TraceState().String(), - Status: status(sd.StatusCode, sd.StatusMessage), + Status: status(sd.Status.Code, sd.Status.Description), StartTimeUnixNano: uint64(sd.StartTime.UnixNano()), EndTimeUnixNano: uint64(sd.EndTime.UnixNano()), Links: links(sd.Links), diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index 9d89a80b7bd..eef72645224 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -249,8 +249,10 @@ func TestSpanData(t *testing.T) { }, }, }, - StatusCode: codes.Error, - StatusMessage: "utterly unrecognized", + Status: tracesdk.Status{ + Code: codes.Error, + Description: "utterly unrecognized", + }, Attributes: []attribute.KeyValue{ attribute.Int64("timeout_ns", 12e9), }, @@ -276,7 +278,7 @@ func TestSpanData(t *testing.T) { Kind: tracepb.Span_SPAN_KIND_SERVER, StartTimeUnixNano: uint64(startTime.UnixNano()), EndTimeUnixNano: uint64(endTime.UnixNano()), - Status: status(spanData.StatusCode, spanData.StatusMessage), + Status: status(spanData.Status.Code, spanData.Status.Description), Events: spanEvents(spanData.MessageEvents), Links: links(spanData.Links), Attributes: Attributes(spanData.Attributes), diff --git a/exporters/otlp/otlp_span_test.go b/exporters/otlp/otlp_span_test.go index 26931ca6bb4..9764280e2e6 100644 --- a/exporters/otlp/otlp_span_test.go +++ b/exporters/otlp/otlp_span_test.go @@ -68,9 +68,11 @@ func TestExportSpans(t *testing.T) { attribute.String("user", "alice"), attribute.Bool("authenticated", true), }, - StatusCode: codes.Ok, - StatusMessage: "Ok", - Resource: resource.NewWithAttributes(attribute.String("instance", "tester-a")), + Status: tracesdk.Status{ + Code: codes.Ok, + Description: "Ok", + }, + Resource: resource.NewWithAttributes(attribute.String("instance", "tester-a")), InstrumentationLibrary: instrumentation.Library{ Name: "lib-a", Version: "v0.1.0", @@ -90,9 +92,11 @@ func TestExportSpans(t *testing.T) { attribute.String("user", "alice"), attribute.Bool("authenticated", true), }, - StatusCode: codes.Ok, - StatusMessage: "Ok", - Resource: resource.NewWithAttributes(attribute.String("instance", "tester-a")), + Status: tracesdk.Status{ + Code: codes.Ok, + Description: "Ok", + }, + Resource: resource.NewWithAttributes(attribute.String("instance", "tester-a")), InstrumentationLibrary: instrumentation.Library{ Name: "lib-b", Version: "v0.1.0", @@ -117,9 +121,11 @@ func TestExportSpans(t *testing.T) { attribute.String("user", "alice"), attribute.Bool("authenticated", true), }, - StatusCode: codes.Ok, - StatusMessage: "Ok", - Resource: resource.NewWithAttributes(attribute.String("instance", "tester-a")), + Status: tracesdk.Status{ + Code: codes.Ok, + Description: "Ok", + }, + Resource: resource.NewWithAttributes(attribute.String("instance", "tester-a")), InstrumentationLibrary: instrumentation.Library{ Name: "lib-a", Version: "v0.1.0", @@ -139,9 +145,11 @@ func TestExportSpans(t *testing.T) { attribute.String("user", "bob"), attribute.Bool("authenticated", false), }, - StatusCode: codes.Error, - StatusMessage: "Unauthenticated", - Resource: resource.NewWithAttributes(attribute.String("instance", "tester-b")), + Status: tracesdk.Status{ + Code: codes.Error, + Description: "Unauthenticated", + }, + Resource: resource.NewWithAttributes(attribute.String("instance", "tester-b")), InstrumentationLibrary: instrumentation.Library{ Name: "lib-a", Version: "v1.1.0", diff --git a/exporters/stdout/trace_test.go b/exporters/stdout/trace_test.go index 8fa64feff58..6c2974a5c57 100644 --- a/exporters/stdout/trace_test.go +++ b/exporters/stdout/trace_test.go @@ -64,10 +64,12 @@ func TestExporter_ExportSpan(t *testing.T) { {Name: "foo", Attributes: []attribute.KeyValue{attribute.String("key", keyValue)}, Time: now}, {Name: "bar", Attributes: []attribute.KeyValue{attribute.Float64("double", doubleValue)}, Time: now}, }, - SpanKind: trace.SpanKindInternal, - StatusCode: codes.Error, - StatusMessage: "interesting", - Resource: resource, + SpanKind: trace.SpanKindInternal, + Status: tracesdk.Status{ + Code: codes.Error, + Description: "interesting", + }, + Resource: resource, } if err := ex.ExportSpans(context.Background(), []*tracesdk.SpanSnapshot{testSpan}); err != nil { t.Fatal(err) @@ -129,8 +131,7 @@ func TestExporter_ExportSpan(t *testing.T) { `}` + `],` + `"Links":null,` + - `"StatusCode":"Error",` + - `"StatusMessage":"interesting",` + + `"Status":{"Code":"Error","Description":"interesting"},` + `"DroppedAttributeCount":0,` + `"DroppedMessageEventCount":0,` + `"DroppedLinkCount":0,` + diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index d3f5f97fc2a..c2926b61f46 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -170,13 +170,13 @@ func spanSnapshotToThrift(ss *sdktrace.SpanSnapshot) *gen.Span { ) } - if ss.StatusCode != codes.Unset { - tags = append(tags, getInt64Tag(keyStatusCode, int64(ss.StatusCode))) - if ss.StatusMessage != "" { - tags = append(tags, getStringTag(keyStatusMessage, ss.StatusMessage)) + if ss.Status.Code != codes.Unset { + tags = append(tags, getInt64Tag(keyStatusCode, int64(ss.Status.Code))) + if ss.Status.Description != "" { + tags = append(tags, getStringTag(keyStatusMessage, ss.Status.Description)) } - if ss.StatusCode == codes.Error { + if ss.Status.Code == codes.Error { tags = append(tags, getBoolTag(keyError, true)) } } diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index cb9a4549593..a503f323b93 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -229,11 +229,11 @@ func Test_spanSnapshotToThrift(t *testing.T) { TraceID: traceID, SpanID: spanID, }), - Name: "/foo", - StartTime: now, - EndTime: now, - StatusCode: codes.Error, - SpanKind: trace.SpanKindClient, + Name: "/foo", + StartTime: now, + EndTime: now, + Status: sdktrace.Status{Code: codes.Error}, + SpanKind: trace.SpanKindClient, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, Version: instrLibVersion, @@ -287,9 +287,11 @@ func Test_spanSnapshotToThrift(t *testing.T) { Time: now, }, }, - StatusCode: codes.Error, - StatusMessage: statusMessage, - SpanKind: trace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: statusMessage, + }, + SpanKind: trace.SpanKindClient, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, Version: instrLibVersion, @@ -370,9 +372,11 @@ func Test_spanSnapshotToThrift(t *testing.T) { Attributes: []attribute.KeyValue{ attribute.Array("arr", []int{0, 1, 2, 3}), }, - StatusCode: codes.Unset, - StatusMessage: statusMessage, - SpanKind: trace.SpanKindInternal, + Status: sdktrace.Status{ + Code: codes.Unset, + Description: statusMessage, + }, + SpanKind: trace.SpanKindInternal, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, Version: instrLibVersion, @@ -421,9 +425,11 @@ func Test_spanSnapshotToThrift(t *testing.T) { attribute.Int64("rk2", rv2), semconv.ServiceNameKey.String("service name"), ), - StatusCode: codes.Unset, - StatusMessage: statusMessage, - SpanKind: trace.SpanKindInternal, + Status: sdktrace.Status{ + Code: codes.Unset, + Description: statusMessage, + }, + SpanKind: trace.SpanKindInternal, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, Version: instrLibVersion, diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go index 74a3ffc71a6..51a8bc75ec2 100644 --- a/exporters/trace/zipkin/model.go +++ b/exporters/trace/zipkin/model.go @@ -187,12 +187,12 @@ func toZipkinTags(data *tracesdk.SpanSnapshot) map[string]string { } } - if data.StatusCode != codes.Unset { - m["otel.status_code"] = data.StatusCode.String() + if data.Status.Code != codes.Unset { + m["otel.status_code"] = data.Status.Code.String() } - if data.StatusCode == codes.Error { - m["error"] = data.StatusMessage + if data.Status.Code == codes.Error { + m["error"] = data.Status.Description } else { delete(m, "error") } diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go index ab0979a0554..1e76c6f2fc9 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -30,6 +30,7 @@ import ( "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/resource" + sdktrace "go.opentelemetry.io/otel/sdk/trace" tracesdk "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/semconv" "go.opentelemetry.io/otel/trace" @@ -74,9 +75,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data with no parent (same as typical, but has // invalid parent) @@ -107,9 +110,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data of unspecified kind { @@ -143,9 +148,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data of internal kind { @@ -179,9 +186,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data of client kind { @@ -218,9 +227,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data of producer kind { @@ -254,9 +265,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data of consumer kind { @@ -290,9 +303,11 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data with no events { @@ -313,9 +328,11 @@ func TestModelConversion(t *testing.T) { attribute.String("attr2", "bar"), }, MessageEvents: nil, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // span data with an "error" attribute set to "false" { @@ -348,8 +365,7 @@ func TestModelConversion(t *testing.T) { Attributes: nil, }, }, - StatusCode: codes.Unset, - Resource: resource, + Resource: resource, }, } @@ -759,8 +775,10 @@ func TestTagsTransformation(t *testing.T) { attribute.String("key", keyValue), attribute.Bool("error", true), }, - StatusCode: codes.Error, - StatusMessage: statusMessage, + Status: sdktrace.Status{ + Code: codes.Error, + Description: statusMessage, + }, }, want: map[string]string{ "error": statusMessage, diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index 95b0b3d625b..bd63f721e9a 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/exporters/trace/zipkin/zipkin_test.go @@ -246,9 +246,11 @@ func TestExportSpans(t *testing.T) { EndTime: time.Date(2020, time.March, 11, 19, 25, 0, 0, time.UTC), Attributes: nil, MessageEvents: nil, - StatusCode: codes.Error, - StatusMessage: "404, file not found", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "404, file not found", + }, + Resource: resource, }, // child { @@ -266,9 +268,11 @@ func TestExportSpans(t *testing.T) { EndTime: time.Date(2020, time.March, 11, 19, 24, 45, 0, time.UTC), Attributes: nil, MessageEvents: nil, - StatusCode: codes.Error, - StatusMessage: "403, forbidden", - Resource: resource, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "403, forbidden", + }, + Resource: resource, }, } models := []zkmodel.SpanModel{ diff --git a/sdk/trace/span.go b/sdk/trace/span.go index e29d6c8595b..6318fcbeff9 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -44,8 +44,7 @@ type ReadOnlySpan interface { Attributes() []attribute.KeyValue Links() []trace.Link Events() []Event - StatusCode() codes.Code - StatusMessage() string + Status() Status IsRecording() bool InstrumentationLibrary() instrumentation.Library Resource() *resource.Resource @@ -89,11 +88,8 @@ type span struct { // value of time.Time until the span is ended. endTime time.Time - // statusCode represents the status of this span as a codes.Code value. - statusCode codes.Code - - // statusMessage represents the status of this span as a string. - statusMessage string + // status is the status of this span. + status Status // childSpanCount holds the number of child spans created for this span. childSpanCount int @@ -151,19 +147,22 @@ func (s *span) IsRecording() bool { return !s.startTime.IsZero() && s.endTime.IsZero() } -// SetStatus sets the status of this span in the form of a code and a -// message. This overrides the existing value of this span's status if one -// exists. Message will be set only if status is error. If this span is not being -// recorded than this method does nothing. -func (s *span) SetStatus(code codes.Code, msg string) { +// SetStatus sets the status of the Span in the form of a code and a +// description, overriding previous values set. The description is only +// included in the set status when the code is for an error. If this span is +// not being recorded than this method does nothing. +func (s *span) SetStatus(code codes.Code, description string) { if !s.IsRecording() { return } - s.mu.Lock() - s.statusCode = code + + status := Status{Code: code} if code == codes.Error { - s.statusMessage = msg + status.Description = description } + + s.mu.Lock() + s.status = status s.mu.Unlock() } @@ -378,18 +377,11 @@ func (s *span) Events() []Event { return s.interfaceArrayToMessageEventArray() } -// StatusCode returns the status code of this span. -func (s *span) StatusCode() codes.Code { +// Status returns the status of this span. +func (s *span) Status() Status { s.mu.Lock() defer s.mu.Unlock() - return s.statusCode -} - -// StatusMessage returns the status message of this span. -func (s *span) StatusMessage() string { - s.mu.Lock() - defer s.mu.Unlock() - return s.statusMessage + return s.status } // InstrumentationLibrary returns the instrumentation.Library associated with @@ -440,8 +432,7 @@ func (s *span) Snapshot() *SpanSnapshot { sd.SpanContext = s.spanContext sd.SpanKind = s.spanKind sd.StartTime = s.startTime - sd.StatusCode = s.statusCode - sd.StatusMessage = s.statusMessage + sd.Status = s.status if s.attributes.evictList.Len() > 0 { sd.Attributes = s.attributes.toKeyValue() @@ -577,6 +568,15 @@ func isSampled(s SamplingResult) bool { return s.Decision == RecordAndSample } +// Status is the classified state of a Span. +type Status struct { + // Code is an identifier of a Spans state classification. + Code codes.Code + // Message is a user hint about why that status was set. It is only + // applicable when Code is Error. + Description string +} + // SpanSnapshot is a snapshot of a span which contains all the information // collected by the span. Its main purpose is exporting completed spans. // Although SpanSnapshot fields can be accessed and potentially modified, @@ -594,8 +594,7 @@ type SpanSnapshot struct { Attributes []attribute.KeyValue MessageEvents []Event Links []trace.Link - StatusCode codes.Code - StatusMessage string + Status Status // DroppedAttributeCount contains dropped attributes for the span itself. DroppedAttributeCount int diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 6495215f116..4789c20d109 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -738,11 +738,13 @@ func TestSetSpanStatus(t *testing.T) { TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - StatusCode: codes.Error, - StatusMessage: "Error", + Parent: sc.WithRemote(true), + Name: "span0", + SpanKind: trace.SpanKindInternal, + Status: Status{ + Code: codes.Error, + Description: "Error", + }, InstrumentationLibrary: instrumentation.Library{Name: "SpanStatus"}, } if diff := cmpDiff(got, want); diff != "" { @@ -766,11 +768,13 @@ func TestSetSpanStatusWithoutMessageWhenStatusIsNotError(t *testing.T) { TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - StatusCode: codes.Ok, - StatusMessage: "", + Parent: sc.WithRemote(true), + Name: "span0", + SpanKind: trace.SpanKindInternal, + Status: Status{ + Code: codes.Ok, + Description: "", + }, InstrumentationLibrary: instrumentation.Library{Name: "SpanStatus"}, } if diff := cmpDiff(got, want); diff != "" { @@ -1115,10 +1119,10 @@ func TestRecordError(t *testing.T) { TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - StatusCode: codes.Unset, - SpanKind: trace.SpanKindInternal, + Parent: sc.WithRemote(true), + Name: "span0", + Status: Status{Code: codes.Unset}, + SpanKind: trace.SpanKindInternal, MessageEvents: []Event{ { Name: semconv.ExceptionEventName, @@ -1154,11 +1158,13 @@ func TestRecordErrorNil(t *testing.T) { TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - StatusCode: codes.Unset, - StatusMessage: "", + Parent: sc.WithRemote(true), + Name: "span0", + SpanKind: trace.SpanKindInternal, + Status: Status{ + Code: codes.Unset, + Description: "", + }, InstrumentationLibrary: instrumentation.Library{Name: "RecordErrorNil"}, } if diff := cmpDiff(got, want); diff != "" { @@ -1380,8 +1386,8 @@ func TestReadOnlySpan(t *testing.T) { assert.Equal(t, linked, ro.Links()[0].SpanContext) assert.Equal(t, kv.Key, ro.Events()[0].Attributes[0].Key) assert.Equal(t, kv.Value, ro.Events()[0].Attributes[0].Value) - assert.Equal(t, codes.Ok, ro.StatusCode()) - assert.Equal(t, "", ro.StatusMessage()) + assert.Equal(t, codes.Ok, ro.Status().Code) + assert.Equal(t, "", ro.Status().Description) assert.Equal(t, "ReadOnlySpan", ro.InstrumentationLibrary().Name) assert.Equal(t, "3", ro.InstrumentationLibrary().Version) assert.Equal(t, kv.Key, ro.Resource().Attributes()[0].Key) diff --git a/trace/trace.go b/trace/trace.go index af1d89e5c5e..d1494100609 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -525,9 +525,9 @@ type Span interface { SpanContext() SpanContext // SetStatus sets the status of the Span in the form of a code and a - // message. SetStatus overrides the value of previous calls to SetStatus - // on the Span. - SetStatus(code codes.Code, msg string) + // description, overriding previous values set. The description is only + // included in a status when the code is for an error. + SetStatus(code codes.Code, description string) // SetName sets the Span name. SetName(name string) From 88a4533d7bc720654c24a7ecf5c2ae7af75cc0ee Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 12:03:53 -0700 Subject: [PATCH 04/14] Remove IsRecording from the ReadOnlySpan interface A read-only span value does not need to know if updates to it will be recorded. It by definition cannot be updated so no point in communicating if an update would be recorded. --- sdk/trace/span.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 6318fcbeff9..eb114e97233 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -45,7 +45,6 @@ type ReadOnlySpan interface { Links() []trace.Link Events() []Event Status() Status - IsRecording() bool InstrumentationLibrary() instrumentation.Library Resource() *resource.Resource Snapshot() *SpanSnapshot From d1b8ca3de85f2913639f1b92eeaa090cec470f76 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 12:19:51 -0700 Subject: [PATCH 05/14] Document the ReadOnlySpan interface --- sdk/trace/span.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index eb114e97233..cabc7927a2d 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -35,18 +35,36 @@ import ( // trace.Span. It is used in places where reading information from a span is // necessary but changing the span isn't necessary or allowed. type ReadOnlySpan interface { + // Name returns the name of the span. Name() string + // SpanContext returns the unique SpanContext that identifies the span. SpanContext() trace.SpanContext + // Parent returns the unique SpanContext that identifies the parent of the + // span if one exists. If the span has no parent the returned SpanContext + // will be invalid. Parent() trace.SpanContext + // SpanKind returns the role the span plays in a Trace. SpanKind() trace.SpanKind + // StartTime returns the time the span started recording. StartTime() time.Time + // EndTime returns the time the span stopped recording. It will be zero if + // the span has not ended. EndTime() time.Time + // Attributes returns the defining attributes of the span. Attributes() []attribute.KeyValue + // Links returns all the links the span has to other spans. Links() []trace.Link + // Events returns all the events that occurred within in the spans + // lifetime. Events() []Event + // Status returns the spans status. Status() Status + // InstrumentationLibrary returns information about the instrumentation + // library that created the span. InstrumentationLibrary() instrumentation.Library + // Resource returns information about the entity that produced the span. Resource() *resource.Resource + // TODO: remove this. Snapshot() *SpanSnapshot // A private method to prevent users implementing the From 86ae1ff5ba02990991602f3485e92324971dca81 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 12:28:24 -0700 Subject: [PATCH 06/14] Rename messageEvent* to just event* --- exporters/otlp/internal/otlptest/data.go | 26 +++++------ exporters/otlp/internal/transform/span.go | 16 +++---- .../otlp/internal/transform/span_test.go | 24 +++++----- exporters/stdout/trace_test.go | 6 +-- exporters/trace/jaeger/jaeger.go | 2 +- exporters/trace/jaeger/jaeger_test.go | 2 +- exporters/trace/zipkin/model.go | 2 +- exporters/trace/zipkin/model_test.go | 18 ++++---- exporters/trace/zipkin/zipkin_test.go | 24 +++++----- sdk/trace/span.go | 44 +++++++++---------- sdk/trace/trace_test.go | 36 +++++++-------- 11 files changed, 100 insertions(+), 100 deletions(-) diff --git a/exporters/otlp/internal/otlptest/data.go b/exporters/otlp/internal/otlptest/data.go index adfdd6b5814..6873668f999 100644 --- a/exporters/otlp/internal/otlptest/data.go +++ b/exporters/otlp/internal/otlptest/data.go @@ -92,19 +92,19 @@ func SingleSpanSnapshot() []*tracesdk.SpanSnapshot { SpanID: trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8}, TraceFlags: trace.FlagsSampled, }), - SpanKind: trace.SpanKindInternal, - Name: "foo", - StartTime: time.Date(2020, time.December, 8, 20, 23, 0, 0, time.UTC), - EndTime: time.Date(2020, time.December, 0, 20, 24, 0, 0, time.UTC), - Attributes: []attribute.KeyValue{}, - MessageEvents: []tracesdk.Event{}, - Links: []trace.Link{}, - Status: tracesdk.Status{Code: codes.Ok}, - DroppedAttributeCount: 0, - DroppedMessageEventCount: 0, - DroppedLinkCount: 0, - ChildSpanCount: 0, - Resource: resource.NewWithAttributes(attribute.String("a", "b")), + SpanKind: trace.SpanKindInternal, + Name: "foo", + StartTime: time.Date(2020, time.December, 8, 20, 23, 0, 0, time.UTC), + EndTime: time.Date(2020, time.December, 0, 20, 24, 0, 0, time.UTC), + Attributes: []attribute.KeyValue{}, + Events: []tracesdk.Event{}, + Links: []trace.Link{}, + Status: tracesdk.Status{Code: codes.Ok}, + DroppedAttributeCount: 0, + DroppedEventCount: 0, + DroppedLinkCount: 0, + ChildSpanCount: 0, + Resource: resource.NewWithAttributes(attribute.String("a", "b")), InstrumentationLibrary: instrumentation.Library{ Name: "bar", Version: "0.0.0", diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index 19801791abd..ae6b3ee22e7 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -25,7 +25,7 @@ import ( ) const ( - maxMessageEventsPerSpan = 128 + maxEventsPerSpan = 128 ) // SpanData transforms a slice of SpanSnapshot into a slice of OTLP @@ -115,9 +115,9 @@ func span(sd *tracesdk.SpanSnapshot) *tracepb.Span { Kind: spanKind(sd.SpanKind), Name: sd.Name, Attributes: Attributes(sd.Attributes), - Events: spanEvents(sd.MessageEvents), + Events: spanEvents(sd.Events), DroppedAttributesCount: uint32(sd.DroppedAttributeCount), - DroppedEventsCount: uint32(sd.DroppedMessageEventCount), + DroppedEventsCount: uint32(sd.DroppedEventCount), DroppedLinksCount: uint32(sd.DroppedLinkCount), } @@ -174,18 +174,18 @@ func spanEvents(es []tracesdk.Event) []*tracepb.Span_Event { } evCount := len(es) - if evCount > maxMessageEventsPerSpan { - evCount = maxMessageEventsPerSpan + if evCount > maxEventsPerSpan { + evCount = maxEventsPerSpan } events := make([]*tracepb.Span_Event, 0, evCount) - messageEvents := 0 + nEvents := 0 // Transform message events for _, e := range es { - if messageEvents >= maxMessageEventsPerSpan { + if nEvents >= maxEventsPerSpan { break } - messageEvents++ + nEvents++ events = append(events, &tracepb.Span_Event{ Name: e.Name, diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index eef72645224..74e2bca7800 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -101,15 +101,15 @@ func TestSpanEvent(t *testing.T) { } func TestExcessiveSpanEvents(t *testing.T) { - e := make([]tracesdk.Event, maxMessageEventsPerSpan+1) - for i := 0; i < maxMessageEventsPerSpan+1; i++ { + e := make([]tracesdk.Event, maxEventsPerSpan+1) + for i := 0; i < maxEventsPerSpan+1; i++ { e[i] = tracesdk.Event{Name: strconv.Itoa(i)} } - assert.Len(t, e, maxMessageEventsPerSpan+1) + assert.Len(t, e, maxEventsPerSpan+1) got := spanEvents(e) - assert.Len(t, got, maxMessageEventsPerSpan) + assert.Len(t, got, maxEventsPerSpan) // Ensure the drop order. - assert.Equal(t, strconv.Itoa(maxMessageEventsPerSpan-1), got[len(got)-1].Name) + assert.Equal(t, strconv.Itoa(maxEventsPerSpan-1), got[len(got)-1].Name) } func TestNilLinks(t *testing.T) { @@ -215,7 +215,7 @@ func TestSpanData(t *testing.T) { Name: "span data to span data", StartTime: startTime, EndTime: endTime, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ {Time: startTime, Attributes: []attribute.KeyValue{ attribute.Int64("CompressedByteSize", 512), @@ -223,7 +223,7 @@ func TestSpanData(t *testing.T) { }, {Time: endTime, Attributes: []attribute.KeyValue{ - attribute.String("MessageEventType", "Recv"), + attribute.String("EventType", "Recv"), }, }, }, @@ -256,10 +256,10 @@ func TestSpanData(t *testing.T) { Attributes: []attribute.KeyValue{ attribute.Int64("timeout_ns", 12e9), }, - DroppedAttributeCount: 1, - DroppedMessageEventCount: 2, - DroppedLinkCount: 3, - Resource: resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)), + DroppedAttributeCount: 1, + DroppedEventCount: 2, + DroppedLinkCount: 3, + Resource: resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)), InstrumentationLibrary: instrumentation.Library{ Name: "go.opentelemetry.io/test/otel", Version: "v0.0.1", @@ -279,7 +279,7 @@ func TestSpanData(t *testing.T) { StartTimeUnixNano: uint64(startTime.UnixNano()), EndTimeUnixNano: uint64(endTime.UnixNano()), Status: status(spanData.Status.Code, spanData.Status.Description), - Events: spanEvents(spanData.MessageEvents), + Events: spanEvents(spanData.Events), Links: links(spanData.Links), Attributes: Attributes(spanData.Attributes), DroppedAttributesCount: 1, diff --git a/exporters/stdout/trace_test.go b/exporters/stdout/trace_test.go index 6c2974a5c57..ccd6b497ce0 100644 --- a/exporters/stdout/trace_test.go +++ b/exporters/stdout/trace_test.go @@ -60,7 +60,7 @@ func TestExporter_ExportSpan(t *testing.T) { attribute.String("key", keyValue), attribute.Float64("double", doubleValue), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ {Name: "foo", Attributes: []attribute.KeyValue{attribute.String("key", keyValue)}, Time: now}, {Name: "bar", Attributes: []attribute.KeyValue{attribute.Float64("double", doubleValue)}, Time: now}, }, @@ -106,7 +106,7 @@ func TestExporter_ExportSpan(t *testing.T) { `"Key":"double",` + `"Value":{"Type":"FLOAT64","Value":123.456}` + `}],` + - `"MessageEvents":[` + + `"Events":[` + `{` + `"Name":"foo",` + `"Attributes":[` + @@ -133,7 +133,7 @@ func TestExporter_ExportSpan(t *testing.T) { `"Links":null,` + `"Status":{"Code":"Error","Description":"interesting"},` + `"DroppedAttributeCount":0,` + - `"DroppedMessageEventCount":0,` + + `"DroppedEventCount":0,` + `"DroppedLinkCount":0,` + `"ChildSpanCount":0,` + `"Resource":[` + diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index c2926b61f46..8fa55a895f3 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -182,7 +182,7 @@ func spanSnapshotToThrift(ss *sdktrace.SpanSnapshot) *gen.Span { } var logs []*gen.Log - for _, a := range ss.MessageEvents { + for _, a := range ss.Events { nTags := len(a.Attributes) if a.Name != "" { nTags++ diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index a503f323b93..39f6f592b4e 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -279,7 +279,7 @@ func Test_spanSnapshotToThrift(t *testing.T) { attribute.Float64("double", doubleValue), attribute.Int64("int", intValue), }, - MessageEvents: []sdktrace.Event{ + Events: []sdktrace.Event{ { Name: eventNameValue, Attributes: []attribute.KeyValue{attribute.String("k1", keyValue)}, diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go index 51a8bc75ec2..a83e1a0179d 100644 --- a/exporters/trace/zipkin/model.go +++ b/exporters/trace/zipkin/model.go @@ -81,7 +81,7 @@ func toZipkinSpanModel(data *tracesdk.SpanSnapshot) zkmodel.SpanModel { ServiceName: getServiceName(data.Resource.Attributes()), }, RemoteEndpoint: toZipkinRemoteEndpoint(data), - Annotations: toZipkinAnnotations(data.MessageEvents), + Annotations: toZipkinAnnotations(data.Events), Tags: toZipkinTags(data), } } diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go index 1e76c6f2fc9..6e2ab83f23a 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -61,7 +61,7 @@ func TestModelConversion(t *testing.T) { attribute.String("attr2", "bar"), attribute.Array("attr3", []int{0, 1, 2}), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -96,7 +96,7 @@ func TestModelConversion(t *testing.T) { attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -134,7 +134,7 @@ func TestModelConversion(t *testing.T) { attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -172,7 +172,7 @@ func TestModelConversion(t *testing.T) { attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -213,7 +213,7 @@ func TestModelConversion(t *testing.T) { attribute.String("net.peer.ip", "1.2.3.4"), attribute.Int64("net.peer.port", 9876), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -251,7 +251,7 @@ func TestModelConversion(t *testing.T) { attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -289,7 +289,7 @@ func TestModelConversion(t *testing.T) { attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", @@ -327,7 +327,7 @@ func TestModelConversion(t *testing.T) { attribute.Int64("attr1", 42), attribute.String("attr2", "bar"), }, - MessageEvents: nil, + Events: nil, Status: sdktrace.Status{ Code: codes.Error, Description: "404, file not found", @@ -351,7 +351,7 @@ func TestModelConversion(t *testing.T) { Attributes: []attribute.KeyValue{ attribute.String("error", "false"), }, - MessageEvents: []tracesdk.Event{ + Events: []tracesdk.Event{ { Time: time.Date(2020, time.March, 11, 19, 24, 30, 0, time.UTC), Name: "ev1", diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index bd63f721e9a..739ad193621 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/exporters/trace/zipkin/zipkin_test.go @@ -240,12 +240,12 @@ func TestExportSpans(t *testing.T) { TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}, SpanID: trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8}, }), - SpanKind: trace.SpanKindServer, - Name: "foo", - StartTime: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - EndTime: time.Date(2020, time.March, 11, 19, 25, 0, 0, time.UTC), - Attributes: nil, - MessageEvents: nil, + SpanKind: trace.SpanKindServer, + Name: "foo", + StartTime: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + EndTime: time.Date(2020, time.March, 11, 19, 25, 0, 0, time.UTC), + Attributes: nil, + Events: nil, Status: sdktrace.Status{ Code: codes.Error, Description: "404, file not found", @@ -262,12 +262,12 @@ func TestExportSpans(t *testing.T) { TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}, SpanID: trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8}, }), - SpanKind: trace.SpanKindServer, - Name: "bar", - StartTime: time.Date(2020, time.March, 11, 19, 24, 15, 0, time.UTC), - EndTime: time.Date(2020, time.March, 11, 19, 24, 45, 0, time.UTC), - Attributes: nil, - MessageEvents: nil, + SpanKind: trace.SpanKindServer, + Name: "bar", + StartTime: time.Date(2020, time.March, 11, 19, 24, 15, 0, time.UTC), + EndTime: time.Date(2020, time.March, 11, 19, 24, 45, 0, time.UTC), + Attributes: nil, + Events: nil, Status: sdktrace.Status{ Code: codes.Error, Description: "403, forbidden", diff --git a/sdk/trace/span.go b/sdk/trace/span.go index cabc7927a2d..f28b0d84de7 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -126,8 +126,8 @@ type span struct { // an oldest entry is removed to create room for a new entry. attributes *attributesMap - // messageEvents are stored in FIFO queue capped by configured limit. - messageEvents *evictedQueue + // events are stored in FIFO queue capped by configured limit. + events *evictedQueue // links are stored in FIFO queue capped by configured limit. links *evictedQueue @@ -308,7 +308,7 @@ func (s *span) addEvent(name string, o ...trace.EventOption) { s.mu.Lock() defer s.mu.Unlock() - s.messageEvents.add(Event{ + s.events.add(Event{ Name: name, Attributes: c.Attributes, DroppedAttributeCount: discarded, @@ -388,10 +388,10 @@ func (s *span) Links() []trace.Link { func (s *span) Events() []Event { s.mu.Lock() defer s.mu.Unlock() - if len(s.messageEvents.queue) == 0 { + if len(s.events.queue) == 0 { return []Event{} } - return s.interfaceArrayToMessageEventArray() + return s.interfaceArrayToEventArray() } // Status returns the status of this span. @@ -455,9 +455,9 @@ func (s *span) Snapshot() *SpanSnapshot { sd.Attributes = s.attributes.toKeyValue() sd.DroppedAttributeCount = s.attributes.droppedCount } - if len(s.messageEvents.queue) > 0 { - sd.MessageEvents = s.interfaceArrayToMessageEventArray() - sd.DroppedMessageEventCount = s.messageEvents.droppedCount + if len(s.events.queue) > 0 { + sd.Events = s.interfaceArrayToEventArray() + sd.DroppedEventCount = s.events.droppedCount } if len(s.links.queue) > 0 { sd.Links = s.interfaceArrayToLinksArray() @@ -474,12 +474,12 @@ func (s *span) interfaceArrayToLinksArray() []trace.Link { return linkArr } -func (s *span) interfaceArrayToMessageEventArray() []Event { - messageEventArr := make([]Event, 0) - for _, value := range s.messageEvents.queue { - messageEventArr = append(messageEventArr, value.(Event)) +func (s *span) interfaceArrayToEventArray() []Event { + eventArr := make([]Event, 0) + for _, value := range s.events.queue { + eventArr = append(eventArr, value.(Event)) } - return messageEventArr + return eventArr } func (s *span) copyToCappedAttributes(attributes ...attribute.KeyValue) { @@ -531,7 +531,7 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, o *trace.Sp spanLimits := provider.spanLimits span.attributes = newAttributesMap(spanLimits.AttributeCountLimit) - span.messageEvents = newEvictedQueue(spanLimits.EventCountLimit) + span.events = newEvictedQueue(spanLimits.EventCountLimit) span.links = newEvictedQueue(spanLimits.LinkCountLimit) span.spanLimits = spanLimits @@ -607,16 +607,16 @@ type SpanSnapshot struct { StartTime time.Time // The wall clock time of EndTime will be adjusted to always be offset // from StartTime by the duration of the span. - EndTime time.Time - Attributes []attribute.KeyValue - MessageEvents []Event - Links []trace.Link - Status Status + EndTime time.Time + Attributes []attribute.KeyValue + Events []Event + Links []trace.Link + Status Status // DroppedAttributeCount contains dropped attributes for the span itself. - DroppedAttributeCount int - DroppedMessageEventCount int - DroppedLinkCount int + DroppedAttributeCount int + DroppedEventCount int + DroppedLinkCount int // ChildSpanCount holds the number of child span created for this span. ChildSpanCount int diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 4789c20d109..9dd9adedc46 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -546,8 +546,8 @@ func TestEvents(t *testing.T) { t.Fatal(err) } - for i := range got.MessageEvents { - if !checkTime(&got.MessageEvents[i].Time) { + for i := range got.Events { + if !checkTime(&got.Events[i].Time) { t.Error("exporting span: expected nonzero Event Time") } } @@ -559,7 +559,7 @@ func TestEvents(t *testing.T) { }), Parent: sc.WithRemote(true), Name: "span0", - MessageEvents: []Event{ + Events: []Event{ {Name: "foo", Attributes: []attribute.KeyValue{k1v1}}, {Name: "bar", Attributes: []attribute.KeyValue{k2v2, k3v3}}, }, @@ -595,8 +595,8 @@ func TestEventsOverLimit(t *testing.T) { t.Fatal(err) } - for i := range got.MessageEvents { - if !checkTime(&got.MessageEvents[i].Time) { + for i := range got.Events { + if !checkTime(&got.Events[i].Time) { t.Error("exporting span: expected nonzero Event Time") } } @@ -608,13 +608,13 @@ func TestEventsOverLimit(t *testing.T) { }), Parent: sc.WithRemote(true), Name: "span0", - MessageEvents: []Event{ + Events: []Event{ {Name: "foo", Attributes: []attribute.KeyValue{k1v1}}, {Name: "bar", Attributes: []attribute.KeyValue{k2v2, k3v3}}, }, - DroppedMessageEventCount: 2, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "EventsOverLimit"}, + DroppedEventCount: 2, + SpanKind: trace.SpanKindInternal, + InstrumentationLibrary: instrumentation.Library{Name: "EventsOverLimit"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("Message Event over limit: -got +want %s", diff) @@ -1123,7 +1123,7 @@ func TestRecordError(t *testing.T) { Name: "span0", Status: Status{Code: codes.Unset}, SpanKind: trace.SpanKindInternal, - MessageEvents: []Event{ + Events: []Event{ { Name: semconv.ExceptionEventName, Time: errTime, @@ -1332,9 +1332,9 @@ func TestSpanCapturesPanic(t *testing.T) { require.PanicsWithError(t, "error message", f) spans := te.Spans() require.Len(t, spans, 1) - require.Len(t, spans[0].MessageEvents, 1) - assert.Equal(t, spans[0].MessageEvents[0].Name, semconv.ExceptionEventName) - assert.Equal(t, spans[0].MessageEvents[0].Attributes, []attribute.KeyValue{ + require.Len(t, spans[0].Events, 1) + assert.Equal(t, spans[0].Events[0].Name, semconv.ExceptionEventName) + assert.Equal(t, spans[0].Events[0].Attributes, []attribute.KeyValue{ semconv.ExceptionTypeKey.String("*errors.errorString"), semconv.ExceptionMessageKey.String("error message"), }) @@ -1402,13 +1402,13 @@ func TestReadOnlySpan(t *testing.T) { d1 := ro.Snapshot() span.AddEvent("baz") d2 := ro.Snapshot() - for _, e := range d1.MessageEvents { + for _, e := range d1.Events { if e.Name == "baz" { t.Errorf("Didn't expect to find 'baz' event") } } var exists bool - for _, e := range d2.MessageEvents { + for _, e := range d2.Events { if e.Name == "baz" { exists = true } @@ -1481,8 +1481,8 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { t.Fatal(err) } - for i := range got.MessageEvents { - if !checkTime(&got.MessageEvents[i].Time) { + for i := range got.Events { + if !checkTime(&got.Events[i].Time) { t.Error("exporting span: expected nonzero Event Time") } } @@ -1495,7 +1495,7 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { Parent: sc.WithRemote(true), Name: "span0", Attributes: nil, - MessageEvents: []Event{ + Events: []Event{ { Name: "test1", Attributes: []attribute.KeyValue{ From b88adf52a5acfecf0648d359733c8fd49b3854d9 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 12:30:13 -0700 Subject: [PATCH 07/14] Move the SpanSnapshot into its own file --- sdk/trace/snapshot.go | 59 +++++++++++++++++++++++++++++++++++++++++++ sdk/trace/span.go | 35 ------------------------- 2 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 sdk/trace/snapshot.go diff --git a/sdk/trace/snapshot.go b/sdk/trace/snapshot.go new file mode 100644 index 00000000000..e475b5a46d6 --- /dev/null +++ b/sdk/trace/snapshot.go @@ -0,0 +1,59 @@ +// 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 ( + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/resource" + "go.opentelemetry.io/otel/trace" +) + +// SpanSnapshot is a snapshot of a span which contains all the information +// collected by the span. Its main purpose is exporting completed spans. +// Although SpanSnapshot fields can be accessed and potentially modified, +// SpanSnapshot should be treated as immutable. Changes to the span from which +// the SpanSnapshot was created are NOT reflected in the SpanSnapshot. +type SpanSnapshot struct { + SpanContext trace.SpanContext + Parent trace.SpanContext + SpanKind trace.SpanKind + Name string + StartTime time.Time + // The wall clock time of EndTime will be adjusted to always be offset + // from StartTime by the duration of the span. + EndTime time.Time + Attributes []attribute.KeyValue + Events []Event + Links []trace.Link + Status Status + + // DroppedAttributeCount contains dropped attributes for the span itself. + DroppedAttributeCount int + DroppedEventCount int + DroppedLinkCount int + + // ChildSpanCount holds the number of child span created for this span. + ChildSpanCount int + + // Resource contains attributes representing an entity that produced this span. + Resource *resource.Resource + + // InstrumentationLibrary defines the instrumentation library used to + // provide instrumentation. + InstrumentationLibrary instrumentation.Library +} diff --git a/sdk/trace/span.go b/sdk/trace/span.go index f28b0d84de7..b5a0fd6275e 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -593,38 +593,3 @@ type Status struct { // applicable when Code is Error. Description string } - -// SpanSnapshot is a snapshot of a span which contains all the information -// collected by the span. Its main purpose is exporting completed spans. -// Although SpanSnapshot fields can be accessed and potentially modified, -// SpanSnapshot should be treated as immutable. Changes to the span from which -// the SpanSnapshot was created are NOT reflected in the SpanSnapshot. -type SpanSnapshot struct { - SpanContext trace.SpanContext - Parent trace.SpanContext - SpanKind trace.SpanKind - Name string - StartTime time.Time - // The wall clock time of EndTime will be adjusted to always be offset - // from StartTime by the duration of the span. - EndTime time.Time - Attributes []attribute.KeyValue - Events []Event - Links []trace.Link - Status Status - - // DroppedAttributeCount contains dropped attributes for the span itself. - DroppedAttributeCount int - DroppedEventCount int - DroppedLinkCount int - - // ChildSpanCount holds the number of child span created for this span. - ChildSpanCount int - - // Resource contains attributes representing an entity that produced this span. - Resource *resource.Resource - - // InstrumentationLibrary defines the instrumentation library used to - // provide instrumentation. - InstrumentationLibrary instrumentation.Library -} From 49edd279025902c7059104bf0e392b4114741edc Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 13:23:19 -0700 Subject: [PATCH 08/14] Update ReadOnlySpan interface with meta info methods Add the DroppedAttributes, DroppedLinks, DroppedEvents, and ChildSpanCount methods to the interface to return additional information about the span not specified by the specification, but that we are already providing. --- sdk/trace/span.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index b5a0fd6275e..671de5ce4c5 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -64,6 +64,19 @@ type ReadOnlySpan interface { InstrumentationLibrary() instrumentation.Library // Resource returns information about the entity that produced the span. Resource() *resource.Resource + // DroppedAttributes returns the number of attributes dropped by the span + // due to limits being reached. + DroppedAttributes() int + // DroppedLinks returns the number of links dropped by the span due to + // limits being reached. + DroppedLinks() int + // DroppedEvents returns the number of events dropped by the span due to + // limits being reached. + DroppedEvents() int + // ChildSpanCount returns the count of spans that consider the span a + // direct parent. + ChildSpanCount() int + // TODO: remove this. Snapshot() *SpanSnapshot @@ -433,6 +446,38 @@ func (s *span) addLink(link trace.Link) { s.links.add(link) } +// DroppedAttributes returns the number of attributes dropped by the span +// due to limits being reached. +func (s *span) DroppedAttributes() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.attributes.droppedCount +} + +// DroppedLinks returns the number of links dropped by the span due to limits +// being reached. +func (s *span) DroppedLinks() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.links.droppedCount +} + +// DroppedEvents returns the number of events dropped by the span due to +// limits being reached. +func (s *span) DroppedEvents() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.events.droppedCount +} + +// ChildSpanCount returns the count of spans that consider the span a +// direct parent. +func (s *span) ChildSpanCount() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.childSpanCount +} + // Snapshot creates a snapshot representing the current state of the span as an // export.SpanSnapshot and returns a pointer to it. func (s *span) Snapshot() *SpanSnapshot { From 45a0e20301ae8b9e8c89cf378ae70bb52712d1da Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 30 Apr 2021 13:46:17 -0700 Subject: [PATCH 09/14] Add SpanStub to the sdk/trace/tracetest pkg --- sdk/trace/tracetest/span.go | 163 ++++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 sdk/trace/tracetest/span.go diff --git a/sdk/trace/tracetest/span.go b/sdk/trace/tracetest/span.go new file mode 100644 index 00000000000..964b317c6f0 --- /dev/null +++ b/sdk/trace/tracetest/span.go @@ -0,0 +1,163 @@ +// 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 tracetest // import "go.opentelemetry.io/otel/sdk/trace/tracetest" + +import ( + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/resource" + tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" +) + +type SpanStubs []SpanStub + +// SpanStubFromReadOnlySpan returns SpanStubs populated from ro. +func SpanStubsFromReadOnlySpans(ro []tracesdk.ReadOnlySpan) SpanStubs { + if len(ro) == 0 { + return nil + } + + s := make(SpanStubs, 0, len(ro)) + for _, r := range ro { + s = append(s, SpanStubFromReadOnlySpan(r)) + } + + return s +} + +// Snapshots returns s as a slice of ReadOnlySpans. +func (s SpanStubs) Snapshots() []tracesdk.ReadOnlySpan { + if len(s) == 0 { + return nil + } + + ro := make([]tracesdk.ReadOnlySpan, len(s)) + for i := 0; i < len(s); i++ { + ro[i] = s[i].Snapshot() + } + return ro +} + +// SpanStub is a stand-in for a Span. +type SpanStub struct { + Name string + SpanContext trace.SpanContext + Parent trace.SpanContext + SpanKind trace.SpanKind + StartTime time.Time + EndTime time.Time + Attributes []attribute.KeyValue + Events []tracesdk.Event + Links []trace.Link + Status tracesdk.Status + DroppedAttributes int + DroppedEvents int + DroppedLinks int + ChildSpanCount int + Resource *resource.Resource + InstrumentationLibrary instrumentation.Library +} + +// SpanStubFromReadOnlySpan returns a SpanStub populated from ro. +func SpanStubFromReadOnlySpan(ro tracesdk.ReadOnlySpan) SpanStub { + if ro == nil { + return SpanStub{} + } + + return SpanStub{ + Name: ro.Name(), + SpanContext: ro.SpanContext(), + Parent: ro.Parent(), + SpanKind: ro.SpanKind(), + StartTime: ro.StartTime(), + EndTime: ro.EndTime(), + Attributes: ro.Attributes(), + Events: ro.Events(), + Links: ro.Links(), + Status: ro.Status(), + DroppedAttributes: ro.DroppedAttributes(), + DroppedEvents: ro.DroppedEvents(), + DroppedLinks: ro.DroppedLinks(), + ChildSpanCount: ro.ChildSpanCount(), + Resource: ro.Resource(), + InstrumentationLibrary: ro.InstrumentationLibrary(), + } +} + +// Snapshot returns a read-only copy of the SpanStub. +func (s SpanStub) Snapshot() tracesdk.ReadOnlySpan { + return spanSnapshot{ + name: s.Name, + spanContext: s.SpanContext, + parent: s.Parent, + spanKind: s.SpanKind, + startTime: s.StartTime, + endTime: s.EndTime, + attributes: s.Attributes, + events: s.Events, + links: s.Links, + status: s.Status, + droppedAttributes: s.DroppedAttributes, + droppedEvents: s.DroppedEvents, + droppedLinks: s.DroppedLinks, + childSpanCount: s.ChildSpanCount, + resource: s.Resource, + instrumentationLibrary: s.InstrumentationLibrary, + } +} + +type spanSnapshot struct { + // Embed the interface to implement the private method. + tracesdk.ReadOnlySpan + + name string + spanContext trace.SpanContext + parent trace.SpanContext + spanKind trace.SpanKind + startTime time.Time + endTime time.Time + attributes []attribute.KeyValue + events []tracesdk.Event + links []trace.Link + status tracesdk.Status + droppedAttributes int + droppedEvents int + droppedLinks int + childSpanCount int + resource *resource.Resource + instrumentationLibrary instrumentation.Library +} + +func (s spanSnapshot) Name() string { return s.name } +func (s spanSnapshot) SpanContext() trace.SpanContext { return s.spanContext } +func (s spanSnapshot) Parent() trace.SpanContext { return s.parent } +func (s spanSnapshot) SpanKind() trace.SpanKind { return s.spanKind } +func (s spanSnapshot) StartTime() time.Time { return s.startTime } +func (s spanSnapshot) EndTime() time.Time { return s.endTime } +func (s spanSnapshot) Attributes() []attribute.KeyValue { return s.attributes } +func (s spanSnapshot) Links() []trace.Link { return s.links } +func (s spanSnapshot) Events() []tracesdk.Event { return s.events } +func (s spanSnapshot) Status() tracesdk.Status { return s.status } +func (s spanSnapshot) DroppedAttributes() int { return s.droppedAttributes } +func (s spanSnapshot) DroppedLinks() int { return s.droppedLinks } +func (s spanSnapshot) DroppedEvents() int { return s.droppedEvents } +func (s spanSnapshot) ChildSpanCount() int { return s.childSpanCount } +func (s spanSnapshot) Resource() *resource.Resource { return s.resource } +func (s spanSnapshot) InstrumentationLibrary() instrumentation.Library { + return s.instrumentationLibrary +} From 32110d2accfb83c1d14c8c5b1f7ca0ab8a348813 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 2 May 2021 09:50:11 -0700 Subject: [PATCH 10/14] Redefine ExportSpans of SpanExporter with ReadOnlySpan --- exporters/otlp/internal/otlptest/data.go | 62 ++-- exporters/otlp/internal/transform/span.go | 44 +-- .../otlp/internal/transform/span_test.go | 21 +- exporters/otlp/otlp.go | 2 +- exporters/otlp/otlp_span_test.go | 11 +- exporters/otlp/otlp_test.go | 23 +- exporters/otlp/otlpgrpc/driver.go | 4 +- .../otlp/otlpgrpc/otlp_integration_test.go | 41 +-- exporters/otlp/otlphttp/driver.go | 4 +- exporters/otlp/protocoldriver.go | 6 +- exporters/stdout/trace.go | 5 +- exporters/stdout/trace_test.go | 224 +++++++------ exporters/trace/jaeger/jaeger.go | 54 +-- .../trace/jaeger/jaeger_benchmark_test.go | 9 +- exporters/trace/jaeger/jaeger_test.go | 42 +-- exporters/trace/zipkin/model.go | 51 +-- exporters/trace/zipkin/model_test.go | 47 +-- exporters/trace/zipkin/zipkin.go | 2 +- exporters/trace/zipkin/zipkin_test.go | 5 +- sdk/trace/batch_span_processor.go | 14 +- sdk/trace/batch_span_processor_test.go | 6 +- sdk/trace/simple_span_processor.go | 3 +- sdk/trace/simple_span_processor_test.go | 6 +- sdk/trace/snapshot.go | 144 ++++++-- sdk/trace/span.go | 42 ++- sdk/trace/span_exporter.go | 8 +- .../span_processor_filter_example_test.go | 4 +- sdk/trace/trace_test.go | 310 +++++++++--------- sdk/trace/tracetest/test.go | 12 +- sdk/trace/tracetest/test_test.go | 19 +- 30 files changed, 678 insertions(+), 547 deletions(-) diff --git a/exporters/otlp/internal/otlptest/data.go b/exporters/otlp/internal/otlptest/data.go index 6873668f999..b980e1edc04 100644 --- a/exporters/otlp/internal/otlptest/data.go +++ b/exporters/otlp/internal/otlptest/data.go @@ -28,6 +28,7 @@ import ( "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" "go.opentelemetry.io/otel/sdk/resource" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" ) @@ -80,37 +81,38 @@ func (OneRecordCheckpointSet) ForEach(kindSelector exportmetric.ExportKindSelect // SingleSpanSnapshot returns a one-element slice with a snapshot. It // may be useful for testing driver's trace export. -func SingleSpanSnapshot() []*tracesdk.SpanSnapshot { - sd := &tracesdk.SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9}, - SpanID: trace.SpanID{3, 4, 5, 6, 7, 8, 9, 0}, - TraceFlags: trace.FlagsSampled, - }), - Parent: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9}, - SpanID: trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8}, - TraceFlags: trace.FlagsSampled, - }), - SpanKind: trace.SpanKindInternal, - Name: "foo", - StartTime: time.Date(2020, time.December, 8, 20, 23, 0, 0, time.UTC), - EndTime: time.Date(2020, time.December, 0, 20, 24, 0, 0, time.UTC), - Attributes: []attribute.KeyValue{}, - Events: []tracesdk.Event{}, - Links: []trace.Link{}, - Status: tracesdk.Status{Code: codes.Ok}, - DroppedAttributeCount: 0, - DroppedEventCount: 0, - DroppedLinkCount: 0, - ChildSpanCount: 0, - Resource: resource.NewWithAttributes(attribute.String("a", "b")), - InstrumentationLibrary: instrumentation.Library{ - Name: "bar", - Version: "0.0.0", +func SingleSpanSnapshot() []tracesdk.ReadOnlySpan { + return tracetest.SpanStubs{ + { + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9}, + SpanID: trace.SpanID{3, 4, 5, 6, 7, 8, 9, 0}, + TraceFlags: trace.FlagsSampled, + }), + Parent: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9}, + SpanID: trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8}, + TraceFlags: trace.FlagsSampled, + }), + SpanKind: trace.SpanKindInternal, + Name: "foo", + StartTime: time.Date(2020, time.December, 8, 20, 23, 0, 0, time.UTC), + EndTime: time.Date(2020, time.December, 0, 20, 24, 0, 0, time.UTC), + Attributes: []attribute.KeyValue{}, + Events: []tracesdk.Event{}, + Links: []trace.Link{}, + Status: tracesdk.Status{Code: codes.Ok}, + DroppedAttributes: 0, + DroppedEvents: 0, + DroppedLinks: 0, + ChildSpanCount: 0, + Resource: resource.NewWithAttributes(attribute.String("a", "b")), + InstrumentationLibrary: instrumentation.Library{ + Name: "bar", + Version: "0.0.0", + }, }, - } - return []*tracesdk.SpanSnapshot{sd} + }.Snapshots() } // EmptyCheckpointSet is a checkpointer that has no records at all. diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index ae6b3ee22e7..351d67f51e8 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -28,9 +28,9 @@ const ( maxEventsPerSpan = 128 ) -// SpanData transforms a slice of SpanSnapshot into a slice of OTLP +// Spans transforms a slice of SpanSnapshot into a slice of OTLP // ResourceSpans. -func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans { +func Spans(sdl []tracesdk.ReadOnlySpan) []*tracepb.ResourceSpans { if len(sdl) == 0 { return nil } @@ -49,16 +49,16 @@ func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans { continue } - rKey := sd.Resource.Equivalent() + rKey := sd.Resource().Equivalent() iKey := ilsKey{ r: rKey, - il: sd.InstrumentationLibrary, + il: sd.InstrumentationLibrary(), } ils, iOk := ilsm[iKey] if !iOk { // Either the resource or instrumentation library were unknown. ils = &tracepb.InstrumentationLibrarySpans{ - InstrumentationLibrary: instrumentationLibrary(sd.InstrumentationLibrary), + InstrumentationLibrary: instrumentationLibrary(sd.InstrumentationLibrary()), Spans: []*tracepb.Span{}, } } @@ -70,7 +70,7 @@ func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans { resources++ // The resource was unknown. rs = &tracepb.ResourceSpans{ - Resource: Resource(sd.Resource), + Resource: Resource(sd.Resource()), InstrumentationLibrarySpans: []*tracepb.InstrumentationLibrarySpans{ils}, } rsm[rKey] = rs @@ -96,32 +96,32 @@ func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans { } // span transforms a Span into an OTLP span. -func span(sd *tracesdk.SpanSnapshot) *tracepb.Span { +func span(sd tracesdk.ReadOnlySpan) *tracepb.Span { if sd == nil { return nil } - tid := sd.SpanContext.TraceID() - sid := sd.SpanContext.SpanID() + tid := sd.SpanContext().TraceID() + sid := sd.SpanContext().SpanID() s := &tracepb.Span{ TraceId: tid[:], SpanId: sid[:], - TraceState: sd.SpanContext.TraceState().String(), - Status: status(sd.Status.Code, sd.Status.Description), - StartTimeUnixNano: uint64(sd.StartTime.UnixNano()), - EndTimeUnixNano: uint64(sd.EndTime.UnixNano()), - Links: links(sd.Links), - Kind: spanKind(sd.SpanKind), - Name: sd.Name, - Attributes: Attributes(sd.Attributes), - Events: spanEvents(sd.Events), - DroppedAttributesCount: uint32(sd.DroppedAttributeCount), - DroppedEventsCount: uint32(sd.DroppedEventCount), - DroppedLinksCount: uint32(sd.DroppedLinkCount), + TraceState: sd.SpanContext().TraceState().String(), + Status: status(sd.Status().Code, sd.Status().Description), + StartTimeUnixNano: uint64(sd.StartTime().UnixNano()), + EndTimeUnixNano: uint64(sd.EndTime().UnixNano()), + Links: links(sd.Links()), + Kind: spanKind(sd.SpanKind()), + Name: sd.Name(), + Attributes: Attributes(sd.Attributes()), + Events: spanEvents(sd.Events()), + DroppedAttributesCount: uint32(sd.DroppedAttributes()), + DroppedEventsCount: uint32(sd.DroppedEvents()), + DroppedLinksCount: uint32(sd.DroppedLinks()), } - if psid := sd.Parent.SpanID(); psid.IsValid() { + if psid := sd.Parent().SpanID(); psid.IsValid() { s.ParentSpanId = psid[:] } diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index 74e2bca7800..454531b94e5 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -32,6 +32,7 @@ import ( "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/resource" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" ) func TestSpanKind(t *testing.T) { @@ -185,11 +186,11 @@ func TestNilSpan(t *testing.T) { } func TestNilSpanData(t *testing.T) { - assert.Nil(t, SpanData(nil)) + assert.Nil(t, Spans(nil)) } func TestEmptySpanData(t *testing.T) { - assert.Nil(t, SpanData(nil)) + assert.Nil(t, Spans(nil)) } func TestSpanData(t *testing.T) { @@ -199,7 +200,7 @@ func TestSpanData(t *testing.T) { startTime := time.Unix(1585674086, 1234) endTime := startTime.Add(10 * time.Second) traceState, _ := trace.TraceStateFromKeyValues(attribute.String("key1", "val1"), attribute.String("key2", "val2")) - spanData := &tracesdk.SpanSnapshot{ + spanData := tracetest.SpanStub{ SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}, SpanID: trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8}, @@ -256,10 +257,10 @@ func TestSpanData(t *testing.T) { Attributes: []attribute.KeyValue{ attribute.Int64("timeout_ns", 12e9), }, - DroppedAttributeCount: 1, - DroppedEventCount: 2, - DroppedLinkCount: 3, - Resource: resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)), + DroppedAttributes: 1, + DroppedEvents: 2, + DroppedLinks: 3, + Resource: resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)), InstrumentationLibrary: instrumentation.Library{ Name: "go.opentelemetry.io/test/otel", Version: "v0.0.1", @@ -287,7 +288,7 @@ func TestSpanData(t *testing.T) { DroppedLinksCount: 3, } - got := SpanData([]*tracesdk.SpanSnapshot{spanData}) + got := Spans(tracetest.SpanStubs{spanData}.Snapshots()) require.Len(t, got, 1) assert.Equal(t, got[0].GetResource(), Resource(spanData.Resource)) @@ -304,7 +305,7 @@ func TestSpanData(t *testing.T) { // Empty parent span ID should be treated as root span. func TestRootSpanData(t *testing.T) { - sd := SpanData([]*tracesdk.SpanSnapshot{{}}) + sd := Spans(tracetest.SpanStubs{{}}.Snapshots()) require.Len(t, sd, 1) rs := sd[0] got := rs.GetInstrumentationLibrarySpans()[0].GetSpans()[0].GetParentSpanId() @@ -314,5 +315,5 @@ func TestRootSpanData(t *testing.T) { } func TestSpanDataNilResource(t *testing.T) { - assert.NotPanics(t, func() { SpanData([]*tracesdk.SpanSnapshot{{}}) }) + assert.NotPanics(t, func() { Spans(tracetest.SpanStubs{{}}.Snapshots()) }) } diff --git a/exporters/otlp/otlp.go b/exporters/otlp/otlp.go index 098d93b423a..01685624d5d 100644 --- a/exporters/otlp/otlp.go +++ b/exporters/otlp/otlp.go @@ -131,7 +131,7 @@ func (e *Exporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) // ExportSpans transforms and batches trace SpanSnapshots into OTLP Trace and // transmits them to the configured collector. -func (e *Exporter) ExportSpans(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { +func (e *Exporter) ExportSpans(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { return e.driver.ExportTraces(ctx, ss) } diff --git a/exporters/otlp/otlp_span_test.go b/exporters/otlp/otlp_span_test.go index 9764280e2e6..1d4905180df 100644 --- a/exporters/otlp/otlp_span_test.go +++ b/exporters/otlp/otlp_span_test.go @@ -31,6 +31,7 @@ import ( "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/resource" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" ) func TestExportSpans(t *testing.T) { @@ -41,19 +42,19 @@ func TestExportSpans(t *testing.T) { endTime := startTime.Add(10 * time.Second) for _, test := range []struct { - sd []*tracesdk.SpanSnapshot + sd tracetest.SpanStubs want []*tracepb.ResourceSpans }{ { - []*tracesdk.SpanSnapshot(nil), + tracetest.SpanStubsFromReadOnlySpans(nil), []*tracepb.ResourceSpans(nil), }, { - []*tracesdk.SpanSnapshot{}, + tracetest.SpanStubs{}, []*tracepb.ResourceSpans(nil), }, { - []*tracesdk.SpanSnapshot{ + tracetest.SpanStubs{ { SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: trace.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}), @@ -338,7 +339,7 @@ func TestExportSpans(t *testing.T) { }, } { driver.Reset() - assert.NoError(t, exp.ExportSpans(context.Background(), test.sd)) + assert.NoError(t, exp.ExportSpans(context.Background(), test.sd.Snapshots())) assert.ElementsMatch(t, test.want, driver.rs) } } diff --git a/exporters/otlp/otlp_test.go b/exporters/otlp/otlp_test.go index 42c7cde8049..eee609522b6 100644 --- a/exporters/otlp/otlp_test.go +++ b/exporters/otlp/otlp_test.go @@ -29,16 +29,17 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/internal/transform" metricsdk "go.opentelemetry.io/otel/sdk/export/metric" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" metricpb "go.opentelemetry.io/proto/otlp/metrics/v1" tracepb "go.opentelemetry.io/proto/otlp/trace/v1" ) -func stubSpanSnapshot(count int) []*tracesdk.SpanSnapshot { - spans := make([]*tracesdk.SpanSnapshot, 0, count) +func readonlyspans(count int) []tracesdk.ReadOnlySpan { + spans := make(tracetest.SpanStubs, 0, count) for i := 0; i < count; i++ { - spans = append(spans, new(tracesdk.SpanSnapshot)) + spans = append(spans, tracetest.SpanStub{}) } - return spans + return spans.Snapshots() } type stubCheckpointSet struct { @@ -71,7 +72,7 @@ type stubProtocolDriver struct { injectedStopError error rm []metricsdk.Record - rs []tracesdk.SpanSnapshot + rs tracetest.SpanStubs } var _ otlp.ProtocolDriver = (*stubProtocolDriver)(nil) @@ -104,13 +105,13 @@ func (m *stubProtocolDriver) ExportMetrics(parent context.Context, cps metricsdk }) } -func (m *stubProtocolDriver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { +func (m *stubProtocolDriver) ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { m.tracesExported++ for _, rs := range ss { if rs == nil { continue } - m.rs = append(m.rs, *rs) + m.rs = append(m.rs, tracetest.SpanStubFromReadOnlySpan(rs)) } return nil } @@ -144,8 +145,8 @@ func (m *stubTransformingProtocolDriver) ExportMetrics(parent context.Context, c return nil } -func (m *stubTransformingProtocolDriver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { - for _, rs := range transform.SpanData(ss) { +func (m *stubTransformingProtocolDriver) ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { + for _, rs := range transform.Spans(ss) { if rs == nil { continue } @@ -289,7 +290,7 @@ func TestSplitDriver(t *testing.T) { assertExport := func(t testing.TB, ctx context.Context, driver otlp.ProtocolDriver) { t.Helper() assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector())) - assert.NoError(t, driver.ExportTraces(ctx, stubSpanSnapshot(spanCount))) + assert.NoError(t, driver.ExportTraces(ctx, readonlyspans(spanCount))) } t.Run("with metric/trace drivers configured", func(t *testing.T) { @@ -385,7 +386,7 @@ func TestSplitDriver(t *testing.T) { assert.NoError(t, driver.Start(ctx)) assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector())) - assert.NoError(t, driver.ExportTraces(ctx, stubSpanSnapshot(spanCount))) + assert.NoError(t, driver.ExportTraces(ctx, readonlyspans(spanCount))) assert.NoError(t, driver.Stop(ctx)) }) diff --git a/exporters/otlp/otlpgrpc/driver.go b/exporters/otlp/otlpgrpc/driver.go index 39544564ca9..4b700ff11f2 100644 --- a/exporters/otlp/otlpgrpc/driver.go +++ b/exporters/otlp/otlpgrpc/driver.go @@ -161,7 +161,7 @@ func (md *metricsDriver) uploadMetrics(ctx context.Context, protoMetrics []*metr // ExportTraces implements otlp.ProtocolDriver. It transforms spans to // protobuf binary format and sends the result to the collector. -func (d *driver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { +func (d *driver) ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { if !d.tracesDriver.connection.connected() { return fmt.Errorf("traces exporter is disconnected from the server %s: %w", d.tracesDriver.connection.sCfg.Endpoint, d.tracesDriver.connection.lastConnectError()) } @@ -170,7 +170,7 @@ func (d *driver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) ctx, tCancel := context.WithTimeout(ctx, d.tracesDriver.connection.sCfg.Timeout) defer tCancel() - protoSpans := transform.SpanData(ss) + protoSpans := transform.Spans(ss) if len(protoSpans) == 0 { return nil } diff --git a/exporters/otlp/otlpgrpc/otlp_integration_test.go b/exporters/otlp/otlpgrpc/otlp_integration_test.go index 4eb7306faa0..abdd8b1666c 100644 --- a/exporters/otlp/otlpgrpc/otlp_integration_test.go +++ b/exporters/otlp/otlpgrpc/otlp_integration_test.go @@ -38,9 +38,12 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/internal/otlptest" "go.opentelemetry.io/otel/exporters/otlp/otlpgrpc" sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" commonpb "go.opentelemetry.io/proto/otlp/common/v1" ) +var roSpans = tracetest.SpanStubs{{Name: "Span 0"}}.Snapshots() + func TestNewExporter_endToEnd(t *testing.T) { tests := []struct { name string @@ -165,11 +168,11 @@ func TestNewExporter_collectorConnectionDiesThenReconnectsWhenInRestMode(t *test // first export, it will send disconnected message to the channel on export failure, // trigger almost immediate reconnection - require.Error(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "in the midst"}})) + require.Error(t, exp.ExportSpans(ctx, roSpans)) // second export, it will detect connection issue, change state of exporter to disconnected and // send message to disconnected channel but this time reconnection gouroutine will be in (rest mode, not listening to the disconnected channel) - require.Error(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "in the midst"}})) + require.Error(t, exp.ExportSpans(ctx, roSpans)) // as a result we have exporter in disconnected state waiting for disconnection message to reconnect @@ -184,7 +187,7 @@ func TestNewExporter_collectorConnectionDiesThenReconnectsWhenInRestMode(t *test for i := 0; i < n; i++ { // when disconnected exp.ExportSpans doesnt send disconnected messages again // it just quits and return last connection error - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Resurrected"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) } nmaSpans := nmc.getSpans() @@ -214,7 +217,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { { name: "Do not retry if succeeded", fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) span := mc.getSpans() @@ -228,7 +231,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { status.Error(codes.OK, ""), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) span := mc.getSpans() @@ -250,7 +253,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { status.Error(codes.Unavailable, "backend under pressure"), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) span := mc.getSpans() @@ -270,7 +273,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { status.Error(codes.InvalidArgument, "invalid arguments"), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - require.Error(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}})) + require.Error(t, exp.ExportSpans(ctx, roSpans)) span := mc.getSpans() @@ -296,7 +299,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { status.Error(codes.DataLoss, ""), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) span := mc.getSpans() @@ -319,7 +322,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { newThrottlingError(codes.ResourceExhausted, time.Second*30), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - err := exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}}) + err := exp.ExportSpans(ctx, roSpans) require.Error(t, err) require.Equal(t, "context deadline exceeded", err.Error()) @@ -341,7 +344,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { newThrottlingError(codes.ResourceExhausted, time.Minute), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - err := exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}}) + err := exp.ExportSpans(ctx, roSpans) require.Error(t, err) require.Equal(t, "max elapsed time expired when respecting server throttle: rpc error: code = ResourceExhausted desc = ", err.Error()) @@ -368,7 +371,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { status.Error(codes.Unavailable, "unavailable"), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - err := exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}}) + err := exp.ExportSpans(ctx, roSpans) require.Error(t, err) require.Equal(t, "max elapsed time expired: rpc error: code = Unavailable desc = unavailable", err.Error()) @@ -388,7 +391,7 @@ func TestExporterExportFailureAndRecoveryModes(t *testing.T) { status.Error(codes.Unavailable, "unavailable"), }, fn: func(t *testing.T, ctx context.Context, exp *otlp.Exporter, mc *mockCollector) { - err := exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}}) + err := exp.ExportSpans(ctx, roSpans) require.Error(t, err) require.Equal(t, "rpc error: code = Unavailable desc = unavailable", err.Error()) @@ -451,7 +454,7 @@ func TestPermanentErrorsShouldNotBeRetried(t *testing.T) { exp := newGRPCExporter(t, ctx, mc.endpoint) - err := exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Spans"}}) + err := exp.ExportSpans(ctx, roSpans) require.Error(t, err) require.Len(t, mc.getSpans(), 0) require.Equal(t, 1, mc.traceSvc.requests, "trace service must receive 1 permanent error requests.") @@ -492,7 +495,7 @@ func TestNewExporter_collectorConnectionDiesThenReconnects(t *testing.T) { for j := 0; j < 3; j++ { // No endpoint up. - require.Error(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "in the midst"}})) + require.Error(t, exp.ExportSpans(ctx, roSpans)) // Now resurrect the collector by making a new one but reusing the // old endpoint, and the collector should reconnect automatically. @@ -503,7 +506,7 @@ func TestNewExporter_collectorConnectionDiesThenReconnects(t *testing.T) { n := 10 for i := 0; i < n; i++ { - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "Resurrected"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) } nmaSpans := nmc.getSpans() @@ -565,7 +568,7 @@ func TestNewExporter_withHeaders(t *testing.T) { ctx := context.Background() exp := newGRPCExporter(t, ctx, mc.endpoint, otlpgrpc.WithHeaders(map[string]string{"header1": "value1"})) - require.NoError(t, exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "in the midst"}})) + require.NoError(t, exp.ExportSpans(ctx, roSpans)) defer func() { _ = exp.Shutdown(ctx) @@ -589,7 +592,7 @@ func TestNewExporter_WithTimeout(t *testing.T) { { name: "Timeout Spans", fn: func(exp *otlp.Exporter) error { - return exp.ExportSpans(context.Background(), []*sdktrace.SpanSnapshot{{Name: "timed out"}}) + return exp.ExportSpans(context.Background(), roSpans) }, timeout: time.Millisecond * 100, code: codes.DeadlineExceeded, @@ -608,7 +611,7 @@ func TestNewExporter_WithTimeout(t *testing.T) { { name: "No Timeout Spans", fn: func(exp *otlp.Exporter) error { - return exp.ExportSpans(context.Background(), []*sdktrace.SpanSnapshot{{Name: "timed out"}}) + return exp.ExportSpans(context.Background(), roSpans) }, timeout: time.Minute, spans: 1, @@ -673,7 +676,7 @@ func TestNewExporter_withInvalidSecurityConfiguration(t *testing.T) { t.Fatalf("failed to create a new collector exporter: %v", err) } - err = exp.ExportSpans(ctx, []*sdktrace.SpanSnapshot{{Name: "misconfiguration"}}) + err = exp.ExportSpans(ctx, roSpans) expectedErr := fmt.Sprintf("traces exporter is disconnected from the server %s: grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)", mc.endpoint) diff --git a/exporters/otlp/otlphttp/driver.go b/exporters/otlp/otlphttp/driver.go index eb47c25650f..6ad5d678c25 100644 --- a/exporters/otlp/otlphttp/driver.go +++ b/exporters/otlp/otlphttp/driver.go @@ -187,8 +187,8 @@ func (d *driver) ExportMetrics(ctx context.Context, cps metricsdk.CheckpointSet, } // ExportTraces implements otlp.ProtocolDriver. -func (d *driver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { - protoSpans := transform.SpanData(ss) +func (d *driver) ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { + protoSpans := transform.Spans(ss) if len(protoSpans) == 0 { return nil } diff --git a/exporters/otlp/protocoldriver.go b/exporters/otlp/protocoldriver.go index 209b970d69a..5d15b378ce5 100644 --- a/exporters/otlp/protocoldriver.go +++ b/exporters/otlp/protocoldriver.go @@ -48,7 +48,7 @@ type ProtocolDriver interface { // format and send it to the collector. May be called // concurrently with ExportMetrics, so the manager needs to // take this into account by doing proper locking. - ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error + ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error } // SplitConfig is used to configure a split driver. @@ -151,7 +151,7 @@ func (d *splitDriver) ExportMetrics(ctx context.Context, cps metricsdk.Checkpoin // ExportTraces implements ProtocolDriver. It forwards the call to the // driver used for sending spans. -func (d *splitDriver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { +func (d *splitDriver) ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { return d.trace.ExportTraces(ctx, ss) } @@ -171,6 +171,6 @@ func (d *noopDriver) ExportMetrics(ctx context.Context, cps metricsdk.Checkpoint } // ExportTraces does nothing. -func (d *noopDriver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { +func (d *noopDriver) ExportTraces(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { return nil } diff --git a/exporters/stdout/trace.go b/exporters/stdout/trace.go index fbbee19928b..9720ffd3f47 100644 --- a/exporters/stdout/trace.go +++ b/exporters/stdout/trace.go @@ -21,6 +21,7 @@ import ( "sync" "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" ) // Exporter is an implementation of trace.SpanSyncer that writes spans to stdout. @@ -32,7 +33,7 @@ type traceExporter struct { } // ExportSpans writes SpanSnapshots in json format to stdout. -func (e *traceExporter) ExportSpans(ctx context.Context, ss []*trace.SpanSnapshot) error { +func (e *traceExporter) ExportSpans(ctx context.Context, ss []trace.ReadOnlySpan) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -43,7 +44,7 @@ func (e *traceExporter) ExportSpans(ctx context.Context, ss []*trace.SpanSnapsho if e.config.DisableTraceExport || len(ss) == 0 { return nil } - out, err := e.marshal(ss) + out, err := e.marshal(tracetest.SpanStubsFromReadOnlySpans(ss)) if err != nil { return err } diff --git a/exporters/stdout/trace_test.go b/exporters/stdout/trace_test.go index ccd6b497ce0..8b60790dfcb 100644 --- a/exporters/stdout/trace_test.go +++ b/exporters/stdout/trace_test.go @@ -22,18 +22,21 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/exporters/stdout" "go.opentelemetry.io/otel/sdk/resource" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" ) func TestExporter_ExportSpan(t *testing.T) { // write to buffer for testing var b bytes.Buffer - ex, err := stdout.NewExporter(stdout.WithWriter(&b)) + ex, err := stdout.NewExporter(stdout.WithWriter(&b), stdout.WithPrettyPrint()) if err != nil { t.Errorf("Error constructing stdout exporter %s", err) } @@ -47,108 +50,139 @@ func TestExporter_ExportSpan(t *testing.T) { doubleValue := 123.456 resource := resource.NewWithAttributes(attribute.String("rk1", "rv11")) - testSpan := &tracesdk.SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: traceID, - SpanID: spanID, - TraceState: traceState, - }), - Name: "/foo", - StartTime: now, - EndTime: now, - Attributes: []attribute.KeyValue{ - attribute.String("key", keyValue), - attribute.Float64("double", doubleValue), - }, - Events: []tracesdk.Event{ - {Name: "foo", Attributes: []attribute.KeyValue{attribute.String("key", keyValue)}, Time: now}, - {Name: "bar", Attributes: []attribute.KeyValue{attribute.Float64("double", doubleValue)}, Time: now}, + ro := tracetest.SpanStubs{ + { + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceState: traceState, + }), + Name: "/foo", + StartTime: now, + EndTime: now, + Attributes: []attribute.KeyValue{ + attribute.String("key", keyValue), + attribute.Float64("double", doubleValue), + }, + Events: []tracesdk.Event{ + {Name: "foo", Attributes: []attribute.KeyValue{attribute.String("key", keyValue)}, Time: now}, + {Name: "bar", Attributes: []attribute.KeyValue{attribute.Float64("double", doubleValue)}, Time: now}, + }, + SpanKind: trace.SpanKindInternal, + Status: tracesdk.Status{ + Code: codes.Error, + Description: "interesting", + }, + Resource: resource, }, - SpanKind: trace.SpanKindInternal, - Status: tracesdk.Status{ - Code: codes.Error, - Description: "interesting", - }, - Resource: resource, - } - if err := ex.ExportSpans(context.Background(), []*tracesdk.SpanSnapshot{testSpan}); err != nil { + }.Snapshots() + if err := ex.ExportSpans(context.Background(), ro); err != nil { t.Fatal(err) } expectedSerializedNow, _ := json.Marshal(now) got := b.String() - expectedOutput := `[{"SpanContext":{` + - `"TraceID":"0102030405060708090a0b0c0d0e0f10",` + - `"SpanID":"0102030405060708","TraceFlags":"00",` + - `"TraceState":[` + - `{` + - `"Key":"key",` + - `"Value":{"Type":"STRING","Value":"val"}` + - `}],"Remote":false},` + - `"Parent":{` + - `"TraceID":"00000000000000000000000000000000",` + - `"SpanID":"0000000000000000",` + - `"TraceFlags":"00",` + - `"TraceState":null,` + - `"Remote":false` + - `},` + - `"SpanKind":1,` + - `"Name":"/foo",` + - `"StartTime":` + string(expectedSerializedNow) + "," + - `"EndTime":` + string(expectedSerializedNow) + "," + - `"Attributes":[` + - `{` + - `"Key":"key",` + - `"Value":{"Type":"STRING","Value":"value"}` + - `},` + - `{` + - `"Key":"double",` + - `"Value":{"Type":"FLOAT64","Value":123.456}` + - `}],` + - `"Events":[` + - `{` + - `"Name":"foo",` + - `"Attributes":[` + - `{` + - `"Key":"key",` + - `"Value":{"Type":"STRING","Value":"value"}` + - `}` + - `],` + - `"DroppedAttributeCount":0,` + - `"Time":` + string(expectedSerializedNow) + - `},` + - `{` + - `"Name":"bar",` + - `"Attributes":[` + - `{` + - `"Key":"double",` + - `"Value":{"Type":"FLOAT64","Value":123.456}` + - `}` + - `],` + - `"DroppedAttributeCount":0,` + - `"Time":` + string(expectedSerializedNow) + - `}` + - `],` + - `"Links":null,` + - `"Status":{"Code":"Error","Description":"interesting"},` + - `"DroppedAttributeCount":0,` + - `"DroppedEventCount":0,` + - `"DroppedLinkCount":0,` + - `"ChildSpanCount":0,` + - `"Resource":[` + - `{` + - `"Key":"rk1",` + - `"Value":{"Type":"STRING","Value":"rv11"}` + - `}],` + - `"InstrumentationLibrary":{` + - `"Name":"",` + - `"Version":""` + - `}}]` + "\n" - - if got != expectedOutput { - t.Errorf("Want: %v but got: %v", expectedOutput, got) + expectedOutput := `[ + { + "Name": "/foo", + "SpanContext": { + "TraceID": "0102030405060708090a0b0c0d0e0f10", + "SpanID": "0102030405060708", + "TraceFlags": "00", + "TraceState": [ + { + "Key": "key", + "Value": { + "Type": "STRING", + "Value": "val" + } + } + ], + "Remote": false + }, + "Parent": { + "TraceID": "00000000000000000000000000000000", + "SpanID": "0000000000000000", + "TraceFlags": "00", + "TraceState": null, + "Remote": false + }, + "SpanKind": 1, + "StartTime": ` + string(expectedSerializedNow) + `, + "EndTime": ` + string(expectedSerializedNow) + `, + "Attributes": [ + { + "Key": "key", + "Value": { + "Type": "STRING", + "Value": "value" + } + }, + { + "Key": "double", + "Value": { + "Type": "FLOAT64", + "Value": 123.456 + } + } + ], + "Events": [ + { + "Name": "foo", + "Attributes": [ + { + "Key": "key", + "Value": { + "Type": "STRING", + "Value": "value" + } + } + ], + "DroppedAttributeCount": 0, + "Time": ` + string(expectedSerializedNow) + ` + }, + { + "Name": "bar", + "Attributes": [ + { + "Key": "double", + "Value": { + "Type": "FLOAT64", + "Value": 123.456 + } + } + ], + "DroppedAttributeCount": 0, + "Time": ` + string(expectedSerializedNow) + ` + } + ], + "Links": null, + "Status": { + "Code": "Error", + "Description": "interesting" + }, + "DroppedAttributes": 0, + "DroppedEvents": 0, + "DroppedLinks": 0, + "ChildSpanCount": 0, + "Resource": [ + { + "Key": "rk1", + "Value": { + "Type": "STRING", + "Value": "rv11" + } + } + ], + "InstrumentationLibrary": { + "Name": "", + "Version": "" + } } +] +` + assert.Equal(t, expectedOutput, got) } func TestExporterShutdownHonorsTimeout(t *testing.T) { diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index 8fa55a895f3..ed087334b10 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -104,7 +104,7 @@ type Exporter struct { var _ sdktrace.SpanExporter = (*Exporter)(nil) // ExportSpans transforms and exports OpenTelemetry spans to Jaeger. -func (e *Exporter) ExportSpans(ctx context.Context, spans []*sdktrace.SpanSnapshot) error { +func (e *Exporter) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { // Return fast if context is already canceled or Exporter shutdown. select { case <-ctx.Done(): @@ -148,41 +148,42 @@ func (e *Exporter) Shutdown(ctx context.Context) error { return e.uploader.shutdown(ctx) } -func spanSnapshotToThrift(ss *sdktrace.SpanSnapshot) *gen.Span { - tags := make([]*gen.Tag, 0, len(ss.Attributes)) - for _, kv := range ss.Attributes { +func spanToThrift(ss sdktrace.ReadOnlySpan) *gen.Span { + attr := ss.Attributes() + tags := make([]*gen.Tag, 0, len(attr)) + for _, kv := range attr { tag := keyValueToTag(kv) if tag != nil { tags = append(tags, tag) } } - if il := ss.InstrumentationLibrary; il.Name != "" { + if il := ss.InstrumentationLibrary(); il.Name != "" { tags = append(tags, getStringTag(keyInstrumentationLibraryName, il.Name)) if il.Version != "" { tags = append(tags, getStringTag(keyInstrumentationLibraryVersion, il.Version)) } } - if ss.SpanKind != trace.SpanKindInternal { + if ss.SpanKind() != trace.SpanKindInternal { tags = append(tags, - getStringTag(keySpanKind, ss.SpanKind.String()), + getStringTag(keySpanKind, ss.SpanKind().String()), ) } - if ss.Status.Code != codes.Unset { - tags = append(tags, getInt64Tag(keyStatusCode, int64(ss.Status.Code))) - if ss.Status.Description != "" { - tags = append(tags, getStringTag(keyStatusMessage, ss.Status.Description)) + if ss.Status().Code != codes.Unset { + tags = append(tags, getInt64Tag(keyStatusCode, int64(ss.Status().Code))) + if ss.Status().Description != "" { + tags = append(tags, getStringTag(keyStatusMessage, ss.Status().Description)) } - if ss.Status.Code == codes.Error { + if ss.Status().Code == codes.Error { tags = append(tags, getBoolTag(keyError, true)) } } var logs []*gen.Log - for _, a := range ss.Events { + for _, a := range ss.Events() { nTags := len(a.Attributes) if a.Name != "" { nTags++ @@ -212,7 +213,7 @@ func spanSnapshotToThrift(ss *sdktrace.SpanSnapshot) *gen.Span { } var refs []*gen.SpanRef - for _, link := range ss.Links { + for _, link := range ss.Links() { tid := link.TraceID() sid := link.SpanID() refs = append(refs, &gen.SpanRef{ @@ -223,18 +224,18 @@ func spanSnapshotToThrift(ss *sdktrace.SpanSnapshot) *gen.Span { }) } - tid := ss.SpanContext.TraceID() - sid := ss.SpanContext.SpanID() - psid := ss.Parent.SpanID() + tid := ss.SpanContext().TraceID() + sid := ss.SpanContext().SpanID() + psid := ss.Parent().SpanID() return &gen.Span{ TraceIdHigh: int64(binary.BigEndian.Uint64(tid[0:8])), TraceIdLow: int64(binary.BigEndian.Uint64(tid[8:16])), SpanId: int64(binary.BigEndian.Uint64(sid[:])), ParentSpanId: int64(binary.BigEndian.Uint64(psid[:])), - OperationName: ss.Name, // TODO: if span kind is added then add prefix "Sent"/"Recv" - Flags: int32(ss.SpanContext.TraceFlags()), - StartTime: ss.StartTime.UnixNano() / 1000, - Duration: ss.EndTime.Sub(ss.StartTime).Nanoseconds() / 1000, + OperationName: ss.Name(), // TODO: if span kind is added then add prefix "Sent"/"Recv" + Flags: int32(ss.SpanContext().TraceFlags()), + StartTime: ss.StartTime().UnixNano() / 1000, + Duration: ss.EndTime().Sub(ss.StartTime()).Nanoseconds() / 1000, Tags: tags, Logs: logs, References: refs, @@ -308,9 +309,8 @@ func getBoolTag(k string, b bool) *gen.Tag { } } -// jaegerBatchList transforms a slice of SpanSnapshot into a slice of jaeger -// Batch. -func jaegerBatchList(ssl []*sdktrace.SpanSnapshot, defaultServiceName string) []*gen.Batch { +// jaegerBatchList transforms a slice of spans into a slice of jaeger Batch. +func jaegerBatchList(ssl []sdktrace.ReadOnlySpan, defaultServiceName string) []*gen.Batch { if len(ssl) == 0 { return nil } @@ -322,15 +322,15 @@ func jaegerBatchList(ssl []*sdktrace.SpanSnapshot, defaultServiceName string) [] continue } - resourceKey := ss.Resource.Equivalent() + resourceKey := ss.Resource().Equivalent() batch, bOK := batchDict[resourceKey] if !bOK { batch = &gen.Batch{ - Process: process(ss.Resource, defaultServiceName), + Process: process(ss.Resource(), defaultServiceName), Spans: []*gen.Span{}, } } - batch.Spans = append(batch.Spans, spanSnapshotToThrift(ss)) + batch.Spans = append(batch.Spans, spanToThrift(ss)) batchDict[resourceKey] = batch } diff --git a/exporters/trace/jaeger/jaeger_benchmark_test.go b/exporters/trace/jaeger/jaeger_benchmark_test.go index 212279dc201..92a11d35941 100644 --- a/exporters/trace/jaeger/jaeger_benchmark_test.go +++ b/exporters/trace/jaeger/jaeger_benchmark_test.go @@ -22,6 +22,7 @@ import ( "go.opentelemetry.io/otel/sdk/instrumentation" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" ) @@ -49,12 +50,12 @@ func init() { }) } -func spans(n int) []*tracesdk.SpanSnapshot { +func spans(n int) []tracesdk.ReadOnlySpan { now := time.Now() - s := make([]*tracesdk.SpanSnapshot, n) + s := make(tracetest.SpanStubs, n) for i := 0; i < n; i++ { name := fmt.Sprintf("span %d", i) - s[i] = &tracesdk.SpanSnapshot{ + s[i] = tracetest.SpanStub{ SpanContext: spanContext, Name: name, StartTime: now, @@ -65,7 +66,7 @@ func spans(n int) []*tracesdk.SpanSnapshot { }, } } - return s + return s.Snapshots() } func benchmarkExportSpans(b *testing.B, o EndpointOption, i int) { diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index 39f6f592b4e..8156c1fc8a2 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -36,6 +36,7 @@ import ( "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/semconv" "go.opentelemetry.io/otel/trace" ) @@ -219,12 +220,12 @@ func Test_spanSnapshotToThrift(t *testing.T) { tests := []struct { name string - data *sdktrace.SpanSnapshot + data tracetest.SpanStub want *gen.Span }{ { name: "no status description", - data: &sdktrace.SpanSnapshot{ + data: tracetest.SpanStub{ SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, @@ -258,7 +259,7 @@ func Test_spanSnapshotToThrift(t *testing.T) { }, { name: "no parent", - data: &sdktrace.SpanSnapshot{ + data: tracetest.SpanStub{ SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, @@ -349,7 +350,7 @@ func Test_spanSnapshotToThrift(t *testing.T) { }, { name: "with parent", - data: &sdktrace.SpanSnapshot{ + data: tracetest.SpanStub{ SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, @@ -408,7 +409,7 @@ func Test_spanSnapshotToThrift(t *testing.T) { }, { name: "resources do not affect the tags", - data: &sdktrace.SpanSnapshot{ + data: tracetest.SpanStub{ SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, @@ -452,7 +453,7 @@ func Test_spanSnapshotToThrift(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := spanSnapshotToThrift(tt.data) + got := spanToThrift(tt.data.Snapshot()) sort.Slice(got.Tags, func(i, j int) bool { return got.Tags[i].Key < got.Tags[j].Key }) @@ -496,7 +497,7 @@ func TestExporterExportSpansHonorsCancel(t *testing.T) { e, err := NewRawExporter(withTestCollectorEndpoint()) require.NoError(t, err) now := time.Now() - ss := []*sdktrace.SpanSnapshot{ + ss := tracetest.SpanStubs{ { Name: "s1", Resource: resource.NewWithAttributes( @@ -519,14 +520,14 @@ func TestExporterExportSpansHonorsCancel(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - assert.EqualError(t, e.ExportSpans(ctx, ss), context.Canceled.Error()) + assert.EqualError(t, e.ExportSpans(ctx, ss.Snapshots()), context.Canceled.Error()) } func TestExporterExportSpansHonorsTimeout(t *testing.T) { e, err := NewRawExporter(withTestCollectorEndpoint()) require.NoError(t, err) now := time.Now() - ss := []*sdktrace.SpanSnapshot{ + ss := tracetest.SpanStubs{ { Name: "s1", Resource: resource.NewWithAttributes( @@ -550,7 +551,7 @@ func TestExporterExportSpansHonorsTimeout(t *testing.T) { defer cancel() <-ctx.Done() - assert.EqualError(t, e.ExportSpans(ctx, ss), context.DeadlineExceeded.Error()) + assert.EqualError(t, e.ExportSpans(ctx, ss.Snapshots()), context.DeadlineExceeded.Error()) } func TestJaegerBatchList(t *testing.T) { @@ -562,19 +563,19 @@ func TestJaegerBatchList(t *testing.T) { testCases := []struct { name string - spanSnapshotList []*sdktrace.SpanSnapshot + roSpans []sdktrace.ReadOnlySpan defaultServiceName string expectedBatchList []*gen.Batch }{ { name: "no span shots", - spanSnapshotList: nil, + roSpans: nil, expectedBatchList: nil, }, { name: "span's snapshot contains nil span", - spanSnapshotList: []*sdktrace.SpanSnapshot{ - { + roSpans: []sdktrace.ReadOnlySpan{ + tracetest.SpanStub{ Name: "s1", Resource: resource.NewWithAttributes( semconv.ServiceNameKey.String("name"), @@ -582,7 +583,7 @@ func TestJaegerBatchList(t *testing.T) { ), StartTime: now, EndTime: now, - }, + }.Snapshot(), nil, }, expectedBatchList: []*gen.Batch{ @@ -607,7 +608,7 @@ func TestJaegerBatchList(t *testing.T) { }, { name: "merge spans that have the same resources", - spanSnapshotList: []*sdktrace.SpanSnapshot{ + roSpans: tracetest.SpanStubs{ { Name: "s1", Resource: resource.NewWithAttributes( @@ -635,7 +636,7 @@ func TestJaegerBatchList(t *testing.T) { StartTime: now, EndTime: now, }, - }, + }.Snapshots(), expectedBatchList: []*gen.Batch{ { Process: &gen.Process{ @@ -682,7 +683,7 @@ func TestJaegerBatchList(t *testing.T) { }, { name: "no service name in spans", - spanSnapshotList: []*sdktrace.SpanSnapshot{ + roSpans: tracetest.SpanStubs{ { Name: "s1", Resource: resource.NewWithAttributes( @@ -691,8 +692,7 @@ func TestJaegerBatchList(t *testing.T) { StartTime: now, EndTime: now, }, - nil, - }, + }.Snapshots(), defaultServiceName: "default service name", expectedBatchList: []*gen.Batch{ { @@ -718,7 +718,7 @@ func TestJaegerBatchList(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - batchList := jaegerBatchList(tc.spanSnapshotList, tc.defaultServiceName) + batchList := jaegerBatchList(tc.roSpans, tc.defaultServiceName) assert.ElementsMatch(t, tc.expectedBatchList, batchList) }) diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go index a83e1a0179d..bdd48d4f89b 100644 --- a/exporters/trace/zipkin/model.go +++ b/exporters/trace/zipkin/model.go @@ -51,7 +51,7 @@ func init() { } } -func toZipkinSpanModels(batch []*tracesdk.SpanSnapshot) []zkmodel.SpanModel { +func toZipkinSpanModels(batch []tracesdk.ReadOnlySpan) []zkmodel.SpanModel { models := make([]zkmodel.SpanModel, 0, len(batch)) for _, data := range batch { models = append(models, toZipkinSpanModel(data)) @@ -69,28 +69,28 @@ func getServiceName(attrs []attribute.KeyValue) string { return defaultServiceName } -func toZipkinSpanModel(data *tracesdk.SpanSnapshot) zkmodel.SpanModel { +func toZipkinSpanModel(data tracesdk.ReadOnlySpan) zkmodel.SpanModel { return zkmodel.SpanModel{ SpanContext: toZipkinSpanContext(data), - Name: data.Name, - Kind: toZipkinKind(data.SpanKind), - Timestamp: data.StartTime, - Duration: data.EndTime.Sub(data.StartTime), + Name: data.Name(), + Kind: toZipkinKind(data.SpanKind()), + Timestamp: data.StartTime(), + Duration: data.EndTime().Sub(data.StartTime()), Shared: false, LocalEndpoint: &zkmodel.Endpoint{ - ServiceName: getServiceName(data.Resource.Attributes()), + ServiceName: getServiceName(data.Resource().Attributes()), }, RemoteEndpoint: toZipkinRemoteEndpoint(data), - Annotations: toZipkinAnnotations(data.Events), + Annotations: toZipkinAnnotations(data.Events()), Tags: toZipkinTags(data), } } -func toZipkinSpanContext(data *tracesdk.SpanSnapshot) zkmodel.SpanContext { +func toZipkinSpanContext(data tracesdk.ReadOnlySpan) zkmodel.SpanContext { return zkmodel.SpanContext{ - TraceID: toZipkinTraceID(data.SpanContext.TraceID()), - ID: toZipkinID(data.SpanContext.SpanID()), - ParentID: toZipkinParentID(data.Parent.SpanID()), + TraceID: toZipkinTraceID(data.SpanContext().TraceID()), + ID: toZipkinID(data.SpanContext().SpanID()), + ParentID: toZipkinParentID(data.Parent().SpanID()), Debug: false, Sampled: nil, Err: nil, @@ -174,9 +174,10 @@ var extraZipkinTags = []string{ keyInstrumentationLibraryVersion, } -func toZipkinTags(data *tracesdk.SpanSnapshot) map[string]string { - m := make(map[string]string, len(data.Attributes)+len(extraZipkinTags)) - for _, kv := range data.Attributes { +func toZipkinTags(data tracesdk.ReadOnlySpan) map[string]string { + attr := data.Attributes() + m := make(map[string]string, len(attr)+len(extraZipkinTags)) + for _, kv := range attr { switch kv.Value.Type() { // For array attributes, serialize as JSON list string. case attribute.ARRAY: @@ -187,17 +188,17 @@ func toZipkinTags(data *tracesdk.SpanSnapshot) map[string]string { } } - if data.Status.Code != codes.Unset { - m["otel.status_code"] = data.Status.Code.String() + if data.Status().Code != codes.Unset { + m["otel.status_code"] = data.Status().Code.String() } - if data.Status.Code == codes.Error { - m["error"] = data.Status.Description + if data.Status().Code == codes.Error { + m["error"] = data.Status().Description } else { delete(m, "error") } - if il := data.InstrumentationLibrary; il.Name != "" { + if il := data.InstrumentationLibrary(); il.Name != "" { m[keyInstrumentationLibraryName] = il.Name if il.Version != "" { m[keyInstrumentationLibraryVersion] = il.Version @@ -223,15 +224,15 @@ var remoteEndpointKeyRank = map[attribute.Key]int{ semconv.DBNameKey: 6, } -func toZipkinRemoteEndpoint(data *sdktrace.SpanSnapshot) *zkmodel.Endpoint { +func toZipkinRemoteEndpoint(data sdktrace.ReadOnlySpan) *zkmodel.Endpoint { // Should be set only for client or producer kind - if data.SpanKind != trace.SpanKindClient && - data.SpanKind != trace.SpanKindProducer { + if sk := data.SpanKind(); sk != trace.SpanKindClient && sk != trace.SpanKindProducer { return nil } + attr := data.Attributes() var endpointAttr attribute.KeyValue - for _, kv := range data.Attributes { + for _, kv := range attr { rank, ok := remoteEndpointKeyRank[kv.Key] if !ok { continue @@ -256,7 +257,7 @@ func toZipkinRemoteEndpoint(data *sdktrace.SpanSnapshot) *zkmodel.Endpoint { } } - return remoteEndpointPeerIPWithPort(endpointAttr.Value.AsString(), data.Attributes) + return remoteEndpointPeerIPWithPort(endpointAttr.Value.AsString(), attr) } // Handles `net.peer.ip` remote endpoint separately (should include `net.peer.ip` diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go index 6e2ab83f23a..3be87a01635 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -32,6 +32,7 @@ import ( "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/semconv" "go.opentelemetry.io/otel/trace" ) @@ -41,7 +42,7 @@ func TestModelConversion(t *testing.T) { semconv.ServiceNameKey.String("model-test"), ) - inputBatch := []*tracesdk.SpanSnapshot{ + inputBatch := tracetest.SpanStubs{ // typical span data { SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ @@ -367,7 +368,7 @@ func TestModelConversion(t *testing.T) { }, Resource: resource, }, - } + }.Snapshots() expectedOutputBatch := []zkmodel.SpanModel{ // model for typical span data @@ -734,12 +735,12 @@ func TestTagsTransformation(t *testing.T) { tests := []struct { name string - data *tracesdk.SpanSnapshot + data tracetest.SpanStub want map[string]string }{ { name: "attributes", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ Attributes: []attribute.KeyValue{ attribute.String("key", keyValue), attribute.Float64("double", doubleValue), @@ -756,12 +757,12 @@ func TestTagsTransformation(t *testing.T) { }, { name: "no attributes", - data: &tracesdk.SpanSnapshot{}, + data: tracetest.SpanStub{}, want: nil, }, { name: "omit-noerror", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ Attributes: []attribute.KeyValue{ attribute.Bool("error", false), }, @@ -770,7 +771,7 @@ func TestTagsTransformation(t *testing.T) { }, { name: "statusCode", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ Attributes: []attribute.KeyValue{ attribute.String("key", keyValue), attribute.Bool("error", true), @@ -788,14 +789,14 @@ func TestTagsTransformation(t *testing.T) { }, { name: "instrLib-empty", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ InstrumentationLibrary: instrumentation.Library{}, }, want: nil, }, { name: "instrLib-noversion", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ Attributes: []attribute.KeyValue{}, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, @@ -807,7 +808,7 @@ func TestTagsTransformation(t *testing.T) { }, { name: "instrLib-with-version", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ Attributes: []attribute.KeyValue{}, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, @@ -822,7 +823,7 @@ func TestTagsTransformation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := toZipkinTags(tt.data) + got := toZipkinTags(tt.data.Snapshot()) if diff := cmp.Diff(got, tt.want); diff != "" { t.Errorf("Diff%v", diff) } @@ -833,12 +834,12 @@ func TestTagsTransformation(t *testing.T) { func TestRemoteEndpointTransformation(t *testing.T) { tests := []struct { name string - data *tracesdk.SpanSnapshot + data tracetest.SpanStub want *zkmodel.Endpoint }{ { name: "nil-not-applicable", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindClient, Attributes: []attribute.KeyValue{}, }, @@ -846,7 +847,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "nil-not-found", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindConsumer, Attributes: []attribute.KeyValue{ attribute.String("attr", "test"), @@ -856,7 +857,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "peer-service-rank", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ semconv.PeerServiceKey.String("peer-service-test"), @@ -870,7 +871,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "http-host-rank", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ semconv.HTTPHostKey.String("http-host-test"), @@ -883,7 +884,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "db-name-rank", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ attribute.String("foo", "bar"), @@ -896,7 +897,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "peer-hostname-rank", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ keyPeerHostname.String("peer-hostname-test"), @@ -911,7 +912,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "peer-address-rank", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ keyPeerAddress.String("peer-address-test"), @@ -925,7 +926,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "net-peer-invalid-ip", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ semconv.NetPeerIPKey.String("INVALID"), @@ -935,7 +936,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "net-peer-ipv6-no-port", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ semconv.NetPeerIPKey.String("0:0:1:5ee:bad:c0de:0:0"), @@ -947,7 +948,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { }, { name: "net-peer-ipv4-port", - data: &tracesdk.SpanSnapshot{ + data: tracetest.SpanStub{ SpanKind: trace.SpanKindProducer, Attributes: []attribute.KeyValue{ semconv.NetPeerIPKey.String("1.2.3.4"), @@ -962,7 +963,7 @@ func TestRemoteEndpointTransformation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := toZipkinRemoteEndpoint(tt.data) + got := toZipkinRemoteEndpoint(tt.data.Snapshot()) if diff := cmp.Diff(got, tt.want); diff != "" { t.Errorf("Diff%v", diff) } diff --git a/exporters/trace/zipkin/zipkin.go b/exporters/trace/zipkin/zipkin.go index bdfe1cd95b1..ab756abfdd8 100644 --- a/exporters/trace/zipkin/zipkin.go +++ b/exporters/trace/zipkin/zipkin.go @@ -134,7 +134,7 @@ func InstallNewPipeline(collectorURL string, opts ...Option) error { } // ExportSpans exports SpanSnapshots to a Zipkin receiver. -func (e *Exporter) ExportSpans(ctx context.Context, ss []*sdktrace.SpanSnapshot) error { +func (e *Exporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index 739ad193621..be7cf2ae454 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/exporters/trace/zipkin/zipkin_test.go @@ -34,6 +34,7 @@ import ( "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/semconv" "go.opentelemetry.io/otel/trace" ) @@ -233,7 +234,7 @@ func TestExportSpans(t *testing.T) { semconv.ServiceNameKey.String("exporter-test"), ) - spans := []*sdktrace.SpanSnapshot{ + spans := tracetest.SpanStubs{ // parent { SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ @@ -274,7 +275,7 @@ func TestExportSpans(t *testing.T) { }, Resource: resource, }, - } + }.Snapshots() models := []zkmodel.SpanModel{ // model of parent { diff --git a/sdk/trace/batch_span_processor.go b/sdk/trace/batch_span_processor.go index 6687839ee43..fbda5872465 100644 --- a/sdk/trace/batch_span_processor.go +++ b/sdk/trace/batch_span_processor.go @@ -68,10 +68,10 @@ type batchSpanProcessor struct { e SpanExporter o BatchSpanProcessorOptions - queue chan *SpanSnapshot + queue chan ReadOnlySpan dropped uint32 - batch []*SpanSnapshot + batch []ReadOnlySpan batchMutex sync.Mutex timer *time.Timer stopWait sync.WaitGroup @@ -98,9 +98,9 @@ func NewBatchSpanProcessor(exporter SpanExporter, options ...BatchSpanProcessorO bsp := &batchSpanProcessor{ e: exporter, o: o, - batch: make([]*SpanSnapshot, 0, o.MaxExportBatchSize), + batch: make([]ReadOnlySpan, 0, o.MaxExportBatchSize), timer: time.NewTimer(o.BatchTimeout), - queue: make(chan *SpanSnapshot, o.MaxQueueSize), + queue: make(chan ReadOnlySpan, o.MaxQueueSize), stopCh: make(chan struct{}), } @@ -123,7 +123,7 @@ func (bsp *batchSpanProcessor) OnEnd(s ReadOnlySpan) { if bsp.e == nil { return } - bsp.enqueue(s.Snapshot()) + bsp.enqueue(s) } // Shutdown flushes the queue and waits until all spans are processed. @@ -294,8 +294,8 @@ func (bsp *batchSpanProcessor) drainQueue() { } } -func (bsp *batchSpanProcessor) enqueue(sd *SpanSnapshot) { - if !sd.SpanContext.IsSampled() { +func (bsp *batchSpanProcessor) enqueue(sd ReadOnlySpan) { + if !sd.SpanContext().IsSampled() { return } diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go index 0cea6e29468..6aae90bf9c3 100644 --- a/sdk/trace/batch_span_processor_test.go +++ b/sdk/trace/batch_span_processor_test.go @@ -32,7 +32,7 @@ import ( type testBatchExporter struct { mu sync.Mutex - spans []*sdktrace.SpanSnapshot + spans []sdktrace.ReadOnlySpan sizes []int batchCount int shutdownCount int @@ -43,7 +43,7 @@ type testBatchExporter struct { err error } -func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []*sdktrace.SpanSnapshot) error { +func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error { t.mu.Lock() defer t.mu.Unlock() @@ -421,7 +421,7 @@ func assertMaxSpanDiff(t *testing.T, want, got, maxDif int) { type indefiniteExporter struct{} func (indefiniteExporter) Shutdown(context.Context) error { return nil } -func (indefiniteExporter) ExportSpans(ctx context.Context, _ []*sdktrace.SpanSnapshot) error { +func (indefiniteExporter) ExportSpans(ctx context.Context, _ []sdktrace.ReadOnlySpan) error { <-ctx.Done() return ctx.Err() } diff --git a/sdk/trace/simple_span_processor.go b/sdk/trace/simple_span_processor.go index 5b935e6a6af..9dc87f893b9 100644 --- a/sdk/trace/simple_span_processor.go +++ b/sdk/trace/simple_span_processor.go @@ -55,8 +55,7 @@ func (ssp *simpleSpanProcessor) OnEnd(s ReadOnlySpan) { defer ssp.exporterMu.RUnlock() if ssp.exporter != nil && s.SpanContext().TraceFlags().IsSampled() { - ss := s.Snapshot() - if err := ssp.exporter.ExportSpans(context.Background(), []*SpanSnapshot{ss}); err != nil { + if err := ssp.exporter.ExportSpans(context.Background(), []ReadOnlySpan{s}); err != nil { otel.Handle(err) } } diff --git a/sdk/trace/simple_span_processor_test.go b/sdk/trace/simple_span_processor_test.go index bfdb1eecb9d..c9b04763feb 100644 --- a/sdk/trace/simple_span_processor_test.go +++ b/sdk/trace/simple_span_processor_test.go @@ -31,11 +31,11 @@ var ( ) type testExporter struct { - spans []*sdktrace.SpanSnapshot + spans []sdktrace.ReadOnlySpan shutdown bool } -func (t *testExporter) ExportSpans(ctx context.Context, ss []*sdktrace.SpanSnapshot) error { +func (t *testExporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error { t.spans = append(t.spans, ss...) return nil } @@ -80,7 +80,7 @@ func TestSimpleSpanProcessorOnEnd(t *testing.T) { startSpan(tp).End() wantTraceID := tid - gotTraceID := te.spans[0].SpanContext.TraceID() + gotTraceID := te.spans[0].SpanContext().TraceID() if wantTraceID != gotTraceID { t.Errorf("SimplerSpanProcessor OnEnd() check: got %+v, want %+v\n", gotTraceID, wantTraceID) } diff --git a/sdk/trace/snapshot.go b/sdk/trace/snapshot.go index e475b5a46d6..e6a96290779 100644 --- a/sdk/trace/snapshot.go +++ b/sdk/trace/snapshot.go @@ -28,32 +28,122 @@ import ( // Although SpanSnapshot fields can be accessed and potentially modified, // SpanSnapshot should be treated as immutable. Changes to the span from which // the SpanSnapshot was created are NOT reflected in the SpanSnapshot. +// +// TODO: unexport and rename to snapshot. +// TODO: clean project docs of this type after rename. type SpanSnapshot struct { - SpanContext trace.SpanContext - Parent trace.SpanContext - SpanKind trace.SpanKind - Name string - StartTime time.Time - // The wall clock time of EndTime will be adjusted to always be offset - // from StartTime by the duration of the span. - EndTime time.Time - Attributes []attribute.KeyValue - Events []Event - Links []trace.Link - Status Status - - // DroppedAttributeCount contains dropped attributes for the span itself. - DroppedAttributeCount int - DroppedEventCount int - DroppedLinkCount int - - // ChildSpanCount holds the number of child span created for this span. - ChildSpanCount int - - // Resource contains attributes representing an entity that produced this span. - Resource *resource.Resource - - // InstrumentationLibrary defines the instrumentation library used to - // provide instrumentation. - InstrumentationLibrary instrumentation.Library + name string + spanContext trace.SpanContext + parent trace.SpanContext + spanKind trace.SpanKind + startTime time.Time + endTime time.Time + attributes []attribute.KeyValue + events []Event + links []trace.Link + status Status + childSpanCount int + droppedAttributeCount int + droppedEventCount int + droppedLinkCount int + resource *resource.Resource + instrumentationLibrary instrumentation.Library +} + +var _ ReadOnlySpan = SpanSnapshot{} + +func (s SpanSnapshot) private() {} + +// Name returns the name of the span. +func (s SpanSnapshot) Name() string { + return s.name +} + +// SpanContext returns the unique SpanContext that identifies the span. +func (s SpanSnapshot) SpanContext() trace.SpanContext { + return s.spanContext +} + +// Parent returns the unique SpanContext that identifies the parent of the +// span if one exists. If the span has no parent the returned SpanContext +// will be invalid. +func (s SpanSnapshot) Parent() trace.SpanContext { + return s.parent +} + +// SpanKind returns the role the span plays in a Trace. +func (s SpanSnapshot) SpanKind() trace.SpanKind { + return s.spanKind +} + +// StartTime returns the time the span started recording. +func (s SpanSnapshot) StartTime() time.Time { + return s.startTime +} + +// EndTime returns the time the span stopped recording. It will be zero if +// the span has not ended. +func (s SpanSnapshot) EndTime() time.Time { + return s.endTime +} + +// Attributes returns the defining attributes of the span. +func (s SpanSnapshot) Attributes() []attribute.KeyValue { + return s.attributes +} + +// Links returns all the links the span has to other spans. +func (s SpanSnapshot) Links() []trace.Link { + return s.links +} + +// Events returns all the events that occurred within in the spans +// lifetime. +func (s SpanSnapshot) Events() []Event { + return s.events +} + +// Status returns the spans status. +func (s SpanSnapshot) Status() Status { + return s.status +} + +// InstrumentationLibrary returns information about the instrumentation +// library that created the span. +func (s SpanSnapshot) InstrumentationLibrary() instrumentation.Library { + return s.instrumentationLibrary +} + +// Resource returns information about the entity that produced the span. +func (s SpanSnapshot) Resource() *resource.Resource { + return s.resource +} + +// DroppedAttributes returns the number of attributes dropped by the span +// due to limits being reached. +func (s SpanSnapshot) DroppedAttributes() int { + return s.droppedAttributeCount +} + +// DroppedLinks returns the number of links dropped by the span due to limits +// being reached. +func (s SpanSnapshot) DroppedLinks() int { + return s.droppedLinkCount +} + +// DroppedEvents returns the number of events dropped by the span due to +// limits being reached. +func (s SpanSnapshot) DroppedEvents() int { + return s.droppedEventCount +} + +// ChildSpanCount returns the count of spans that consider the span a +// direct parent. +func (s SpanSnapshot) ChildSpanCount() int { + return s.childSpanCount +} + +// TODO: remove this. +func (s SpanSnapshot) Snapshot() *SpanSnapshot { + return &s } diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 671de5ce4c5..07dba668c76 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -77,9 +77,6 @@ type ReadOnlySpan interface { // direct parent. ChildSpanCount() int - // TODO: remove this. - Snapshot() *SpanSnapshot - // A private method to prevent users implementing the // interface and so future additions to it will not // violate compatibility. @@ -265,7 +262,7 @@ func (s *span) End(options ...trace.SpanOption) { mustExportOrProcess := ok && len(sps) > 0 if mustExportOrProcess { for _, sp := range sps { - sp.sp.OnEnd(s) + sp.sp.OnEnd(s.snapshot()) } } } @@ -478,35 +475,34 @@ func (s *span) ChildSpanCount() int { return s.childSpanCount } -// Snapshot creates a snapshot representing the current state of the span as an -// export.SpanSnapshot and returns a pointer to it. -func (s *span) Snapshot() *SpanSnapshot { +// snapshot creates a read-only copy of the current state of the span. +func (s *span) snapshot() ReadOnlySpan { var sd SpanSnapshot s.mu.Lock() defer s.mu.Unlock() - sd.ChildSpanCount = s.childSpanCount - sd.EndTime = s.endTime - sd.InstrumentationLibrary = s.instrumentationLibrary - sd.Name = s.name - sd.Parent = s.parent - sd.Resource = s.resource - sd.SpanContext = s.spanContext - sd.SpanKind = s.spanKind - sd.StartTime = s.startTime - sd.Status = s.status + sd.endTime = s.endTime + sd.instrumentationLibrary = s.instrumentationLibrary + sd.name = s.name + sd.parent = s.parent + sd.resource = s.resource + sd.spanContext = s.spanContext + sd.spanKind = s.spanKind + sd.startTime = s.startTime + sd.status = s.status + sd.childSpanCount = s.childSpanCount if s.attributes.evictList.Len() > 0 { - sd.Attributes = s.attributes.toKeyValue() - sd.DroppedAttributeCount = s.attributes.droppedCount + sd.attributes = s.attributes.toKeyValue() + sd.droppedAttributeCount = s.attributes.droppedCount } if len(s.events.queue) > 0 { - sd.Events = s.interfaceArrayToEventArray() - sd.DroppedEventCount = s.events.droppedCount + sd.events = s.interfaceArrayToEventArray() + sd.droppedEventCount = s.events.droppedCount } if len(s.links.queue) > 0 { - sd.Links = s.interfaceArrayToLinksArray() - sd.DroppedLinkCount = s.links.droppedCount + sd.links = s.interfaceArrayToLinksArray() + sd.droppedLinkCount = s.links.droppedCount } return &sd } diff --git a/sdk/trace/span_exporter.go b/sdk/trace/span_exporter.go index 40e121069d1..ab9403a7dd2 100644 --- a/sdk/trace/span_exporter.go +++ b/sdk/trace/span_exporter.go @@ -16,10 +16,10 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace" import "context" -// SpanExporter handles the delivery of SpanSnapshot structs to external -// receivers. This is the final component in the trace export pipeline. +// SpanExporter handles the delivery of spans to external receivers. This is +// the final component in the trace export pipeline. type SpanExporter interface { - // ExportSpans exports a batch of SpanSnapshots. + // ExportSpans exports a batch of spans. // // This function is called synchronously, so there is no concurrency // safety requirement. However, due to the synchronous calling pattern, @@ -30,7 +30,7 @@ type SpanExporter interface { // calls this function will not implement any retry logic. All errors // returned by this function are considered unrecoverable and will be // reported to a configured error Handler. - ExportSpans(ctx context.Context, ss []*SpanSnapshot) error + ExportSpans(ctx context.Context, ss []ReadOnlySpan) error // Shutdown notifies the exporter of a pending halt to operations. The // exporter is expected to preform any cleanup or synchronization it // requires while honoring all timeouts and cancellations contained in diff --git a/sdk/trace/span_processor_filter_example_test.go b/sdk/trace/span_processor_filter_example_test.go index 3983a007ce0..6409bb57aaf 100644 --- a/sdk/trace/span_processor_filter_example_test.go +++ b/sdk/trace/span_processor_filter_example_test.go @@ -76,8 +76,8 @@ func (f InstrumentationBlacklist) OnEnd(s ReadOnlySpan) { type noopExporter struct{} -func (noopExporter) ExportSpans(context.Context, []*SpanSnapshot) error { return nil } -func (noopExporter) Shutdown(context.Context) error { return nil } +func (noopExporter) ExportSpans(context.Context, []ReadOnlySpan) error { return nil } +func (noopExporter) Shutdown(context.Context) error { return nil } func ExampleSpanProcessor_filtered() { exportSP := NewSimpleSpanProcessor(noopExporter{}) diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 9dd9adedc46..56efa872455 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -110,14 +110,14 @@ func NewTestExporter() *testExporter { return &testExporter{idx: make(map[string]int)} } -func (te *testExporter) ExportSpans(_ context.Context, ss []*SpanSnapshot) error { +func (te *testExporter) ExportSpans(_ context.Context, ss []ReadOnlySpan) error { te.mu.Lock() defer te.mu.Unlock() i := len(te.spans) for _, s := range ss { - te.idx[s.Name] = i - te.spans = append(te.spans, s) + te.idx[s.Name()] = i + te.spans = append(te.spans, s.(*SpanSnapshot)) i++ } return nil @@ -390,18 +390,18 @@ func TestSetSpanAttributesOnStart(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Attributes: []attribute.KeyValue{ + parent: sc.WithRemote(true), + name: "span0", + attributes: []attribute.KeyValue{ attribute.String("key1", "value1"), attribute.String("key2", "value2"), }, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "StartSpanAttribute"}, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "StartSpanAttribute"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanAttributesOnStart: -got +want %s", diff) @@ -419,17 +419,17 @@ func TestSetSpanAttributes(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Attributes: []attribute.KeyValue{ + parent: sc.WithRemote(true), + name: "span0", + attributes: []attribute.KeyValue{ attribute.String("key1", "value1"), }, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "SpanAttribute"}, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "SpanAttribute"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanAttributes: -got +want %s", diff) @@ -454,8 +454,8 @@ func TestSamplerAttributesLocalChildSpan(t *testing.T) { gotSpan0, gotSpan1 := got[0], got[1] // Ensure sampler is called for local child spans by verifying the // attributes set by the sampler are set on the child span. - assert.Equal(t, []attribute.KeyValue{attribute.Int("callCount", 2)}, gotSpan0.Attributes) - assert.Equal(t, []attribute.KeyValue{attribute.Int("callCount", 1)}, gotSpan1.Attributes) + assert.Equal(t, []attribute.KeyValue{attribute.Int("callCount", 2)}, gotSpan0.Attributes()) + assert.Equal(t, []attribute.KeyValue{attribute.Int("callCount", 1)}, gotSpan1.Attributes()) } func TestSetSpanAttributesOverLimit(t *testing.T) { @@ -475,19 +475,19 @@ func TestSetSpanAttributesOverLimit(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Attributes: []attribute.KeyValue{ + parent: sc.WithRemote(true), + name: "span0", + attributes: []attribute.KeyValue{ attribute.Bool("key1", false), attribute.Int64("key4", 4), }, - SpanKind: trace.SpanKindInternal, - DroppedAttributeCount: 1, - InstrumentationLibrary: instrumentation.Library{Name: "SpanAttributesOverLimit"}, + spanKind: trace.SpanKindInternal, + droppedAttributeCount: 1, + instrumentationLibrary: instrumentation.Library{Name: "SpanAttributesOverLimit"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanAttributesOverLimit: -got +want %s", diff) @@ -509,18 +509,18 @@ func TestSetSpanAttributesWithInvalidKey(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Attributes: []attribute.KeyValue{ + parent: sc.WithRemote(true), + name: "span0", + attributes: []attribute.KeyValue{ attribute.Bool("key1", false), }, - SpanKind: trace.SpanKindInternal, - DroppedAttributeCount: 0, - InstrumentationLibrary: instrumentation.Library{Name: "SpanToSetInvalidKeyOrValue"}, + spanKind: trace.SpanKindInternal, + droppedAttributeCount: 0, + instrumentationLibrary: instrumentation.Library{Name: "SpanToSetInvalidKeyOrValue"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanAttributesWithInvalidKey: -got +want %s", diff) @@ -546,25 +546,25 @@ func TestEvents(t *testing.T) { t.Fatal(err) } - for i := range got.Events { - if !checkTime(&got.Events[i].Time) { + for i := range got.Events() { + if !checkTime(&got.Events()[i].Time) { t.Error("exporting span: expected nonzero Event Time") } } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Events: []Event{ + parent: sc.WithRemote(true), + name: "span0", + events: []Event{ {Name: "foo", Attributes: []attribute.KeyValue{k1v1}}, {Name: "bar", Attributes: []attribute.KeyValue{k2v2, k3v3}}, }, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "Events"}, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "Events"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("Message Events: -got +want %s", diff) @@ -595,26 +595,26 @@ func TestEventsOverLimit(t *testing.T) { t.Fatal(err) } - for i := range got.Events { - if !checkTime(&got.Events[i].Time) { + for i := range got.Events() { + if !checkTime(&got.Events()[i].Time) { t.Error("exporting span: expected nonzero Event Time") } } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Events: []Event{ + parent: sc.WithRemote(true), + name: "span0", + events: []Event{ {Name: "foo", Attributes: []attribute.KeyValue{k1v1}}, {Name: "bar", Attributes: []attribute.KeyValue{k2v2, k3v3}}, }, - DroppedEventCount: 2, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "EventsOverLimit"}, + droppedEventCount: 2, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "EventsOverLimit"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("Message Event over limit: -got +want %s", diff) @@ -644,15 +644,15 @@ func TestLinks(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Links: links, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "Links"}, + parent: sc.WithRemote(true), + name: "span0", + links: links, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "Links"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("Link: -got +want %s", diff) @@ -685,19 +685,19 @@ func TestLinksOverLimit(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Links: []trace.Link{ + parent: sc.WithRemote(true), + name: "span0", + links: []trace.Link{ {SpanContext: sc2, Attributes: []attribute.KeyValue{k2v2}}, {SpanContext: sc3, Attributes: []attribute.KeyValue{k3v3}}, }, - DroppedLinkCount: 1, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "LinksOverLimit"}, + droppedLinkCount: 1, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "LinksOverLimit"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("Link over limit: -got +want %s", diff) @@ -717,8 +717,8 @@ func TestSetSpanName(t *testing.T) { t.Fatal(err) } - if got.Name != want { - t.Errorf("span.Name: got %q; want %q", got.Name, want) + if got.Name() != want { + t.Errorf("span.Name: got %q; want %q", got.Name(), want) } } @@ -734,18 +734,18 @@ func TestSetSpanStatus(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - Status: Status{ + parent: sc.WithRemote(true), + name: "span0", + spanKind: trace.SpanKindInternal, + status: Status{ Code: codes.Error, Description: "Error", }, - InstrumentationLibrary: instrumentation.Library{Name: "SpanStatus"}, + instrumentationLibrary: instrumentation.Library{Name: "SpanStatus"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanStatus: -got +want %s", diff) @@ -764,18 +764,18 @@ func TestSetSpanStatusWithoutMessageWhenStatusIsNotError(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - Status: Status{ + parent: sc.WithRemote(true), + name: "span0", + spanKind: trace.SpanKindInternal, + status: Status{ Code: codes.Ok, Description: "", }, - InstrumentationLibrary: instrumentation.Library{Name: "SpanStatus"}, + instrumentationLibrary: instrumentation.Library{Name: "SpanStatus"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanStatus: -got +want %s", diff) @@ -784,6 +784,7 @@ func TestSetSpanStatusWithoutMessageWhenStatusIsNotError(t *testing.T) { func cmpDiff(x, y interface{}) string { return cmp.Diff(x, y, + cmp.AllowUnexported(SpanSnapshot{}), cmp.AllowUnexported(attribute.Value{}), cmp.AllowUnexported(Event{}), cmp.AllowUnexported(trace.TraceState{})) @@ -843,15 +844,14 @@ func startLocalSpan(tp *TracerProvider, ctx context.Context, trName, name string } // endSpan is a test utility function that ends the span in the context and -// returns the exported export.SpanSnapshot. +// returns the exported span. // It requires that span be sampled using one of these methods // 1. Passing parent span context in context // 2. Use WithSampler(AlwaysSample()) // 3. Configuring AlwaysSample() as default sampler // // It also does some basic tests on the span. -// It also clears spanID in the export.SpanSnapshot to make the comparison -// easier. +// It also clears spanID in the to make the comparison easier. func endSpan(te *testExporter, span trace.Span) (*SpanSnapshot, error) { if !span.IsRecording() { return nil, fmt.Errorf("IsRecording: got false, want true") @@ -864,14 +864,14 @@ func endSpan(te *testExporter, span trace.Span) (*SpanSnapshot, error) { return nil, fmt.Errorf("got %d exported spans, want one span", te.Len()) } got := te.Spans()[0] - if !got.SpanContext.SpanID().IsValid() { + if !got.SpanContext().SpanID().IsValid() { return nil, fmt.Errorf("exporting span: expected nonzero SpanID") } - got.SpanContext = got.SpanContext.WithSpanID(trace.SpanID{}) - if !checkTime(&got.StartTime) { + got.spanContext = got.SpanContext().WithSpanID(trace.SpanID{}) + if !checkTime(&got.startTime) { return nil, fmt.Errorf("exporting span: expected nonzero StartTime") } - if !checkTime(&got.EndTime) { + if !checkTime(&got.endTime) { return nil, fmt.Errorf("exporting span: expected nonzero EndTime") } return got, nil @@ -939,16 +939,16 @@ func TestStartSpanAfterEnd(t *testing.T) { t.Fatal("span-2 not recorded") } - if got, want := gotSpan1.SpanContext.TraceID(), gotParent.SpanContext.TraceID(); got != want { + if got, want := gotSpan1.SpanContext().TraceID(), gotParent.SpanContext().TraceID(); got != want { t.Errorf("span-1.TraceID=%q; want %q", got, want) } - if got, want := gotSpan2.SpanContext.TraceID(), gotParent.SpanContext.TraceID(); got != want { + if got, want := gotSpan2.SpanContext().TraceID(), gotParent.SpanContext().TraceID(); got != want { t.Errorf("span-2.TraceID=%q; want %q", got, want) } - if got, want := gotSpan1.Parent.SpanID(), gotParent.SpanContext.SpanID(); got != want { + if got, want := gotSpan1.Parent().SpanID(), gotParent.SpanContext().SpanID(); got != want { t.Errorf("span-1.ParentSpanID=%q; want %q (parent.SpanID)", got, want) } - if got, want := gotSpan2.Parent.SpanID(), gotSpan1.SpanContext.SpanID(); got != want { + if got, want := gotSpan2.Parent().SpanID(), gotSpan1.SpanContext().SpanID(); got != want { t.Errorf("span-2.ParentSpanID=%q; want %q (span1.SpanID)", got, want) } } @@ -988,16 +988,16 @@ func TestChildSpanCount(t *testing.T) { t.Fatal("span-3 not recorded") } - if got, want := gotSpan3.ChildSpanCount, 0; got != want { + if got, want := gotSpan3.ChildSpanCount(), 0; got != want { t.Errorf("span-3.ChildSpanCount=%d; want %d", got, want) } - if got, want := gotSpan2.ChildSpanCount, 0; got != want { + if got, want := gotSpan2.ChildSpanCount(), 0; got != want { t.Errorf("span-2.ChildSpanCount=%d; want %d", got, want) } - if got, want := gotSpan1.ChildSpanCount, 1; got != want { + if got, want := gotSpan1.ChildSpanCount(), 1; got != want { t.Errorf("span-1.ChildSpanCount=%d; want %d", got, want) } - if got, want := gotParent.ChildSpanCount, 2; got != want { + if got, want := gotParent.ChildSpanCount(), 2; got != want { t.Errorf("parent.ChildSpanCount=%d; want %d", got, want) } } @@ -1075,11 +1075,11 @@ func TestCustomStartEndTime(t *testing.T) { t.Fatalf("got %d exported spans, want one span", te.Len()) } got := te.Spans()[0] - if got.StartTime != startTime { - t.Errorf("expected start time to be %s, got %s", startTime, got.StartTime) + if got.StartTime() != startTime { + t.Errorf("expected start time to be %s, got %s", startTime, got.StartTime()) } - if got.EndTime != endTime { - t.Errorf("expected end time to be %s, got %s", endTime, got.EndTime) + if got.EndTime() != endTime { + t.Errorf("expected end time to be %s, got %s", endTime, got.EndTime()) } } @@ -1115,15 +1115,15 @@ func TestRecordError(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Status: Status{Code: codes.Unset}, - SpanKind: trace.SpanKindInternal, - Events: []Event{ + parent: sc.WithRemote(true), + name: "span0", + status: Status{Code: codes.Unset}, + spanKind: trace.SpanKindInternal, + events: []Event{ { Name: semconv.ExceptionEventName, Time: errTime, @@ -1133,7 +1133,7 @@ func TestRecordError(t *testing.T) { }, }, }, - InstrumentationLibrary: instrumentation.Library{Name: "RecordError"}, + instrumentationLibrary: instrumentation.Library{Name: "RecordError"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SpanErrorOptions: -got +want %s", diff) @@ -1154,18 +1154,18 @@ func TestRecordErrorNil(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - Status: Status{ + parent: sc.WithRemote(true), + name: "span0", + spanKind: trace.SpanKindInternal, + status: Status{ Code: codes.Unset, Description: "", }, - InstrumentationLibrary: instrumentation.Library{Name: "RecordErrorNil"}, + instrumentationLibrary: instrumentation.Library{Name: "RecordErrorNil"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SpanErrorOptions: -got +want %s", diff) @@ -1183,8 +1183,8 @@ func TestWithSpanKind(t *testing.T) { t.Error(err.Error()) } - if spanData.SpanKind != trace.SpanKindInternal { - t.Errorf("Default value of Spankind should be Internal: got %+v, want %+v\n", spanData.SpanKind, trace.SpanKindInternal) + if spanData.SpanKind() != trace.SpanKindInternal { + t.Errorf("Default value of Spankind should be Internal: got %+v, want %+v\n", spanData.SpanKind(), trace.SpanKindInternal) } sks := []trace.SpanKind{ @@ -1204,8 +1204,8 @@ func TestWithSpanKind(t *testing.T) { t.Error(err.Error()) } - if spanData.SpanKind != sk { - t.Errorf("WithSpanKind check: got %+v, want %+v\n", spanData.SpanKind, sks) + if spanData.SpanKind() != sk { + t.Errorf("WithSpanKind check: got %+v, want %+v\n", spanData.SpanKind(), sks) } } } @@ -1264,18 +1264,18 @@ func TestWithResource(t *testing.T) { t.Error(err.Error()) } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Attributes: []attribute.KeyValue{ + parent: sc.WithRemote(true), + name: "span0", + attributes: []attribute.KeyValue{ attribute.String("key1", "value1"), }, - SpanKind: trace.SpanKindInternal, - Resource: tc.want, - InstrumentationLibrary: instrumentation.Library{Name: "WithResource"}, + spanKind: trace.SpanKindInternal, + resource: tc.want, + instrumentationLibrary: instrumentation.Library{Name: "WithResource"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("WithResource:\n -got +want %s", diff) @@ -1300,14 +1300,14 @@ func TestWithInstrumentationVersion(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{ + parent: sc.WithRemote(true), + name: "span0", + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{ Name: "WithInstrumentationVersion", Version: "v0.1.0", }, @@ -1332,9 +1332,9 @@ func TestSpanCapturesPanic(t *testing.T) { require.PanicsWithError(t, "error message", f) spans := te.Spans() require.Len(t, spans, 1) - require.Len(t, spans[0].Events, 1) - assert.Equal(t, spans[0].Events[0].Name, semconv.ExceptionEventName) - assert.Equal(t, spans[0].Events[0].Attributes, []attribute.KeyValue{ + require.Len(t, spans[0].Events(), 1) + assert.Equal(t, spans[0].Events()[0].Name, semconv.ExceptionEventName) + assert.Equal(t, spans[0].Events()[0].Attributes, []attribute.KeyValue{ semconv.ExceptionTypeKey.String("*errors.errorString"), semconv.ExceptionMessageKey.String("error message"), }) @@ -1365,14 +1365,14 @@ func TestReadOnlySpan(t *testing.T) { }) st := time.Now() - ctx, span := tr.Start(ctx, "foo", trace.WithTimestamp(st), + ctx, s := tr.Start(ctx, "foo", trace.WithTimestamp(st), trace.WithLinks(trace.Link{SpanContext: linked})) - span.SetAttributes(kv) - span.AddEvent("foo", trace.WithAttributes(kv)) - span.SetStatus(codes.Ok, "foo") + s.SetAttributes(kv) + s.AddEvent("foo", trace.WithAttributes(kv)) + s.SetStatus(codes.Ok, "foo") // Verify span implements ReadOnlySpan. - ro, ok := span.(ReadOnlySpan) + ro, ok := s.(ReadOnlySpan) require.True(t, ok) assert.Equal(t, "foo", ro.Name()) @@ -1394,21 +1394,21 @@ func TestReadOnlySpan(t *testing.T) { assert.Equal(t, kv.Value, ro.Resource().Attributes()[0].Value) // Verify changes to the original span are reflected in the ReadOnlySpan. - span.SetName("bar") + s.SetName("bar") assert.Equal(t, "bar", ro.Name()) - // Verify Snapshot() returns snapshots that are independent from the + // Verify snapshot() returns snapshots that are independent from the // original span and from one another. - d1 := ro.Snapshot() - span.AddEvent("baz") - d2 := ro.Snapshot() - for _, e := range d1.Events { + d1 := s.(*span).snapshot() + s.AddEvent("baz") + d2 := s.(*span).snapshot() + for _, e := range d1.Events() { if e.Name == "baz" { t.Errorf("Didn't expect to find 'baz' event") } } var exists bool - for _, e := range d2.Events { + for _, e := range d2.Events() { if e.Name == "baz" { exists = true } @@ -1418,7 +1418,7 @@ func TestReadOnlySpan(t *testing.T) { } et := st.Add(time.Millisecond) - span.End(trace.WithTimestamp(et)) + s.End(trace.WithTimestamp(et)) assert.Equal(t, et, ro.EndTime()) } @@ -1481,21 +1481,21 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { t.Fatal(err) } - for i := range got.Events { - if !checkTime(&got.Events[i].Time) { + for i := range got.Events() { + if !checkTime(&got.Events()[i].Time) { t.Error("exporting span: expected nonzero Event Time") } } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Attributes: nil, - Events: []Event{ + parent: sc.WithRemote(true), + name: "span0", + attributes: nil, + events: []Event{ { Name: "test1", Attributes: []attribute.KeyValue{ @@ -1512,8 +1512,8 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { DroppedAttributeCount: 2, }, }, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "AddSpanEventWithOverLimitedAttributes"}, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "AddSpanEventWithOverLimitedAttributes"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("SetSpanAttributesOverLimit: -got +want %s", diff) @@ -1547,13 +1547,13 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { } want := &SpanSnapshot{ - SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, }), - Parent: sc.WithRemote(true), - Name: "span0", - Links: []trace.Link{ + parent: sc.WithRemote(true), + name: "span0", + links: []trace.Link{ { SpanContext: sc1, Attributes: []attribute.KeyValue{k1v1}, @@ -1565,8 +1565,8 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { DroppedAttributeCount: 2, }, }, - SpanKind: trace.SpanKindInternal, - InstrumentationLibrary: instrumentation.Library{Name: "Links"}, + spanKind: trace.SpanKindInternal, + instrumentationLibrary: instrumentation.Library{Name: "Links"}, } if diff := cmpDiff(got, want); diff != "" { t.Errorf("Link: -got +want %s", diff) @@ -1706,7 +1706,7 @@ func TestSamplerTraceState(t *testing.T) { return } - receivedState := got[0].SpanContext.TraceState() + receivedState := got[0].SpanContext().TraceState() if diff := cmpDiff(receivedState, ts.want); diff != "" { t.Errorf("TraceState not propagated: -got +want %s", diff) diff --git a/sdk/trace/tracetest/test.go b/sdk/trace/tracetest/test.go index ad1d2f226f3..180e5869546 100644 --- a/sdk/trace/tracetest/test.go +++ b/sdk/trace/tracetest/test.go @@ -36,7 +36,7 @@ func NewNoopExporter() *NoopExporter { type NoopExporter struct{} // ExportSpans handles export of SpanSnapshots by dropping them. -func (nsb *NoopExporter) ExportSpans(context.Context, []*trace.SpanSnapshot) error { return nil } +func (nsb *NoopExporter) ExportSpans(context.Context, []trace.ReadOnlySpan) error { return nil } // Shutdown stops the exporter by doing nothing. func (nsb *NoopExporter) Shutdown(context.Context) error { return nil } @@ -51,14 +51,14 @@ func NewInMemoryExporter() *InMemoryExporter { // InMemoryExporter is an exporter that stores all received spans in-memory. type InMemoryExporter struct { mu sync.Mutex - ss []*trace.SpanSnapshot + ss SpanStubs } // ExportSpans handles export of SpanSnapshots by storing them in memory. -func (imsb *InMemoryExporter) ExportSpans(_ context.Context, ss []*trace.SpanSnapshot) error { +func (imsb *InMemoryExporter) ExportSpans(_ context.Context, ss []trace.ReadOnlySpan) error { imsb.mu.Lock() defer imsb.mu.Unlock() - imsb.ss = append(imsb.ss, ss...) + imsb.ss = append(imsb.ss, SpanStubsFromReadOnlySpans(ss)...) return nil } @@ -76,10 +76,10 @@ func (imsb *InMemoryExporter) Reset() { } // GetSpans returns the current in-memory stored spans. -func (imsb *InMemoryExporter) GetSpans() []*trace.SpanSnapshot { +func (imsb *InMemoryExporter) GetSpans() SpanStubs { imsb.mu.Lock() defer imsb.mu.Unlock() - ret := make([]*trace.SpanSnapshot, len(imsb.ss)) + ret := make(SpanStubs, len(imsb.ss)) copy(ret, imsb.ss) return ret } diff --git a/sdk/trace/tracetest/test_test.go b/sdk/trace/tracetest/test_test.go index 8fee38b65fc..e718207f467 100644 --- a/sdk/trace/tracetest/test_test.go +++ b/sdk/trace/tracetest/test_test.go @@ -16,12 +16,11 @@ package tracetest import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "go.opentelemetry.io/otel/sdk/trace" ) // TestNoop tests only that the no-op does not crash in different scenarios. @@ -29,8 +28,8 @@ func TestNoop(t *testing.T) { nsb := NewNoopExporter() require.NoError(t, nsb.ExportSpans(context.Background(), nil)) - require.NoError(t, nsb.ExportSpans(context.Background(), make([]*trace.SpanSnapshot, 10))) - require.NoError(t, nsb.ExportSpans(context.Background(), make([]*trace.SpanSnapshot, 0, 10))) + require.NoError(t, nsb.ExportSpans(context.Background(), make(SpanStubs, 10).Snapshots())) + require.NoError(t, nsb.ExportSpans(context.Background(), make(SpanStubs, 0, 10).Snapshots())) } func TestNewInMemoryExporter(t *testing.T) { @@ -39,23 +38,23 @@ func TestNewInMemoryExporter(t *testing.T) { require.NoError(t, imsb.ExportSpans(context.Background(), nil)) assert.Len(t, imsb.GetSpans(), 0) - input := make([]*trace.SpanSnapshot, 10) + input := make(SpanStubs, 10) for i := 0; i < 10; i++ { - input[i] = new(trace.SpanSnapshot) + input[i] = SpanStub{Name: fmt.Sprintf("span %d", i)} } - require.NoError(t, imsb.ExportSpans(context.Background(), input)) + require.NoError(t, imsb.ExportSpans(context.Background(), input.Snapshots())) sds := imsb.GetSpans() assert.Len(t, sds, 10) for i, sd := range sds { - assert.Same(t, input[i], sd) + assert.Equal(t, input[i], sd) } imsb.Reset() // Ensure that operations on the internal storage does not change the previously returned value. assert.Len(t, sds, 10) assert.Len(t, imsb.GetSpans(), 0) - require.NoError(t, imsb.ExportSpans(context.Background(), input[0:1])) + require.NoError(t, imsb.ExportSpans(context.Background(), input.Snapshots()[0:1])) sds = imsb.GetSpans() assert.Len(t, sds, 1) - assert.Same(t, input[0], sds[0]) + assert.Equal(t, input[0], sds[0]) } From 4c92b721e2cecc7ba44ec92bd34b1e43f1fa73e0 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 2 May 2021 10:19:24 -0700 Subject: [PATCH 11/14] Rename SpanSnapshot to snapshot and purge docs --- exporters/otlp/internal/otlptest/data.go | 4 +- exporters/otlp/internal/transform/span.go | 2 +- exporters/otlp/otlp.go | 6 +-- .../otlp/otlpgrpc/otlp_integration_test.go | 6 +-- exporters/otlp/otlphttp/driver_test.go | 18 +++---- exporters/stdout/trace.go | 8 +-- exporters/trace/zipkin/zipkin.go | 12 ++--- sdk/trace/batch_span_processor.go | 2 +- sdk/trace/batch_span_processor_test.go | 8 +-- sdk/trace/simple_span_processor_test.go | 4 +- sdk/trace/snapshot.go | 50 ++++++++----------- sdk/trace/span.go | 2 +- sdk/trace/span_exporter.go | 2 +- sdk/trace/trace_test.go | 50 +++++++++---------- sdk/trace/tracetest/test.go | 14 +++--- 15 files changed, 90 insertions(+), 98 deletions(-) diff --git a/exporters/otlp/internal/otlptest/data.go b/exporters/otlp/internal/otlptest/data.go index b980e1edc04..08f5dc38d00 100644 --- a/exporters/otlp/internal/otlptest/data.go +++ b/exporters/otlp/internal/otlptest/data.go @@ -79,9 +79,9 @@ func (OneRecordCheckpointSet) ForEach(kindSelector exportmetric.ExportKindSelect return recordFunc(rec) } -// SingleSpanSnapshot returns a one-element slice with a snapshot. It +// SingleReadOnlySpan returns a one-element slice with a read-only span. It // may be useful for testing driver's trace export. -func SingleSpanSnapshot() []tracesdk.ReadOnlySpan { +func SingleReadOnlySpan() []tracesdk.ReadOnlySpan { return tracetest.SpanStubs{ { SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index 351d67f51e8..77d1db7a46e 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -28,7 +28,7 @@ const ( maxEventsPerSpan = 128 ) -// Spans transforms a slice of SpanSnapshot into a slice of OTLP +// Spans transforms a slice of OpenTelemetry spans into a slice of OTLP // ResourceSpans. func Spans(sdl []tracesdk.ReadOnlySpan) []*tracepb.ResourceSpans { if len(sdl) == 0 { diff --git a/exporters/otlp/otlp.go b/exporters/otlp/otlp.go index 01685624d5d..8595ea42bd8 100644 --- a/exporters/otlp/otlp.go +++ b/exporters/otlp/otlp.go @@ -129,10 +129,10 @@ func (e *Exporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) return e.cfg.exportKindSelector.ExportKindFor(desc, kind) } -// ExportSpans transforms and batches trace SpanSnapshots into OTLP Trace and +// ExportSpans transforms and batches OpenTelemetry spans into OTLP Trace and // transmits them to the configured collector. -func (e *Exporter) ExportSpans(ctx context.Context, ss []tracesdk.ReadOnlySpan) error { - return e.driver.ExportTraces(ctx, ss) +func (e *Exporter) ExportSpans(ctx context.Context, spans []tracesdk.ReadOnlySpan) error { + return e.driver.ExportTraces(ctx, spans) } // NewExportPipeline sets up a complete export pipeline diff --git a/exporters/otlp/otlpgrpc/otlp_integration_test.go b/exporters/otlp/otlpgrpc/otlp_integration_test.go index abdd8b1666c..25dc243bad2 100644 --- a/exporters/otlp/otlpgrpc/otlp_integration_test.go +++ b/exporters/otlp/otlpgrpc/otlp_integration_test.go @@ -192,7 +192,7 @@ func TestNewExporter_collectorConnectionDiesThenReconnectsWhenInRestMode(t *test nmaSpans := nmc.getSpans() - // Expecting 10 SpanSnapshots that were sampled, given that + // Expecting 10 spans that were sampled, given that if g, w := len(nmaSpans), n; g != w { t.Fatalf("Connected collector: spans: got %d want %d", g, w) } @@ -510,7 +510,7 @@ func TestNewExporter_collectorConnectionDiesThenReconnects(t *testing.T) { } nmaSpans := nmc.getSpans() - // Expecting 10 SpanSnapshots that were sampled, given that + // Expecting 10 spans that were sampled, given that if g, w := len(nmaSpans), n; g != w { t.Fatalf("Round #%d: Connected collector: spans: got %d want %d", j, g, w) } @@ -836,7 +836,7 @@ func TestDisconnected(t *testing.T) { }() assert.Error(t, exp.Export(ctx, otlptest.OneRecordCheckpointSet{})) - assert.Error(t, exp.ExportSpans(ctx, otlptest.SingleSpanSnapshot())) + assert.Error(t, exp.ExportSpans(ctx, otlptest.SingleReadOnlySpan())) } func TestEmptyData(t *testing.T) { diff --git a/exporters/otlp/otlphttp/driver_test.go b/exporters/otlp/otlphttp/driver_test.go index 10190154c37..6fa94f67536 100644 --- a/exporters/otlp/otlphttp/driver_test.go +++ b/exporters/otlp/otlphttp/driver_test.go @@ -165,7 +165,7 @@ func TestRetry(t *testing.T) { defer func() { assert.NoError(t, exporter.Shutdown(ctx)) }() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.NoError(t, err) assert.Len(t, mc.GetSpans(), 1) } @@ -187,7 +187,7 @@ func TestTimeout(t *testing.T) { defer func() { assert.NoError(t, exporter.Shutdown(ctx)) }() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Equal(t, true, os.IsTimeout(err)) } @@ -212,7 +212,7 @@ func TestRetryFailed(t *testing.T) { defer func() { assert.NoError(t, exporter.Shutdown(ctx)) }() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Empty(t, mc.GetSpans()) } @@ -237,7 +237,7 @@ func TestNoRetry(t *testing.T) { defer func() { assert.NoError(t, exporter.Shutdown(ctx)) }() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Equal(t, fmt.Sprintf("failed to send traces to http://%s/v1/traces with HTTP status 400 Bad Request", mc.endpoint), err.Error()) assert.Empty(t, mc.GetSpans()) @@ -325,7 +325,7 @@ func TestUnreasonableMaxAttempts(t *testing.T) { defer func() { assert.NoError(t, exporter.Shutdown(ctx)) }() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Empty(t, mc.GetSpans()) }) @@ -361,7 +361,7 @@ func TestUnreasonableBackoff(t *testing.T) { defer func() { assert.NoError(t, exporter.Shutdown(ctx)) }() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Empty(t, mc.GetSpans()) } @@ -381,7 +381,7 @@ func TestCancelledContext(t *testing.T) { assert.NoError(t, exporter.Shutdown(ctx)) }() cancel() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Empty(t, mc.GetSpans()) } @@ -409,7 +409,7 @@ func TestDeadlineContext(t *testing.T) { }() ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - err = exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err = exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Empty(t, mc.GetSpans()) } @@ -437,7 +437,7 @@ func TestStopWhileExporting(t *testing.T) { }() doneCh := make(chan struct{}) go func() { - err := exporter.ExportSpans(ctx, otlptest.SingleSpanSnapshot()) + err := exporter.ExportSpans(ctx, otlptest.SingleReadOnlySpan()) assert.Error(t, err) assert.Empty(t, mc.GetSpans()) close(doneCh) diff --git a/exporters/stdout/trace.go b/exporters/stdout/trace.go index 9720ffd3f47..3aaefc54873 100644 --- a/exporters/stdout/trace.go +++ b/exporters/stdout/trace.go @@ -32,8 +32,8 @@ type traceExporter struct { stopped bool } -// ExportSpans writes SpanSnapshots in json format to stdout. -func (e *traceExporter) ExportSpans(ctx context.Context, ss []trace.ReadOnlySpan) error { +// ExportSpans writes spans in json format to stdout. +func (e *traceExporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -41,10 +41,10 @@ func (e *traceExporter) ExportSpans(ctx context.Context, ss []trace.ReadOnlySpan return nil } - if e.config.DisableTraceExport || len(ss) == 0 { + if e.config.DisableTraceExport || len(spans) == 0 { return nil } - out, err := e.marshal(tracetest.SpanStubsFromReadOnlySpans(ss)) + out, err := e.marshal(tracetest.SpanStubsFromReadOnlySpans(spans)) if err != nil { return err } diff --git a/exporters/trace/zipkin/zipkin.go b/exporters/trace/zipkin/zipkin.go index ab756abfdd8..afb0c24d95b 100644 --- a/exporters/trace/zipkin/zipkin.go +++ b/exporters/trace/zipkin/zipkin.go @@ -31,9 +31,7 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" ) -// Exporter exports SpanSnapshots to the zipkin collector. It implements -// the SpanBatcher interface, so it needs to be used together with the -// WithBatcher option when setting up the exporter pipeline. +// Exporter exports spans to the zipkin collector. type Exporter struct { url string client *http.Client @@ -133,8 +131,8 @@ func InstallNewPipeline(collectorURL string, opts ...Option) error { return nil } -// ExportSpans exports SpanSnapshots to a Zipkin receiver. -func (e *Exporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error { +// ExportSpans exports spans to a Zipkin receiver. +func (e *Exporter) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -143,11 +141,11 @@ func (e *Exporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) return nil } - if len(ss) == 0 { + if len(spans) == 0 { e.logf("no spans to export") return nil } - models := toZipkinSpanModels(ss) + models := toZipkinSpanModels(spans) body, err := json.Marshal(models) if err != nil { return e.errf("failed to serialize zipkin models to JSON: %v", err) diff --git a/sdk/trace/batch_span_processor.go b/sdk/trace/batch_span_processor.go index fbda5872465..6dc75acd864 100644 --- a/sdk/trace/batch_span_processor.go +++ b/sdk/trace/batch_span_processor.go @@ -63,7 +63,7 @@ type BatchSpanProcessorOptions struct { } // batchSpanProcessor is a SpanProcessor that batches asynchronously-received -// SpanSnapshots and sends them to a trace.Exporter when complete. +// spans and sends them to a trace.Exporter when complete. type batchSpanProcessor struct { e SpanExporter o BatchSpanProcessorOptions diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go index 6aae90bf9c3..3da75aeaffc 100644 --- a/sdk/trace/batch_span_processor_test.go +++ b/sdk/trace/batch_span_processor_test.go @@ -43,12 +43,12 @@ type testBatchExporter struct { err error } -func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error { +func (t *testBatchExporter) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { t.mu.Lock() defer t.mu.Unlock() if t.idx < len(t.errors) { - t.droppedCount += len(ss) + t.droppedCount += len(spans) err := t.errors[t.idx] t.idx++ return err @@ -63,8 +63,8 @@ func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadO default: } - t.spans = append(t.spans, ss...) - t.sizes = append(t.sizes, len(ss)) + t.spans = append(t.spans, spans...) + t.sizes = append(t.sizes, len(spans)) t.batchCount++ return nil } diff --git a/sdk/trace/simple_span_processor_test.go b/sdk/trace/simple_span_processor_test.go index c9b04763feb..688cc4703bd 100644 --- a/sdk/trace/simple_span_processor_test.go +++ b/sdk/trace/simple_span_processor_test.go @@ -35,8 +35,8 @@ type testExporter struct { shutdown bool } -func (t *testExporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error { - t.spans = append(t.spans, ss...) +func (t *testExporter) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { + t.spans = append(t.spans, spans...) return nil } diff --git a/sdk/trace/snapshot.go b/sdk/trace/snapshot.go index e6a96290779..6bd232cc2b0 100644 --- a/sdk/trace/snapshot.go +++ b/sdk/trace/snapshot.go @@ -23,15 +23,9 @@ import ( "go.opentelemetry.io/otel/trace" ) -// SpanSnapshot is a snapshot of a span which contains all the information -// collected by the span. Its main purpose is exporting completed spans. -// Although SpanSnapshot fields can be accessed and potentially modified, -// SpanSnapshot should be treated as immutable. Changes to the span from which -// the SpanSnapshot was created are NOT reflected in the SpanSnapshot. -// -// TODO: unexport and rename to snapshot. -// TODO: clean project docs of this type after rename. -type SpanSnapshot struct { +// snapshot is an record of a spans state at a particular checkpointed time. +// It is used as a read-only representation of that state. +type snapshot struct { name string spanContext trace.SpanContext parent trace.SpanContext @@ -50,100 +44,100 @@ type SpanSnapshot struct { instrumentationLibrary instrumentation.Library } -var _ ReadOnlySpan = SpanSnapshot{} +var _ ReadOnlySpan = snapshot{} -func (s SpanSnapshot) private() {} +func (s snapshot) private() {} // Name returns the name of the span. -func (s SpanSnapshot) Name() string { +func (s snapshot) Name() string { return s.name } // SpanContext returns the unique SpanContext that identifies the span. -func (s SpanSnapshot) SpanContext() trace.SpanContext { +func (s snapshot) SpanContext() trace.SpanContext { return s.spanContext } // Parent returns the unique SpanContext that identifies the parent of the // span if one exists. If the span has no parent the returned SpanContext // will be invalid. -func (s SpanSnapshot) Parent() trace.SpanContext { +func (s snapshot) Parent() trace.SpanContext { return s.parent } // SpanKind returns the role the span plays in a Trace. -func (s SpanSnapshot) SpanKind() trace.SpanKind { +func (s snapshot) SpanKind() trace.SpanKind { return s.spanKind } // StartTime returns the time the span started recording. -func (s SpanSnapshot) StartTime() time.Time { +func (s snapshot) StartTime() time.Time { return s.startTime } // EndTime returns the time the span stopped recording. It will be zero if // the span has not ended. -func (s SpanSnapshot) EndTime() time.Time { +func (s snapshot) EndTime() time.Time { return s.endTime } // Attributes returns the defining attributes of the span. -func (s SpanSnapshot) Attributes() []attribute.KeyValue { +func (s snapshot) Attributes() []attribute.KeyValue { return s.attributes } // Links returns all the links the span has to other spans. -func (s SpanSnapshot) Links() []trace.Link { +func (s snapshot) Links() []trace.Link { return s.links } // Events returns all the events that occurred within in the spans // lifetime. -func (s SpanSnapshot) Events() []Event { +func (s snapshot) Events() []Event { return s.events } // Status returns the spans status. -func (s SpanSnapshot) Status() Status { +func (s snapshot) Status() Status { return s.status } // InstrumentationLibrary returns information about the instrumentation // library that created the span. -func (s SpanSnapshot) InstrumentationLibrary() instrumentation.Library { +func (s snapshot) InstrumentationLibrary() instrumentation.Library { return s.instrumentationLibrary } // Resource returns information about the entity that produced the span. -func (s SpanSnapshot) Resource() *resource.Resource { +func (s snapshot) Resource() *resource.Resource { return s.resource } // DroppedAttributes returns the number of attributes dropped by the span // due to limits being reached. -func (s SpanSnapshot) DroppedAttributes() int { +func (s snapshot) DroppedAttributes() int { return s.droppedAttributeCount } // DroppedLinks returns the number of links dropped by the span due to limits // being reached. -func (s SpanSnapshot) DroppedLinks() int { +func (s snapshot) DroppedLinks() int { return s.droppedLinkCount } // DroppedEvents returns the number of events dropped by the span due to // limits being reached. -func (s SpanSnapshot) DroppedEvents() int { +func (s snapshot) DroppedEvents() int { return s.droppedEventCount } // ChildSpanCount returns the count of spans that consider the span a // direct parent. -func (s SpanSnapshot) ChildSpanCount() int { +func (s snapshot) ChildSpanCount() int { return s.childSpanCount } // TODO: remove this. -func (s SpanSnapshot) Snapshot() *SpanSnapshot { +func (s snapshot) Snapshot() *snapshot { return &s } diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 07dba668c76..374975d5f6a 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -477,7 +477,7 @@ func (s *span) ChildSpanCount() int { // snapshot creates a read-only copy of the current state of the span. func (s *span) snapshot() ReadOnlySpan { - var sd SpanSnapshot + var sd snapshot s.mu.Lock() defer s.mu.Unlock() diff --git a/sdk/trace/span_exporter.go b/sdk/trace/span_exporter.go index ab9403a7dd2..2d5400223e5 100644 --- a/sdk/trace/span_exporter.go +++ b/sdk/trace/span_exporter.go @@ -30,7 +30,7 @@ type SpanExporter interface { // calls this function will not implement any retry logic. All errors // returned by this function are considered unrecoverable and will be // reported to a configured error Handler. - ExportSpans(ctx context.Context, ss []ReadOnlySpan) error + ExportSpans(ctx context.Context, spans []ReadOnlySpan) error // Shutdown notifies the exporter of a pending halt to operations. The // exporter is expected to preform any cleanup or synchronization it // requires while honoring all timeouts and cancellations contained in diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 56efa872455..a340ddb1bae 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -103,36 +103,36 @@ func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) { type testExporter struct { mu sync.RWMutex idx map[string]int - spans []*SpanSnapshot + spans []*snapshot } func NewTestExporter() *testExporter { return &testExporter{idx: make(map[string]int)} } -func (te *testExporter) ExportSpans(_ context.Context, ss []ReadOnlySpan) error { +func (te *testExporter) ExportSpans(_ context.Context, spans []ReadOnlySpan) error { te.mu.Lock() defer te.mu.Unlock() i := len(te.spans) - for _, s := range ss { + for _, s := range spans { te.idx[s.Name()] = i - te.spans = append(te.spans, s.(*SpanSnapshot)) + te.spans = append(te.spans, s.(*snapshot)) i++ } return nil } -func (te *testExporter) Spans() []*SpanSnapshot { +func (te *testExporter) Spans() []*snapshot { te.mu.RLock() defer te.mu.RUnlock() - cp := make([]*SpanSnapshot, len(te.spans)) + cp := make([]*snapshot, len(te.spans)) copy(cp, te.spans) return cp } -func (te *testExporter) GetSpan(name string) (*SpanSnapshot, bool) { +func (te *testExporter) GetSpan(name string) (*snapshot, bool) { te.mu.RLock() defer te.mu.RUnlock() i, ok := te.idx[name] @@ -389,7 +389,7 @@ func TestSetSpanAttributesOnStart(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -418,7 +418,7 @@ func TestSetSpanAttributes(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -474,7 +474,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -508,7 +508,7 @@ func TestSetSpanAttributesWithInvalidKey(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -552,7 +552,7 @@ func TestEvents(t *testing.T) { } } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -601,7 +601,7 @@ func TestEventsOverLimit(t *testing.T) { } } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -643,7 +643,7 @@ func TestLinks(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -684,7 +684,7 @@ func TestLinksOverLimit(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -733,7 +733,7 @@ func TestSetSpanStatus(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -763,7 +763,7 @@ func TestSetSpanStatusWithoutMessageWhenStatusIsNotError(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -784,7 +784,7 @@ func TestSetSpanStatusWithoutMessageWhenStatusIsNotError(t *testing.T) { func cmpDiff(x, y interface{}) string { return cmp.Diff(x, y, - cmp.AllowUnexported(SpanSnapshot{}), + cmp.AllowUnexported(snapshot{}), cmp.AllowUnexported(attribute.Value{}), cmp.AllowUnexported(Event{}), cmp.AllowUnexported(trace.TraceState{})) @@ -852,7 +852,7 @@ func startLocalSpan(tp *TracerProvider, ctx context.Context, trName, name string // // It also does some basic tests on the span. // It also clears spanID in the to make the comparison easier. -func endSpan(te *testExporter, span trace.Span) (*SpanSnapshot, error) { +func endSpan(te *testExporter, span trace.Span) (*snapshot, error) { if !span.IsRecording() { return nil, fmt.Errorf("IsRecording: got false, want true") } @@ -1114,7 +1114,7 @@ func TestRecordError(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -1153,7 +1153,7 @@ func TestRecordErrorNil(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -1263,7 +1263,7 @@ func TestWithResource(t *testing.T) { if err != nil { t.Error(err.Error()) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -1299,7 +1299,7 @@ func TestWithInstrumentationVersion(t *testing.T) { t.Error(err.Error()) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -1487,7 +1487,7 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { } } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, @@ -1546,7 +1546,7 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { t.Fatal(err) } - want := &SpanSnapshot{ + want := &snapshot{ spanContext: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: tid, TraceFlags: 0x1, diff --git a/sdk/trace/tracetest/test.go b/sdk/trace/tracetest/test.go index 180e5869546..104489e79fd 100644 --- a/sdk/trace/tracetest/test.go +++ b/sdk/trace/tracetest/test.go @@ -31,11 +31,11 @@ func NewNoopExporter() *NoopExporter { return new(NoopExporter) } -// NoopExporter is an exporter that drops all received SpanSnapshots and -// performs no action. +// NoopExporter is an exporter that drops all received spans and performs no +// action. type NoopExporter struct{} -// ExportSpans handles export of SpanSnapshots by dropping them. +// ExportSpans handles export of spans by dropping them. func (nsb *NoopExporter) ExportSpans(context.Context, []trace.ReadOnlySpan) error { return nil } // Shutdown stops the exporter by doing nothing. @@ -54,15 +54,15 @@ type InMemoryExporter struct { ss SpanStubs } -// ExportSpans handles export of SpanSnapshots by storing them in memory. -func (imsb *InMemoryExporter) ExportSpans(_ context.Context, ss []trace.ReadOnlySpan) error { +// ExportSpans handles export of spans by storing them in memory. +func (imsb *InMemoryExporter) ExportSpans(_ context.Context, spans []trace.ReadOnlySpan) error { imsb.mu.Lock() defer imsb.mu.Unlock() - imsb.ss = append(imsb.ss, SpanStubsFromReadOnlySpans(ss)...) + imsb.ss = append(imsb.ss, SpanStubsFromReadOnlySpans(spans)...) return nil } -// Shutdown stops the exporter by clearing SpanSnapshots held in memory. +// Shutdown stops the exporter by clearing spans held in memory. func (imsb *InMemoryExporter) Shutdown(context.Context) error { imsb.Reset() return nil From 7eb21f19a890352874a39b814c9f8e8088a872dd Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 2 May 2021 10:21:04 -0700 Subject: [PATCH 12/14] Remove Snapshot method from snapshot type This method is a hold-over from previous version of the ReadOnlySpan interface is not needed. --- sdk/trace/snapshot.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sdk/trace/snapshot.go b/sdk/trace/snapshot.go index 6bd232cc2b0..847617a382b 100644 --- a/sdk/trace/snapshot.go +++ b/sdk/trace/snapshot.go @@ -136,8 +136,3 @@ func (s snapshot) DroppedEvents() int { func (s snapshot) ChildSpanCount() int { return s.childSpanCount } - -// TODO: remove this. -func (s snapshot) Snapshot() *snapshot { - return &s -} From 973ec4fc584941bcaa61f4f4394071be291aef44 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 3 May 2021 12:26:41 -0700 Subject: [PATCH 13/14] Update CHANGELOG with changes --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14aae8ad00e..b98708e4caa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | 14 | Unavailable | | 15 | Data Loss | - The `Status` type was added to the `go.opentelemetry.io/otel/sdk/trace` package to represent the status of a span. (#1874) +- The `SpanStub` type and its associated functions were added to the `go.opentelemetry.io/otel/sdk/trace/tracetest` package. + This type can be used as a testing replacement for the `SpanSnapshot` that was removed from the `go.opentelemetry.io/otel/sdk/trace` package. (#1873) ### Changed @@ -34,6 +36,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Renamed `CloudZoneKey` to `CloudAvailabilityZoneKey` in Resource semantic conventions according to spec. (#1871) - The `StatusCode` and `StatusMessage` methods of the `ReadOnlySpan` interface and the `Span` produced by the `go.opentelemetry.io/otel/sdk/trace` package have been replaced with a single `Status` method. This method returns the status of a span using the new `Status` type. (#1874) +- The `ExportSpans` method of the`SpanExporter` interface type was updated to accept `ReadOnlySpan`s instead of the removed `SpanSnapshot`. + This brings the export interface into compliance with the specification in that it now accepts an explicitly immutable type instead of just an implied one. (#1873) ### Deprecated @@ -41,6 +45,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Remove `resource.WithoutBuiltin()`. Use `resource.New()`. (#1810) - Unexported types `resource.FromEnv`, `resource.Host`, and `resource.TelemetrySDK`, Use the corresponding `With*()` to use individually. (#1810) +- Removed the `Tracer` and `IsRecording` method from the `ReadOnlySpan` in the `go.opentelemetry.io/otel/sdk/trace`. + The `Tracer` method is not a required to be included in this interface and given the mutable nature of the tracer that is associated with a span, this method is not appropriate. + The `IsRecording` method returns if the span is recording or not. + A read-only span value does not need to know if updates to it will be recorded or not. + By definition, it cannot be updated so there is no point in communicating if an update is recorded. (#1873) +- Removed the `SpanSnapshot` type from the `go.opentelemetry.io/otel/sdk/trace` package. + The use of this type has been replaced with the use of the explicitly immutable `ReadOnlySpan` type. + When a concrete representation of a read-only span is needed for testing, the newly added `SpanStub` in the `go.opentelemetry.io/otel/sdk/trace/tracetest` package should be used. (#1873) ### Fixed From 187e4c022b1a63580517d8bd56032c1de8b44ccc Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 4 May 2021 16:39:49 -0700 Subject: [PATCH 14/14] Fix failure from sync with main --- exporters/trace/jaeger/agent_test.go | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/exporters/trace/jaeger/agent_test.go b/exporters/trace/jaeger/agent_test.go index 44a230cc343..82a152e982c 100644 --- a/exporters/trace/jaeger/agent_test.go +++ b/exporters/trace/jaeger/agent_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" - tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" ) func TestNewAgentClientUDPWithParamsBadHostport(t *testing.T) { @@ -113,10 +113,7 @@ func TestJaegerAgentUDPLimitBatching(t *testing.T) { // 1500 spans, size 79559, does not fit within one UDP packet with the default size of 65000. n := 1500 - s := make([]*tracesdk.SpanSnapshot, n) - for i := 0; i < n; i++ { - s[i] = &tracesdk.SpanSnapshot{} - } + s := make(tracetest.SpanStubs, n).Snapshots() exp, err := NewRawExporter( WithAgentEndpoint(WithAgentHost("localhost"), WithAgentPort("6831")), @@ -129,11 +126,10 @@ func TestJaegerAgentUDPLimitBatching(t *testing.T) { } // generateALargeSpan generates a span with a long name. -func generateALargeSpan() *tracesdk.SpanSnapshot { - span := &tracesdk.SpanSnapshot{ +func generateALargeSpan() tracetest.SpanStub { + return tracetest.SpanStub{ Name: "a-longer-name-that-makes-it-exceeds-limit", } - return span } func TestSpanExceedsMaxPacketLimit(t *testing.T) { @@ -141,10 +137,9 @@ func TestSpanExceedsMaxPacketLimit(t *testing.T) { // 106 is the serialized size of a span with default values. maxSize := 106 - span := generateALargeSpan() - largeSpans := []*tracesdk.SpanSnapshot{span, {}} - normalSpans := []*tracesdk.SpanSnapshot{{}, {}} + largeSpans := tracetest.SpanStubs{generateALargeSpan(), {}}.Snapshots() + normalSpans := tracetest.SpanStubs{{}, {}}.Snapshots() exp, err := NewRawExporter( WithAgentEndpoint(WithAgentHost("localhost"), WithAgentPort("6831"), WithMaxPacketSize(maxSize+1)), @@ -161,7 +156,7 @@ func TestEmitBatchWithMultipleErrors(t *testing.T) { otel.SetErrorHandler(errorHandler{t}) span := generateALargeSpan() - largeSpans := []*tracesdk.SpanSnapshot{span, span} + largeSpans := tracetest.SpanStubs{span, span}.Snapshots() // make max packet size smaller than span maxSize := len(span.Name) exp, err := NewRawExporter(