diff --git a/CHANGELOG.md b/CHANGELOG.md index 9092b5d618e..237b663b65e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,20 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add the `ReadOnlySpan` and `ReadWriteSpan` interfaces to provide better control for accessing span data. (#1360) + +### Changed + +- Rename `export.SpanData` to `export.SpanSnapshot` and use it only for exporting spans. (#1360) +- Store the parent's full `SpanContext` rather than just its span ID in the `span` struct. (#1360) +- Improve span duration accuracy. (#1360) + +### Removed + +- Remove `errUninitializedSpan` as its only usage is now obsolete. (#1360) + ## [0.15.0] - 2020-12-10 ### Added diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index 353ce2399fa..7e575a6d14d 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -28,8 +28,9 @@ const ( maxMessageEventsPerSpan = 128 ) -// SpanData transforms a slice of SpanData into a slice of OTLP ResourceSpans. -func SpanData(sdl []*export.SpanData) []*tracepb.ResourceSpans { +// SpanData transforms a slice of SpanSnapshot into a slice of OTLP +// ResourceSpans. +func SpanData(sdl []*export.SpanSnapshot) []*tracepb.ResourceSpans { if len(sdl) == 0 { return nil } @@ -95,7 +96,7 @@ func SpanData(sdl []*export.SpanData) []*tracepb.ResourceSpans { } // span transforms a Span into an OTLP span. -func span(sd *export.SpanData) *tracepb.Span { +func span(sd *export.SpanSnapshot) *tracepb.Span { if sd == nil { return nil } diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index 1c6b8eb1b92..1a80a18946a 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -199,7 +199,7 @@ func TestSpanData(t *testing.T) { // March 31, 2020 5:01:26 1234nanos (UTC) startTime := time.Unix(1585674086, 1234) endTime := startTime.Add(10 * time.Second) - spanData := &export.SpanData{ + spanData := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ 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}, @@ -279,7 +279,7 @@ func TestSpanData(t *testing.T) { DroppedLinksCount: 3, } - got := SpanData([]*export.SpanData{spanData}) + got := SpanData([]*export.SpanSnapshot{spanData}) require.Len(t, got, 1) assert.Equal(t, got[0].GetResource(), Resource(spanData.Resource)) @@ -296,7 +296,7 @@ func TestSpanData(t *testing.T) { // Empty parent span ID should be treated as root span. func TestRootSpanData(t *testing.T) { - sd := SpanData([]*export.SpanData{{}}) + sd := SpanData([]*export.SpanSnapshot{{}}) require.Len(t, sd, 1) rs := sd[0] got := rs.GetInstrumentationLibrarySpans()[0].GetSpans()[0].GetParentSpanId() @@ -306,5 +306,5 @@ func TestRootSpanData(t *testing.T) { } func TestSpanDataNilResource(t *testing.T) { - assert.NotPanics(t, func() { SpanData([]*export.SpanData{{}}) }) + assert.NotPanics(t, func() { SpanData([]*export.SpanSnapshot{{}}) }) } diff --git a/exporters/otlp/otlp.go b/exporters/otlp/otlp.go index f8c1c5b0e76..d040781e07c 100644 --- a/exporters/otlp/otlp.go +++ b/exporters/otlp/otlp.go @@ -197,12 +197,12 @@ func (e *Exporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) return e.exportKindSelector.ExportKindFor(desc, kind) } -// ExportSpans exports a batch of SpanData. -func (e *Exporter) ExportSpans(ctx context.Context, sds []*tracesdk.SpanData) error { - return e.uploadTraces(ctx, sds) +// ExportSpans exports a batch of SpanSnapshot. +func (e *Exporter) ExportSpans(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { + return e.uploadTraces(ctx, ss) } -func (e *Exporter) uploadTraces(ctx context.Context, sdl []*tracesdk.SpanData) error { +func (e *Exporter) uploadTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error { ctx, cancel := e.cc.contextWithStop(ctx) defer cancel() @@ -210,7 +210,7 @@ func (e *Exporter) uploadTraces(ctx context.Context, sdl []*tracesdk.SpanData) e return nil } - protoSpans := transform.SpanData(sdl) + protoSpans := transform.SpanData(ss) if len(protoSpans) == 0 { return nil } diff --git a/exporters/otlp/otlp_integration_test.go b/exporters/otlp/otlp_integration_test.go index ced8d103558..c452bcfa1e0 100644 --- a/exporters/otlp/otlp_integration_test.go +++ b/exporters/otlp/otlp_integration_test.go @@ -367,7 +367,7 @@ func TestNewExporter_collectorConnectionDiesThenReconnects(t *testing.T) { // No endpoint up. require.Error( t, - exp.ExportSpans(ctx, []*exporttrace.SpanData{{Name: "in the midst"}}), + exp.ExportSpans(ctx, []*exporttrace.SpanSnapshot{{Name: "in the midst"}}), "transport: Error while dialing dial tcp %s: connect: connection refused", mc.address, ) @@ -381,11 +381,11 @@ func TestNewExporter_collectorConnectionDiesThenReconnects(t *testing.T) { n := 10 for i := 0; i < n; i++ { - require.NoError(t, exp.ExportSpans(ctx, []*exporttrace.SpanData{{Name: "Resurrected"}})) + require.NoError(t, exp.ExportSpans(ctx, []*exporttrace.SpanSnapshot{{Name: "Resurrected"}})) } nmaSpans := nmc.getSpans() - // Expecting 10 spanData that were sampled, given that + // Expecting 10 SpanSnapshots 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) } @@ -461,7 +461,7 @@ func TestNewExporter_withHeaders(t *testing.T) { otlp.WithAddress(mc.address), otlp.WithHeaders(map[string]string{"header1": "value1"}), ) - require.NoError(t, exp.ExportSpans(ctx, []*exporttrace.SpanData{{Name: "in the midst"}})) + require.NoError(t, exp.ExportSpans(ctx, []*exporttrace.SpanSnapshot{{Name: "in the midst"}})) defer func() { _ = exp.Shutdown(ctx) diff --git a/exporters/otlp/otlp_span_test.go b/exporters/otlp/otlp_span_test.go index ac7ec652772..e967e507d17 100644 --- a/exporters/otlp/otlp_span_test.go +++ b/exporters/otlp/otlp_span_test.go @@ -68,19 +68,19 @@ func TestExportSpans(t *testing.T) { endTime := startTime.Add(10 * time.Second) for _, test := range []struct { - sd []*tracesdk.SpanData + sd []*tracesdk.SpanSnapshot want []tracepb.ResourceSpans }{ { - []*tracesdk.SpanData(nil), + []*tracesdk.SpanSnapshot(nil), []tracepb.ResourceSpans(nil), }, { - []*tracesdk.SpanData{}, + []*tracesdk.SpanSnapshot{}, []tracepb.ResourceSpans(nil), }, { - []*tracesdk.SpanData{ + []*tracesdk.SpanSnapshot{ { SpanContext: trace.SpanContext{ TraceID: trace.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}), diff --git a/exporters/stdout/trace.go b/exporters/stdout/trace.go index 85a595dbca3..a80e35d174e 100644 --- a/exporters/stdout/trace.go +++ b/exporters/stdout/trace.go @@ -31,8 +31,8 @@ type traceExporter struct { stopped bool } -// ExportSpans writes SpanData in json format to stdout. -func (e *traceExporter) ExportSpans(ctx context.Context, data []*trace.SpanData) error { +// ExportSpans writes SpanSnapshots in json format to stdout. +func (e *traceExporter) ExportSpans(ctx context.Context, ss []*trace.SpanSnapshot) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -40,10 +40,10 @@ func (e *traceExporter) ExportSpans(ctx context.Context, data []*trace.SpanData) return nil } - if e.config.DisableTraceExport || len(data) == 0 { + if e.config.DisableTraceExport || len(ss) == 0 { return nil } - out, err := e.marshal(data) + out, err := e.marshal(ss) if err != nil { return err } diff --git a/exporters/stdout/trace_test.go b/exporters/stdout/trace_test.go index 83df6fe4ab4..ed2f4f31030 100644 --- a/exporters/stdout/trace_test.go +++ b/exporters/stdout/trace_test.go @@ -46,7 +46,7 @@ func TestExporter_ExportSpan(t *testing.T) { doubleValue := 123.456 resource := resource.NewWithAttributes(label.String("rk1", "rv11")) - testSpan := &export.SpanData{ + testSpan := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -67,7 +67,7 @@ func TestExporter_ExportSpan(t *testing.T) { StatusMessage: "interesting", Resource: resource, } - if err := ex.ExportSpans(context.Background(), []*export.SpanData{testSpan}); err != nil { + if err := ex.ExportSpans(context.Background(), []*export.SpanSnapshot{testSpan}); err != nil { t.Fatal(err) } diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index bcee6b8e2b7..cfd3f71ebee 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -210,8 +210,8 @@ type Exporter struct { var _ export.SpanExporter = (*Exporter)(nil) -// ExportSpans exports SpanData to Jaeger. -func (e *Exporter) ExportSpans(ctx context.Context, spans []*export.SpanData) error { +// ExportSpans exports SpanSnapshots to Jaeger. +func (e *Exporter) ExportSpans(ctx context.Context, ss []*export.SpanSnapshot) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -219,9 +219,9 @@ func (e *Exporter) ExportSpans(ctx context.Context, spans []*export.SpanData) er return nil } - for _, span := range spans { + for _, span := range ss { // TODO(jbd): Handle oversized bundlers. - err := e.bundler.Add(spanDataToThrift(span), 1) + err := e.bundler.Add(spanSnapshotToThrift(span), 1) if err != nil { return fmt.Errorf("failed to bundle %q: %w", span.Name, err) } @@ -260,9 +260,9 @@ func (e *Exporter) Shutdown(ctx context.Context) error { return nil } -func spanDataToThrift(data *export.SpanData) *gen.Span { - tags := make([]*gen.Tag, 0, len(data.Attributes)) - for _, kv := range data.Attributes { +func spanSnapshotToThrift(ss *export.SpanSnapshot) *gen.Span { + tags := make([]*gen.Tag, 0, len(ss.Attributes)) + for _, kv := range ss.Attributes { tag := keyValueToTag(kv) if tag != nil { tags = append(tags, tag) @@ -273,14 +273,14 @@ func spanDataToThrift(data *export.SpanData) *gen.Span { // semantic. Should resources be appended before span // attributes, above, to allow span attributes to // overwrite resource attributes? - if data.Resource != nil { - for iter := data.Resource.Iter(); iter.Next(); { + if ss.Resource != nil { + for iter := ss.Resource.Iter(); iter.Next(); { if tag := keyValueToTag(iter.Attribute()); tag != nil { tags = append(tags, tag) } } } - if il := data.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)) @@ -288,19 +288,19 @@ func spanDataToThrift(data *export.SpanData) *gen.Span { } tags = append(tags, - getInt64Tag("status.code", int64(data.StatusCode)), - getStringTag("status.message", data.StatusMessage), - getStringTag("span.kind", data.SpanKind.String()), + getInt64Tag("status.code", int64(ss.StatusCode)), + getStringTag("status.message", ss.StatusMessage), + getStringTag("span.kind", ss.SpanKind.String()), ) // Ensure that if Status.Code is not OK, that we set the "error" tag on the Jaeger span. // See Issue https://github.com/census-instrumentation/opencensus-go/issues/1041 - if data.StatusCode != codes.Ok && data.StatusCode != codes.Unset { + if ss.StatusCode != codes.Ok && ss.StatusCode != codes.Unset { tags = append(tags, getBoolTag("error", true)) } var logs []*gen.Log - for _, a := range data.MessageEvents { + for _, a := range ss.MessageEvents { fields := make([]*gen.Tag, 0, len(a.Attributes)) for _, kv := range a.Attributes { tag := keyValueToTag(kv) @@ -316,7 +316,7 @@ func spanDataToThrift(data *export.SpanData) *gen.Span { } var refs []*gen.SpanRef - for _, link := range data.Links { + for _, link := range ss.Links { refs = append(refs, &gen.SpanRef{ TraceIdHigh: int64(binary.BigEndian.Uint64(link.TraceID[0:8])), TraceIdLow: int64(binary.BigEndian.Uint64(link.TraceID[8:16])), @@ -328,14 +328,14 @@ func spanDataToThrift(data *export.SpanData) *gen.Span { } return &gen.Span{ - TraceIdHigh: int64(binary.BigEndian.Uint64(data.SpanContext.TraceID[0:8])), - TraceIdLow: int64(binary.BigEndian.Uint64(data.SpanContext.TraceID[8:16])), - SpanId: int64(binary.BigEndian.Uint64(data.SpanContext.SpanID[:])), - ParentSpanId: int64(binary.BigEndian.Uint64(data.ParentSpanID[:])), - OperationName: data.Name, // TODO: if span kind is added then add prefix "Sent"/"Recv" - Flags: int32(data.SpanContext.TraceFlags), - StartTime: data.StartTime.UnixNano() / 1000, - Duration: data.EndTime.Sub(data.StartTime).Nanoseconds() / 1000, + TraceIdHigh: int64(binary.BigEndian.Uint64(ss.SpanContext.TraceID[0:8])), + TraceIdLow: int64(binary.BigEndian.Uint64(ss.SpanContext.TraceID[8:16])), + SpanId: int64(binary.BigEndian.Uint64(ss.SpanContext.SpanID[:])), + ParentSpanId: int64(binary.BigEndian.Uint64(ss.ParentSpanID[:])), + 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, diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index 2096a61d677..eea460b18c8 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -353,7 +353,7 @@ func TestExporter_ExportSpan(t *testing.T) { assert.True(t, len(tc.spansUploaded) == 1) } -func Test_spanDataToThrift(t *testing.T) { +func Test_spanSnapshotToThrift(t *testing.T) { now := time.Now() traceID, _ := trace.TraceIDFromHex("0102030405060708090a0b0c0d0e0f10") spanID, _ := trace.SpanIDFromHex("0102030405060708") @@ -376,12 +376,12 @@ func Test_spanDataToThrift(t *testing.T) { tests := []struct { name string - data *export.SpanData + data *export.SpanSnapshot want *gen.Span }{ { name: "no parent", - data: &export.SpanData{ + data: &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -465,7 +465,7 @@ func Test_spanDataToThrift(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := spanDataToThrift(tt.data) + got := spanSnapshotToThrift(tt.data) sort.Slice(got.Tags, func(i, j int) bool { return got.Tags[i].Key < got.Tags[j].Key }) diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go index e9d264b68aa..fe5454b40aa 100644 --- a/exporters/trace/zipkin/model.go +++ b/exporters/trace/zipkin/model.go @@ -31,7 +31,7 @@ const ( keyInstrumentationLibraryVersion = "otel.instrumentation_library.version" ) -func toZipkinSpanModels(batch []*export.SpanData, serviceName string) []zkmodel.SpanModel { +func toZipkinSpanModels(batch []*export.SpanSnapshot, serviceName string) []zkmodel.SpanModel { models := make([]zkmodel.SpanModel, 0, len(batch)) for _, data := range batch { models = append(models, toZipkinSpanModel(data, serviceName)) @@ -39,7 +39,7 @@ func toZipkinSpanModels(batch []*export.SpanData, serviceName string) []zkmodel. return models } -func toZipkinSpanModel(data *export.SpanData, serviceName string) zkmodel.SpanModel { +func toZipkinSpanModel(data *export.SpanSnapshot, serviceName string) zkmodel.SpanModel { return zkmodel.SpanModel{ SpanContext: toZipkinSpanContext(data), Name: data.Name, @@ -56,7 +56,7 @@ func toZipkinSpanModel(data *export.SpanData, serviceName string) zkmodel.SpanMo } } -func toZipkinSpanContext(data *export.SpanData) zkmodel.SpanContext { +func toZipkinSpanContext(data *export.SpanSnapshot) zkmodel.SpanContext { return zkmodel.SpanContext{ TraceID: toZipkinTraceID(data.SpanContext.TraceID), ID: toZipkinID(data.SpanContext.SpanID), @@ -145,7 +145,7 @@ var extraZipkinTags = []string{ keyInstrumentationLibraryVersion, } -func toZipkinTags(data *export.SpanData) map[string]string { +func toZipkinTags(data *export.SpanSnapshot) map[string]string { m := make(map[string]string, len(data.Attributes)+len(extraZipkinTags)) for _, kv := range data.Attributes { m[(string)(kv.Key)] = kv.Value.Emit() diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go index ef0123341c8..51ec10214c1 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -32,7 +32,7 @@ import ( ) func TestModelConversion(t *testing.T) { - inputBatch := []*export.SpanData{ + inputBatch := []*export.SpanSnapshot{ // typical span data { SpanContext: trace.SpanContext{ @@ -671,12 +671,12 @@ func Test_toZipkinTags(t *testing.T) { tests := []struct { name string - data *export.SpanData + data *export.SpanSnapshot want map[string]string }{ { name: "attributes", - data: &export.SpanData{ + data: &export.SpanSnapshot{ Attributes: []label.KeyValue{ label.String("key", keyValue), label.Float64("double", doubleValue), @@ -695,7 +695,7 @@ func Test_toZipkinTags(t *testing.T) { }, { name: "no attributes", - data: &export.SpanData{}, + data: &export.SpanSnapshot{}, want: map[string]string{ "otel.status_code": codes.Unset.String(), "otel.status_description": "", @@ -703,7 +703,7 @@ func Test_toZipkinTags(t *testing.T) { }, { name: "omit-noerror", - data: &export.SpanData{ + data: &export.SpanSnapshot{ Attributes: []label.KeyValue{ label.Bool("error", false), }, @@ -715,7 +715,7 @@ func Test_toZipkinTags(t *testing.T) { }, { name: "statusCode", - data: &export.SpanData{ + data: &export.SpanSnapshot{ Attributes: []label.KeyValue{ label.String("key", keyValue), label.Bool("error", true), @@ -732,7 +732,7 @@ func Test_toZipkinTags(t *testing.T) { }, { name: "instrLib-empty", - data: &export.SpanData{ + data: &export.SpanSnapshot{ InstrumentationLibrary: instrumentation.Library{}, }, want: map[string]string{ @@ -742,7 +742,7 @@ func Test_toZipkinTags(t *testing.T) { }, { name: "instrLib-noversion", - data: &export.SpanData{ + data: &export.SpanSnapshot{ Attributes: []label.KeyValue{}, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, @@ -756,7 +756,7 @@ func Test_toZipkinTags(t *testing.T) { }, { name: "instrLib-with-version", - data: &export.SpanData{ + data: &export.SpanSnapshot{ Attributes: []label.KeyValue{}, InstrumentationLibrary: instrumentation.Library{ Name: instrLibName, diff --git a/exporters/trace/zipkin/zipkin.go b/exporters/trace/zipkin/zipkin.go index 935331451a9..63b3a3e9d9f 100644 --- a/exporters/trace/zipkin/zipkin.go +++ b/exporters/trace/zipkin/zipkin.go @@ -32,7 +32,7 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" ) -// Exporter exports SpanData to the zipkin collector. It implements +// 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. type Exporter struct { @@ -138,8 +138,8 @@ func InstallNewPipeline(collectorURL, serviceName string, opts ...Option) error return nil } -// ExportSpans exports SpanData to a Zipkin receiver. -func (e *Exporter) ExportSpans(ctx context.Context, batch []*export.SpanData) error { +// ExportSpans exports SpanSnapshots to a Zipkin receiver. +func (e *Exporter) ExportSpans(ctx context.Context, ss []*export.SpanSnapshot) error { e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -148,11 +148,11 @@ func (e *Exporter) ExportSpans(ctx context.Context, batch []*export.SpanData) er return nil } - if len(batch) == 0 { + if len(ss) == 0 { e.logf("no spans to export") return nil } - models := toZipkinSpanModels(batch, e.serviceName) + models := toZipkinSpanModels(ss, e.serviceName) body, err := json.Marshal(models) if err != nil { return e.errf("failed to serialize zipkin models to JSON: %v", err) diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index e469f5d88e6..cf04112a5aa 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/exporters/trace/zipkin/zipkin_test.go @@ -239,7 +239,7 @@ func logStoreLogger(s *logStore) *log.Logger { } func TestExportSpans(t *testing.T) { - spans := []*export.SpanData{ + spans := []*export.SpanSnapshot{ // parent { SpanContext: trace.SpanContext{ diff --git a/sdk/export/trace/trace.go b/sdk/export/trace/trace.go index a9883bff73f..96b45b92f6b 100644 --- a/sdk/export/trace/trace.go +++ b/sdk/export/trace/trace.go @@ -26,10 +26,10 @@ import ( "go.opentelemetry.io/otel/sdk/resource" ) -// SpanExporter handles the delivery of SpanData to external receivers. This is -// the final component in the trace export pipeline. +// SpanExporter handles the delivery of SpanSnapshot structs to external +// receivers. This is the final component in the trace export pipeline. type SpanExporter interface { - // ExportSpans exports a batch of SpanData. + // ExportSpans exports a batch of SpanSnapshots. // // This function is called synchronously, so there is no concurrency // safety requirement. However, due to the synchronous calling pattern, @@ -40,7 +40,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, spanData []*SpanData) error + ExportSpans(ctx context.Context, ss []*SpanSnapshot) 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 @@ -48,8 +48,12 @@ type SpanExporter interface { Shutdown(ctx context.Context) error } -// SpanData contains all the information collected by a completed span. -type SpanData struct { +// 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 ParentSpanID trace.SpanID SpanKind trace.SpanKind diff --git a/sdk/export/trace/tracetest/test.go b/sdk/export/trace/tracetest/test.go index bd01f97445b..e1bedad6bfb 100644 --- a/sdk/export/trace/tracetest/test.go +++ b/sdk/export/trace/tracetest/test.go @@ -30,12 +30,12 @@ func NewNoopExporter() *NoopExporter { return new(NoopExporter) } -// NoopExporter is an exporter that drops all received SpanData and performs -// no action. +// NoopExporter is an exporter that drops all received SpanSnapshots and +// performs no action. type NoopExporter struct{} -// ExportSpans handles export of SpanData by dropping it. -func (nsb *NoopExporter) ExportSpans(context.Context, []*trace.SpanData) error { return nil } +// ExportSpans handles export of SpanSnapshots by dropping them. +func (nsb *NoopExporter) ExportSpans(context.Context, []*trace.SpanSnapshot) error { return nil } // Shutdown stops the exporter by doing nothing. func (nsb *NoopExporter) Shutdown(context.Context) error { return nil } @@ -49,19 +49,19 @@ func NewInMemoryExporter() *InMemoryExporter { // InMemoryExporter is an exporter that stores all received spans in-memory. type InMemoryExporter struct { - mu sync.Mutex - sds []*trace.SpanData + mu sync.Mutex + ss []*trace.SpanSnapshot } -// ExportSpans handles export of SpanData by storing it in memory. -func (imsb *InMemoryExporter) ExportSpans(_ context.Context, sds []*trace.SpanData) error { +// ExportSpans handles export of SpanSnapshots by storing them in memory. +func (imsb *InMemoryExporter) ExportSpans(_ context.Context, ss []*trace.SpanSnapshot) error { imsb.mu.Lock() defer imsb.mu.Unlock() - imsb.sds = append(imsb.sds, sds...) + imsb.ss = append(imsb.ss, ss...) return nil } -// Shutdown stops the exporter by clearing SpanData held in memory. +// Shutdown stops the exporter by clearing SpanSnapshots held in memory. func (imsb *InMemoryExporter) Shutdown(context.Context) error { imsb.Reset() return nil @@ -71,14 +71,14 @@ func (imsb *InMemoryExporter) Shutdown(context.Context) error { func (imsb *InMemoryExporter) Reset() { imsb.mu.Lock() defer imsb.mu.Unlock() - imsb.sds = nil + imsb.ss = nil } // GetSpans returns the current in-memory stored spans. -func (imsb *InMemoryExporter) GetSpans() []*trace.SpanData { +func (imsb *InMemoryExporter) GetSpans() []*trace.SpanSnapshot { imsb.mu.Lock() defer imsb.mu.Unlock() - ret := make([]*trace.SpanData, len(imsb.sds)) - copy(ret, imsb.sds) + ret := make([]*trace.SpanSnapshot, len(imsb.ss)) + copy(ret, imsb.ss) return ret } diff --git a/sdk/export/trace/tracetest/test_test.go b/sdk/export/trace/tracetest/test_test.go index 378a71f7d57..380109c0ff8 100644 --- a/sdk/export/trace/tracetest/test_test.go +++ b/sdk/export/trace/tracetest/test_test.go @@ -29,8 +29,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.SpanData, 10))) - require.NoError(t, nsb.ExportSpans(context.Background(), make([]*trace.SpanData, 0, 10))) + require.NoError(t, nsb.ExportSpans(context.Background(), make([]*trace.SpanSnapshot, 10))) + require.NoError(t, nsb.ExportSpans(context.Background(), make([]*trace.SpanSnapshot, 0, 10))) } func TestNewInMemoryExporter(t *testing.T) { @@ -39,9 +39,9 @@ func TestNewInMemoryExporter(t *testing.T) { require.NoError(t, imsb.ExportSpans(context.Background(), nil)) assert.Len(t, imsb.GetSpans(), 0) - input := make([]*trace.SpanData, 10) + input := make([]*trace.SpanSnapshot, 10) for i := 0; i < 10; i++ { - input[i] = new(trace.SpanData) + input[i] = new(trace.SpanSnapshot) } require.NoError(t, imsb.ExportSpans(context.Background(), input)) sds := imsb.GetSpans() diff --git a/sdk/trace/attributesmap.go b/sdk/trace/attributesmap.go index fc1aa8f4323..6b2f1346306 100644 --- a/sdk/trace/attributesmap.go +++ b/sdk/trace/attributesmap.go @@ -18,12 +18,11 @@ import ( "container/list" "go.opentelemetry.io/otel/label" - "go.opentelemetry.io/otel/sdk/export/trace" ) // attributesMap is a capped map of attributes, holding the most recent attributes. // Eviction is done via a LRU method, the oldest entry is removed to create room for a new entry. -// Updates are allowed and refreshes the usage of the key. +// Updates are allowed and they refresh the usage of the key. // // This is based from https://github.com/hashicorp/golang-lru/blob/master/simplelru/lru.go // With a subset of the its operations and specific for holding label.KeyValue @@ -62,10 +61,13 @@ func (am *attributesMap) add(kv label.KeyValue) { } } -func (am *attributesMap) toSpanData(sd *trace.SpanData) { +// toKeyValue copies the attributesMap into a slice of label.KeyValue and +// returns it. If the map is empty, a nil is returned. +// TODO: Is it more efficient to return a pointer to the slice? +func (am *attributesMap) toKeyValue() []label.KeyValue { len := am.evictList.Len() if len == 0 { - return + return nil } attributes := make([]label.KeyValue, 0, len) @@ -75,8 +77,7 @@ func (am *attributesMap) toSpanData(sd *trace.SpanData) { } } - sd.Attributes = attributes - sd.DroppedAttributeCount = am.droppedCount + return attributes } // removeOldest removes the oldest item from the cache. diff --git a/sdk/trace/attributesmap_test.go b/sdk/trace/attributesmap_test.go index be16e45970b..defa378b36b 100644 --- a/sdk/trace/attributesmap_test.go +++ b/sdk/trace/attributesmap_test.go @@ -19,7 +19,6 @@ import ( "testing" "go.opentelemetry.io/otel/label" - export "go.opentelemetry.io/otel/sdk/export/trace" ) const testKeyFmt = "test-key-%d" @@ -75,24 +74,30 @@ func TestAttributesMapGetOldestRemoveOldest(t *testing.T) { } } -func TestAttributesMapToSpanData(t *testing.T) { +func TestAttributesMapToKeyValue(t *testing.T) { attrMap := newAttributesMap(128) for i := 0; i < 128; i++ { attrMap.add(label.Int(fmt.Sprintf(testKeyFmt, i), i)) } - sd := &export.SpanData{} + kv := attrMap.toKeyValue() - attrMap.toSpanData(sd) + gotAttrLen := len(kv) + wantAttrLen := 128 + if gotAttrLen != wantAttrLen { + t.Errorf("len(attrMap.attributes): got '%d'; want '%d'", gotAttrLen, wantAttrLen) + } +} + +func BenchmarkAttributesMapToKeyValue(b *testing.B) { + attrMap := newAttributesMap(128) - if attrMap.droppedCount != sd.DroppedAttributeCount { - t.Errorf("attrMap.droppedCount: got '%d'; want '%d'", attrMap.droppedCount, sd.DroppedAttributeCount) + for i := 0; i < 128; i++ { + attrMap.add(label.Int(fmt.Sprintf(testKeyFmt, i), i)) } - gotAttrLen := len(attrMap.attributes) - wantAttrLen := len(sd.Attributes) - if gotAttrLen != wantAttrLen { - t.Errorf("len(attrMap.attributes): got '%d'; want '%d'", gotAttrLen, wantAttrLen) + for n := 0; n < b.N; n++ { + attrMap.toKeyValue() } } diff --git a/sdk/trace/batch_span_processor.go b/sdk/trace/batch_span_processor.go index cd13733fa66..ad4ee29e7c9 100644 --- a/sdk/trace/batch_span_processor.go +++ b/sdk/trace/batch_span_processor.go @@ -57,16 +57,16 @@ type BatchSpanProcessorOptions struct { BlockOnQueueFull bool } -// BatchSpanProcessor is a SpanProcessor that batches asynchronously received -// SpanData and sends it to a trace.Exporter when complete. +// BatchSpanProcessor is a SpanProcessor that batches asynchronously-received +// SpanSnapshots and sends them to a trace.Exporter when complete. type BatchSpanProcessor struct { e export.SpanExporter o BatchSpanProcessorOptions - queue chan *export.SpanData + queue chan *export.SpanSnapshot dropped uint32 - batch []*export.SpanData + batch []*export.SpanSnapshot batchMutex sync.Mutex timer *time.Timer stopWait sync.WaitGroup @@ -77,7 +77,7 @@ type BatchSpanProcessor struct { var _ SpanProcessor = (*BatchSpanProcessor)(nil) // NewBatchSpanProcessor creates a new BatchSpanProcessor that will send -// SpanData batches to the exporters with the supplied options. +// SpanSnapshot batches to the exporters with the supplied options. // // The returned BatchSpanProcessor needs to be registered with the SDK using // the RegisterSpanProcessor method for it to process spans. @@ -95,9 +95,9 @@ func NewBatchSpanProcessor(exporter export.SpanExporter, options ...BatchSpanPro bsp := &BatchSpanProcessor{ e: exporter, o: o, - batch: make([]*export.SpanData, 0, o.MaxExportBatchSize), + batch: make([]*export.SpanSnapshot, 0, o.MaxExportBatchSize), timer: time.NewTimer(o.BatchTimeout), - queue: make(chan *export.SpanData, o.MaxQueueSize), + queue: make(chan *export.SpanSnapshot, o.MaxQueueSize), stopCh: make(chan struct{}), } @@ -112,15 +112,15 @@ func NewBatchSpanProcessor(exporter export.SpanExporter, options ...BatchSpanPro } // OnStart method does nothing. -func (bsp *BatchSpanProcessor) OnStart(parent context.Context, sd *export.SpanData) {} +func (bsp *BatchSpanProcessor) OnStart(parent context.Context, s ReadWriteSpan) {} -// OnEnd method enqueues export.SpanData for later processing. -func (bsp *BatchSpanProcessor) OnEnd(sd *export.SpanData) { +// OnEnd method enqueues a ReadOnlySpan for later processing. +func (bsp *BatchSpanProcessor) OnEnd(s ReadOnlySpan) { // Do not enqueue spans if we are just going to drop them. if bsp.e == nil { return } - bsp.enqueue(sd) + bsp.enqueue(s.Snapshot()) } // Shutdown flushes the queue and waits until all spans are processed. @@ -240,7 +240,7 @@ func (bsp *BatchSpanProcessor) drainQueue() { } } -func (bsp *BatchSpanProcessor) enqueue(sd *export.SpanData) { +func (bsp *BatchSpanProcessor) enqueue(sd *export.SpanSnapshot) { if !sd.SpanContext.IsSampled() { return } diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go index 67f72ad06df..6675484885b 100644 --- a/sdk/trace/batch_span_processor_test.go +++ b/sdk/trace/batch_span_processor_test.go @@ -29,17 +29,17 @@ import ( type testBatchExporter struct { mu sync.Mutex - spans []*export.SpanData + spans []*export.SpanSnapshot sizes []int batchCount int } -func (t *testBatchExporter) ExportSpans(ctx context.Context, sds []*export.SpanData) error { +func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []*export.SpanSnapshot) error { t.mu.Lock() defer t.mu.Unlock() - t.spans = append(t.spans, sds...) - t.sizes = append(t.sizes, len(sds)) + t.spans = append(t.spans, ss...) + t.sizes = append(t.sizes, len(ss)) t.batchCount++ return nil } @@ -61,10 +61,17 @@ func (t *testBatchExporter) getBatchCount() int { var _ export.SpanExporter = (*testBatchExporter)(nil) func TestNewBatchSpanProcessorWithNilExporter(t *testing.T) { + tp := basicTracerProvider(t) bsp := sdktrace.NewBatchSpanProcessor(nil) + tp.RegisterSpanProcessor(bsp) + tr := tp.Tracer("NilExporter") + + _, span := tr.Start(context.Background(), "foo") + span.End() + // These should not panic. - bsp.OnStart(context.Background(), &export.SpanData{}) - bsp.OnEnd(&export.SpanData{}) + bsp.OnStart(context.Background(), span.(sdktrace.ReadWriteSpan)) + bsp.OnEnd(span.(sdktrace.ReadOnlySpan)) bsp.ForceFlush() err := bsp.Shutdown(context.Background()) if err != nil { diff --git a/sdk/trace/provider_test.go b/sdk/trace/provider_test.go index 2b7b7a69f47..ea0a2f870d5 100644 --- a/sdk/trace/provider_test.go +++ b/sdk/trace/provider_test.go @@ -17,8 +17,6 @@ package trace import ( "context" "testing" - - export "go.opentelemetry.io/otel/sdk/export/trace" ) type basicSpanProcesor struct { @@ -30,9 +28,9 @@ func (t *basicSpanProcesor) Shutdown(context.Context) error { return nil } -func (t *basicSpanProcesor) OnStart(parent context.Context, s *export.SpanData) {} -func (t *basicSpanProcesor) OnEnd(s *export.SpanData) {} -func (t *basicSpanProcesor) ForceFlush() {} +func (t *basicSpanProcesor) OnStart(parent context.Context, s ReadWriteSpan) {} +func (t *basicSpanProcesor) OnEnd(s ReadOnlySpan) {} +func (t *basicSpanProcesor) ForceFlush() {} func TestShutdownTraceProvider(t *testing.T) { stp := NewTracerProvider() diff --git a/sdk/trace/simple_span_processor.go b/sdk/trace/simple_span_processor.go index c60e9903564..1ab95aa5e11 100644 --- a/sdk/trace/simple_span_processor.go +++ b/sdk/trace/simple_span_processor.go @@ -22,7 +22,7 @@ import ( ) // SimpleSpanProcessor is a SpanProcessor that synchronously sends all -// SpanData to a trace.Exporter when the span finishes. +// SpanSnapshots to a trace.Exporter when the span finishes. type SimpleSpanProcessor struct { e export.SpanExporter } @@ -30,7 +30,7 @@ type SimpleSpanProcessor struct { var _ SpanProcessor = (*SimpleSpanProcessor)(nil) // NewSimpleSpanProcessor returns a new SimpleSpanProcessor that will -// synchronously send SpanData to the exporter. +// synchronously send SpanSnapshots to the exporter. func NewSimpleSpanProcessor(exporter export.SpanExporter) *SimpleSpanProcessor { ssp := &SimpleSpanProcessor{ e: exporter, @@ -39,13 +39,14 @@ func NewSimpleSpanProcessor(exporter export.SpanExporter) *SimpleSpanProcessor { } // OnStart method does nothing. -func (ssp *SimpleSpanProcessor) OnStart(parent context.Context, sd *export.SpanData) { +func (ssp *SimpleSpanProcessor) OnStart(parent context.Context, s ReadWriteSpan) { } -// OnEnd method exports SpanData using associated export. -func (ssp *SimpleSpanProcessor) OnEnd(sd *export.SpanData) { - if ssp.e != nil && sd.SpanContext.IsSampled() { - if err := ssp.e.ExportSpans(context.Background(), []*export.SpanData{sd}); err != nil { +// OnEnd method exports a ReadOnlySpan using the associated exporter. +func (ssp *SimpleSpanProcessor) OnEnd(s ReadOnlySpan) { + if ssp.e != nil && s.SpanContext().IsSampled() { + ss := s.Snapshot() + if err := ssp.e.ExportSpans(context.Background(), []*export.SpanSnapshot{ss}); err != nil { otel.Handle(err) } } diff --git a/sdk/trace/simple_span_processor_test.go b/sdk/trace/simple_span_processor_test.go index 8abbad213ed..181dd31790e 100644 --- a/sdk/trace/simple_span_processor_test.go +++ b/sdk/trace/simple_span_processor_test.go @@ -25,11 +25,11 @@ import ( ) type testExporter struct { - spans []*export.SpanData + spans []*export.SpanSnapshot } -func (t *testExporter) ExportSpans(ctx context.Context, spans []*export.SpanData) error { - t.spans = append(t.spans, spans...) +func (t *testExporter) ExportSpans(ctx context.Context, ss []*export.SpanSnapshot) error { + t.spans = append(t.spans, ss...) return nil } diff --git a/sdk/trace/span.go b/sdk/trace/span.go index fb3c6382c94..bf1f77b23e4 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -16,19 +16,19 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace" import ( "context" - "errors" "fmt" "reflect" "sync" "time" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/label" "go.opentelemetry.io/otel/trace" export "go.opentelemetry.io/otel/sdk/export/trace" + "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/internal" + "go.opentelemetry.io/otel/sdk/resource" ) const ( @@ -37,6 +37,40 @@ const ( errorEventName = "error" ) +// 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 + Parent() trace.SpanContext + SpanKind() trace.SpanKind + StartTime() time.Time + EndTime() time.Time + Attributes() []label.KeyValue + Links() []trace.Link + Events() []export.Event + StatusCode() codes.Code + StatusMessage() string + Tracer() trace.Tracer + IsRecording() bool + InstrumentationLibrary() instrumentation.Library + Resource() *resource.Resource + Snapshot() *export.SpanSnapshot +} + +// ReadWriteSpan exposes the same methods as trace.Span and in addition allows +// reading information from the underlying data structure. +// This interface exposes the union of the methods of trace.Span (which is a +// "write-only" span) and ReadOnlySpan. New methods for writing or reading span +// information should be added under trace.Span or ReadOnlySpan, respectively. +type ReadWriteSpan interface { + trace.Span + ReadOnlySpan +} + var emptySpanContext = trace.SpanContext{} // span is an implementation of the OpenTelemetry Span API representing the @@ -45,16 +79,47 @@ type span struct { // mu protects the contents of this span. mu sync.Mutex - // data contains information recorded about the span. - // - // It will be non-nil if we are exporting the span or recording events for it. - // Otherwise, data is nil, and the span is simply a carrier for the - // SpanContext, so that the trace ID is propagated. - data *export.SpanData + // parent holds the parent span of this span as a trace.SpanContext. + parent trace.SpanContext + + // spanKind represents the kind of this span as a trace.SpanKind. + spanKind trace.SpanKind + + // name is the name of this span. + name string + + // startTime is the time at which this span was started. + startTime time.Time + + // endTime is the time at which this span was ended. It contains the zero + // 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 + + // hasRemoteParent is true when this span has a remote parent span. + hasRemoteParent bool + + // childSpanCount holds the number of child spans 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 + + // spanContext holds the SpanContext of this span. spanContext trace.SpanContext - // attributes are capped at configured limit. When the capacity is reached an oldest entry - // is removed to create room for a new entry. + // attributes are capped at configured limit. When the capacity is reached + // an oldest entry is removed to create room for a new entry. attributes *attributesMap // messageEvents are stored in FIFO queue capped by configured limit. @@ -63,9 +128,6 @@ type span struct { // links are stored in FIFO queue capped by configured limit. links *evictedQueue - // endOnce ensures End is only called once. - endOnce sync.Once - // executionTracerTaskEnd ends the execution tracer span. executionTracerTaskEnd func() @@ -86,7 +148,9 @@ func (s *span) IsRecording() bool { if s == nil { return false } - return s.data != nil + s.mu.Lock() + defer s.mu.Unlock() + return s.endTime.IsZero() } func (s *span) SetStatus(code codes.Code, msg string) { @@ -97,8 +161,8 @@ func (s *span) SetStatus(code codes.Code, msg string) { return } s.mu.Lock() - s.data.StatusCode = code - s.data.StatusMessage = msg + s.statusCode = code + s.statusMessage = msg s.mu.Unlock() } @@ -121,6 +185,10 @@ func (s *span) End(options ...trace.SpanOption) { return } + // Store the end time as soon as possible to avoid artificially increasing + // the span's duration in case some operation below takes a while. + et := internal.MonotonicEndTime(s.startTime) + if recovered := recover(); recovered != nil { // Record but don't stop the panic. defer panic(recovered) @@ -136,25 +204,28 @@ func (s *span) End(options ...trace.SpanOption) { if s.executionTracerTaskEnd != nil { s.executionTracerTaskEnd() } + if !s.IsRecording() { return } + config := trace.NewSpanConfig(options...) - s.endOnce.Do(func() { - sps, ok := s.tracer.provider.spanProcessors.Load().(spanProcessorStates) - mustExportOrProcess := ok && len(sps) > 0 - if mustExportOrProcess { - sd := s.makeSpanData() - if config.Timestamp.IsZero() { - sd.EndTime = internal.MonotonicEndTime(sd.StartTime) - } else { - sd.EndTime = config.Timestamp - } - for _, sp := range sps { - sp.sp.OnEnd(sd) - } + + s.mu.Lock() + if config.Timestamp.IsZero() { + s.endTime = et + } else { + s.endTime = config.Timestamp + } + s.mu.Unlock() + + sps, ok := s.tracer.provider.spanProcessors.Load().(spanProcessorStates) + mustExportOrProcess := ok && len(sps) > 0 + if mustExportOrProcess { + for _, sp := range sps { + sp.sp.OnEnd(s) } - }) + } } func (s *span) RecordError(err error, opts ...trace.EventOption) { @@ -202,36 +273,30 @@ func (s *span) addEvent(name string, o ...trace.EventOption) { }) } -var errUninitializedSpan = errors.New("failed to set name on uninitialized span") - func (s *span) SetName(name string) { s.mu.Lock() defer s.mu.Unlock() - if s.data == nil { - otel.Handle(errUninitializedSpan) - return - } - s.data.Name = name + s.name = name // SAMPLING - noParent := !s.data.ParentSpanID.IsValid() + noParent := !s.parent.SpanID.IsValid() var ctx trace.SpanContext if noParent { ctx = trace.SpanContext{} } else { // FIXME: Where do we get the parent context from? - ctx = s.data.SpanContext + ctx = s.spanContext } data := samplingData{ noParent: noParent, - remoteParent: s.data.HasRemoteParent, + remoteParent: s.hasRemoteParent, parent: ctx, name: name, cfg: s.tracer.provider.config.Load().(*Config), span: s, - attributes: s.data.Attributes, - links: s.data.Links, - kind: s.data.SpanKind, + attributes: s.attributes.toKeyValue(), + links: s.interfaceArrayToLinksArray(), + kind: s.spanKind, } sampled := makeSamplingDecision(data) @@ -242,6 +307,87 @@ func (s *span) SetName(name string) { } } +func (s *span) Name() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.name +} + +func (s *span) Parent() trace.SpanContext { + s.mu.Lock() + defer s.mu.Unlock() + return s.parent +} + +func (s *span) SpanKind() trace.SpanKind { + s.mu.Lock() + defer s.mu.Unlock() + return s.spanKind +} + +func (s *span) StartTime() time.Time { + s.mu.Lock() + defer s.mu.Unlock() + return s.startTime +} + +func (s *span) EndTime() time.Time { + s.mu.Lock() + defer s.mu.Unlock() + return s.endTime +} + +func (s *span) Attributes() []label.KeyValue { + s.mu.Lock() + defer s.mu.Unlock() + if s.attributes.evictList.Len() == 0 { + return []label.KeyValue{} + } + return s.attributes.toKeyValue() +} + +func (s *span) Links() []trace.Link { + s.mu.Lock() + defer s.mu.Unlock() + if len(s.links.queue) == 0 { + return []trace.Link{} + } + return s.interfaceArrayToLinksArray() +} + +func (s *span) Events() []export.Event { + s.mu.Lock() + defer s.mu.Unlock() + if len(s.messageEvents.queue) == 0 { + return []export.Event{} + } + return s.interfaceArrayToMessageEventArray() +} + +func (s *span) StatusCode() codes.Code { + s.mu.Lock() + defer s.mu.Unlock() + return s.statusCode +} + +func (s *span) StatusMessage() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.statusMessage +} + +func (s *span) InstrumentationLibrary() instrumentation.Library { + s.mu.Lock() + defer s.mu.Unlock() + return s.instrumentationLibrary +} + +func (s *span) Resource() *resource.Resource { + s.mu.Lock() + defer s.mu.Unlock() + return s.resource +} + func (s *span) addLink(link trace.Link) { if !s.IsRecording() { return @@ -251,16 +397,30 @@ func (s *span) addLink(link trace.Link) { s.links.add(link) } -// makeSpanData produces a SpanData representing the current state of the span. -// It requires that s.data is non-nil. -func (s *span) makeSpanData() *export.SpanData { - var sd export.SpanData +// 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() *export.SpanSnapshot { + var sd export.SpanSnapshot s.mu.Lock() defer s.mu.Unlock() - sd = *s.data - - s.attributes.toSpanData(&sd) + sd.ChildSpanCount = s.childSpanCount + sd.EndTime = s.endTime + sd.HasRemoteParent = s.hasRemoteParent + sd.InstrumentationLibrary = s.instrumentationLibrary + sd.Name = s.name + sd.ParentSpanID = s.parent.SpanID + sd.Resource = s.resource + sd.SpanContext = s.spanContext + sd.SpanKind = s.spanKind + sd.StartTime = s.startTime + sd.StatusCode = s.statusCode + sd.StatusMessage = s.statusMessage + + if s.attributes.evictList.Len() > 0 { + 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 @@ -303,12 +463,11 @@ func (s *span) addChild() { return } s.mu.Lock() - s.data.ChildSpanCount++ + s.childSpanCount++ s.mu.Unlock() } func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trace.SpanContext, remoteParent bool, o *trace.SpanConfig) *span { - var noParent bool span := &span{} span.spanContext = parent @@ -317,13 +476,17 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac if parent == emptySpanContext { // Generate both TraceID and SpanID span.spanContext.TraceID, span.spanContext.SpanID = cfg.IDGenerator.NewIDs(ctx) - noParent = true } else { // TraceID already exists, just generate a SpanID span.spanContext.SpanID = cfg.IDGenerator.NewSpanID(ctx, parent.TraceID) } + + span.attributes = newAttributesMap(cfg.MaxAttributesPerSpan) + span.messageEvents = newEvictedQueue(cfg.MaxEventsPerSpan) + span.links = newEvictedQueue(cfg.MaxLinksPerSpan) + data := samplingData{ - noParent: noParent, + noParent: parent == emptySpanContext, remoteParent: remoteParent, parent: parent, name: name, @@ -343,24 +506,17 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac if startTime.IsZero() { startTime = time.Now() } - span.data = &export.SpanData{ - SpanContext: span.spanContext, - StartTime: startTime, - SpanKind: trace.ValidateSpanKind(o.SpanKind), - Name: name, - HasRemoteParent: remoteParent, - Resource: cfg.Resource, - InstrumentationLibrary: tr.instrumentationLibrary, - } - span.attributes = newAttributesMap(cfg.MaxAttributesPerSpan) - span.messageEvents = newEvictedQueue(cfg.MaxEventsPerSpan) - span.links = newEvictedQueue(cfg.MaxLinksPerSpan) + span.startTime = startTime + + span.spanKind = trace.ValidateSpanKind(o.SpanKind) + span.name = name + span.hasRemoteParent = remoteParent + span.resource = cfg.Resource + span.instrumentationLibrary = tr.instrumentationLibrary span.SetAttributes(sampled.Attributes...) - if !noParent { - span.data.ParentSpanID = parent.SpanID - } + span.parent = parent return span } diff --git a/sdk/trace/span_processor.go b/sdk/trace/span_processor.go index 79f0c61919b..d32fed657f7 100644 --- a/sdk/trace/span_processor.go +++ b/sdk/trace/span_processor.go @@ -17,8 +17,6 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace" import ( "context" "sync" - - export "go.opentelemetry.io/otel/sdk/export/trace" ) // SpanProcessor is interface to add hooks to start and end method invocations. @@ -26,11 +24,11 @@ type SpanProcessor interface { // OnStart method is invoked when span is started. It is a synchronous call // and hence should not block. - OnStart(parent context.Context, sd *export.SpanData) + OnStart(parent context.Context, s ReadWriteSpan) // OnEnd method is invoked when span is finished. It is a synchronous call // and hence should not block. - OnEnd(sd *export.SpanData) + OnEnd(s ReadOnlySpan) // Shutdown is invoked when SDK shuts down. Use this call to cleanup any processor // data. No calls to OnStart and OnEnd method is invoked after Shutdown call is diff --git a/sdk/trace/span_processor_example_test.go b/sdk/trace/span_processor_example_test.go index 2a5a8b7fcbd..c30b0c37d56 100644 --- a/sdk/trace/span_processor_example_test.go +++ b/sdk/trace/span_processor_example_test.go @@ -18,7 +18,6 @@ import ( "context" "time" - export "go.opentelemetry.io/otel/sdk/export/trace" "go.opentelemetry.io/otel/sdk/export/trace/tracetest" ) @@ -34,21 +33,21 @@ type DurationFilter struct { Max time.Duration } -func (f DurationFilter) OnStart(parent context.Context, sd *export.SpanData) { - f.Next.OnStart(parent, sd) +func (f DurationFilter) OnStart(parent context.Context, s ReadWriteSpan) { + f.Next.OnStart(parent, s) } func (f DurationFilter) Shutdown(ctx context.Context) error { return f.Next.Shutdown(ctx) } func (f DurationFilter) ForceFlush() { f.Next.ForceFlush() } -func (f DurationFilter) OnEnd(sd *export.SpanData) { - if f.Min > 0 && sd.EndTime.Sub(sd.StartTime) < f.Min { +func (f DurationFilter) OnEnd(s ReadOnlySpan) { + if f.Min > 0 && s.EndTime().Sub(s.StartTime()) < f.Min { // Drop short lived spans. return } - if f.Max > 0 && sd.EndTime.Sub(sd.StartTime) > f.Max { + if f.Max > 0 && s.EndTime().Sub(s.StartTime()) > f.Max { // Drop long lived spans. return } - f.Next.OnEnd(sd) + f.Next.OnEnd(s) } // InstrumentationBlacklist is a SpanProcessor that drops all spans from @@ -62,17 +61,17 @@ type InstrumentationBlacklist struct { Blacklist map[string]bool } -func (f InstrumentationBlacklist) OnStart(parent context.Context, sd *export.SpanData) { - f.Next.OnStart(parent, sd) +func (f InstrumentationBlacklist) OnStart(parent context.Context, s ReadWriteSpan) { + f.Next.OnStart(parent, s) } func (f InstrumentationBlacklist) Shutdown(ctx context.Context) error { return f.Next.Shutdown(ctx) } func (f InstrumentationBlacklist) ForceFlush() { f.Next.ForceFlush() } -func (f InstrumentationBlacklist) OnEnd(sd *export.SpanData) { - if f.Blacklist != nil && f.Blacklist[sd.InstrumentationLibrary.Name] { +func (f InstrumentationBlacklist) OnEnd(s ReadOnlySpan) { + if f.Blacklist != nil && f.Blacklist[s.InstrumentationLibrary().Name] { // Drop spans from this instrumentation return } - f.Next.OnEnd(sd) + f.Next.OnEnd(s) } func ExampleSpanProcessor() { diff --git a/sdk/trace/span_processor_test.go b/sdk/trace/span_processor_test.go index a607a15d1c3..f2376d37df4 100644 --- a/sdk/trace/span_processor_test.go +++ b/sdk/trace/span_processor_test.go @@ -19,18 +19,18 @@ import ( "testing" "go.opentelemetry.io/otel/label" - export "go.opentelemetry.io/otel/sdk/export/trace" + sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/trace" ) type testSpanProcessor struct { name string - spansStarted []*export.SpanData - spansEnded []*export.SpanData + spansStarted []sdktrace.ReadWriteSpan + spansEnded []sdktrace.ReadOnlySpan shutdownCount int } -func (t *testSpanProcessor) OnStart(parent context.Context, s *export.SpanData) { +func (t *testSpanProcessor) OnStart(parent context.Context, s sdktrace.ReadWriteSpan) { psc := trace.RemoteSpanContextFromContext(parent) kv := []label.KeyValue{ { @@ -50,16 +50,11 @@ func (t *testSpanProcessor) OnStart(parent context.Context, s *export.SpanData) Value: label.StringValue(psc.SpanID.String()), }, } - s.Attributes = append(s.Attributes, kv...) + s.AddEvent("OnStart", trace.WithAttributes(kv...)) t.spansStarted = append(t.spansStarted, s) } -func (t *testSpanProcessor) OnEnd(s *export.SpanData) { - kv := label.KeyValue{ - Key: "OnEnd", - Value: label.StringValue(t.name), - } - s.Attributes = append(s.Attributes, kv) +func (t *testSpanProcessor) OnEnd(s sdktrace.ReadOnlySpan) { t.spansEnded = append(t.spansEnded, s) } @@ -107,28 +102,30 @@ func TestRegisterSpanProcessor(t *testing.T) { c := 0 tidOK := false sidOK := false - for _, kv := range sp.spansStarted[0].Attributes { - switch kv.Key { - case "SpanProcessorName": - gotValue := kv.Value.AsString() - if gotValue != spNames[c] { - t.Errorf("%s: attributes: got %s, want %s\n", name, gotValue, spNames[c]) - } - c++ - case "ParentTraceID": - gotValue := kv.Value.AsString() - if gotValue != parent.TraceID.String() { - t.Errorf("%s: attributes: got %s, want %s\n", name, gotValue, parent.TraceID) - } - tidOK = true - case "ParentSpanID": - gotValue := kv.Value.AsString() - if gotValue != parent.SpanID.String() { - t.Errorf("%s: attributes: got %s, want %s\n", name, gotValue, parent.SpanID) + for _, e := range sp.spansStarted[0].Events() { + for _, kv := range e.Attributes { + switch kv.Key { + case "SpanProcessorName": + gotValue := kv.Value.AsString() + if gotValue != spNames[c] { + t.Errorf("%s: attributes: got %s, want %s\n", name, gotValue, spNames[c]) + } + c++ + case "ParentTraceID": + gotValue := kv.Value.AsString() + if gotValue != parent.TraceID.String() { + t.Errorf("%s: attributes: got %s, want %s\n", name, gotValue, parent.TraceID) + } + tidOK = true + case "ParentSpanID": + gotValue := kv.Value.AsString() + if gotValue != parent.SpanID.String() { + t.Errorf("%s: attributes: got %s, want %s\n", name, gotValue, parent.SpanID) + } + sidOK = true + default: + continue } - sidOK = true - default: - continue } } if c != len(spNames) { @@ -175,21 +172,6 @@ func TestUnregisterSpanProcessor(t *testing.T) { if gotCount != wantCount { t.Errorf("%s: ended count: got %d, want %d\n", name, gotCount, wantCount) } - - c := 0 - for _, kv := range sp.spansEnded[0].Attributes { - if kv.Key != "OnEnd" { - continue - } - gotValue := kv.Value.AsString() - if gotValue != spNames[c] { - t.Errorf("%s: ordered attributes: got %s, want %s\n", name, gotValue, spNames[c]) - } - c++ - } - if c != len(spNames) { - t.Errorf("%s: expected attributes(OnEnd): got %d, want %d\n", name, c, len(spNames)) - } } } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 8ccd44046f8..462978f8181 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -70,19 +70,19 @@ func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) { type testExporter struct { mu sync.RWMutex idx map[string]int - spans []*export.SpanData + spans []*export.SpanSnapshot } func NewTestExporter() *testExporter { return &testExporter{idx: make(map[string]int)} } -func (te *testExporter) ExportSpans(_ context.Context, spans []*export.SpanData) error { +func (te *testExporter) ExportSpans(_ context.Context, ss []*export.SpanSnapshot) error { te.mu.Lock() defer te.mu.Unlock() i := len(te.spans) - for _, s := range spans { + for _, s := range ss { te.idx[s.Name] = i te.spans = append(te.spans, s) i++ @@ -90,16 +90,16 @@ func (te *testExporter) ExportSpans(_ context.Context, spans []*export.SpanData) return nil } -func (te *testExporter) Spans() []*export.SpanData { +func (te *testExporter) Spans() []*export.SpanSnapshot { te.mu.RLock() defer te.mu.RUnlock() - cp := make([]*export.SpanData, len(te.spans)) + cp := make([]*export.SpanSnapshot, len(te.spans)) copy(cp, te.spans) return cp } -func (te *testExporter) GetSpan(name string) (*export.SpanData, bool) { +func (te *testExporter) GetSpan(name string) (*export.SpanSnapshot, bool) { te.mu.RLock() defer te.mu.RUnlock() i, ok := te.idx[name] @@ -351,7 +351,7 @@ func TestSetSpanAttributesOnStart(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -381,7 +381,7 @@ func TestSetSpanAttributes(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -433,7 +433,7 @@ func TestSamplerAttributesLocalChildSpan(t *testing.T) { checkTime(&got[1].StartTime) checkTime(&got[1].EndTime) - want := []*export.SpanData{ + want := []*export.SpanSnapshot{ { SpanContext: trace.SpanContext{ TraceID: tid, @@ -483,7 +483,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -529,7 +529,7 @@ func TestEvents(t *testing.T) { } } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -580,7 +580,7 @@ func TestEventsOverLimit(t *testing.T) { } } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -623,7 +623,7 @@ func TestLinks(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -666,7 +666,7 @@ func TestLinksOverLimit(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -720,7 +720,7 @@ func TestSetSpanStatus(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -812,15 +812,16 @@ 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.SpanData. +// returns the exported export.SpanSnapshot. // 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.SpanData to make the comparison easier. -func endSpan(te *testExporter, span trace.Span) (*export.SpanData, error) { +// It also clears spanID in the export.SpanSnapshot to make the comparison +// easier. +func endSpan(te *testExporter, span trace.Span) (*export.SpanSnapshot, error) { if !span.IsRecording() { return nil, fmt.Errorf("IsRecording: got false, want true") } @@ -858,12 +859,22 @@ func TestEndSpanTwice(t *testing.T) { te := NewTestExporter() tp := NewTracerProvider(WithSyncer(te)) - span := startSpan(tp, "EndSpanTwice") - span.End() - span.End() + st := time.Now() + et1 := st.Add(100 * time.Millisecond) + et2 := st.Add(200 * time.Millisecond) + + span := startSpan(tp, "EndSpanTwice", trace.WithTimestamp(st)) + span.End(trace.WithTimestamp(et1)) + span.End(trace.WithTimestamp(et2)) + if te.Len() != 1 { t.Fatalf("expected only a single span, got %#v", te.Spans()) } + + ro := span.(ReadOnlySpan) + if ro.EndTime() != et1 { + t.Fatalf("2nd call to End() should not modify end time") + } } func TestStartSpanAfterEnd(t *testing.T) { @@ -1070,7 +1081,7 @@ func TestRecordError(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -1110,7 +1121,7 @@ func TestRecordErrorNil(t *testing.T) { t.Fatal(err) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -1178,7 +1189,7 @@ func TestWithResource(t *testing.T) { t.Error(err.Error()) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -1213,7 +1224,7 @@ func TestWithInstrumentationVersion(t *testing.T) { t.Error(err.Error()) } - want := &export.SpanData{ + want := &export.SpanSnapshot{ SpanContext: trace.SpanContext{ TraceID: tid, TraceFlags: 0x1, @@ -1255,3 +1266,120 @@ func TestSpanCapturesPanic(t *testing.T) { errorMessageKey.String("error message"), }) } + +func TestReadOnlySpan(t *testing.T) { + kv := label.String("foo", "bar") + + tp := NewTracerProvider(WithResource(resource.NewWithAttributes(kv))) + cfg := tp.config.Load().(*Config) + tr := tp.Tracer("ReadOnlySpan", trace.WithInstrumentationVersion("3")) + + // Initialize parent context. + tID, sID := cfg.IDGenerator.NewIDs(context.Background()) + parent := trace.SpanContext{ + TraceID: tID, + SpanID: sID, + TraceFlags: 0x1, + } + ctx := trace.ContextWithRemoteSpanContext(context.Background(), parent) + + // Initialize linked context. + tID, sID = cfg.IDGenerator.NewIDs(context.Background()) + linked := trace.SpanContext{ + TraceID: tID, + SpanID: sID, + TraceFlags: 0x1, + } + + st := time.Now() + ctx, span := 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") + + // Verify span implements ReadOnlySpan. + ro, ok := span.(ReadOnlySpan) + require.True(t, ok) + + assert.Equal(t, "foo", ro.Name()) + assert.Equal(t, trace.SpanContextFromContext(ctx), ro.SpanContext()) + assert.Equal(t, parent, ro.Parent()) + assert.Equal(t, trace.SpanKindInternal, ro.SpanKind()) + assert.Equal(t, st, ro.StartTime()) + assert.True(t, ro.EndTime().IsZero()) + assert.Equal(t, kv.Key, ro.Attributes()[0].Key) + assert.Equal(t, kv.Value, ro.Attributes()[0].Value) + 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, "foo", ro.StatusMessage()) + assert.Equal(t, "ReadOnlySpan", ro.InstrumentationLibrary().Name) + assert.Equal(t, "3", ro.InstrumentationLibrary().Version) + assert.Equal(t, kv.Key, ro.Resource().Attributes()[0].Key) + assert.Equal(t, kv.Value, ro.Resource().Attributes()[0].Value) + + // Verify changes to the original span are reflected in the ReadOnlySpan. + span.SetName("bar") + assert.Equal(t, "bar", ro.Name()) + + // 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.MessageEvents { + if e.Name == "baz" { + t.Errorf("Didn't expect to find 'baz' event") + } + } + var exists bool + for _, e := range d2.MessageEvents { + if e.Name == "baz" { + exists = true + } + } + if !exists { + t.Errorf("Expected to find 'baz' event") + } + + et := st.Add(time.Millisecond) + span.End(trace.WithTimestamp(et)) + assert.Equal(t, et, ro.EndTime()) +} + +func TestReadWriteSpan(t *testing.T) { + tp := NewTracerProvider() + cfg := tp.config.Load().(*Config) + tr := tp.Tracer("ReadWriteSpan") + + // Initialize parent context. + tID, sID := cfg.IDGenerator.NewIDs(context.Background()) + parent := trace.SpanContext{ + TraceID: tID, + SpanID: sID, + TraceFlags: 0x1, + } + ctx := trace.ContextWithRemoteSpanContext(context.Background(), parent) + + _, span := tr.Start(ctx, "foo") + defer span.End() + + // Verify span implements ReadOnlySpan. + rw, ok := span.(ReadWriteSpan) + require.True(t, ok) + + // Verify the span can be read from. + assert.False(t, rw.StartTime().IsZero()) + + // Verify the span can be written to. + rw.SetName("bar") + assert.Equal(t, "bar", rw.Name()) + + // NOTE: This function tests ReadWriteSpan which is an interface which + // embeds trace.Span and ReadOnlySpan. Since both of these interfaces have + // their own tests, there is no point in testing all the possible methods + // available via ReadWriteSpan as doing so would mean creating a lot of + // duplication. +} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index ea90bb65f32..c85f866fd68 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -61,7 +61,7 @@ func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanO if span.IsRecording() { sps, _ := tr.provider.spanProcessors.Load().(spanProcessorStates) for _, sp := range sps { - sp.sp.OnStart(ctx, span.data) + sp.sp.OnStart(ctx, span) } }