From c3c4273eccd7bfab8fcaa44ccd6be6f9dd532efd Mon Sep 17 00:00:00 2001 From: Johannes Liebermann Date: Fri, 11 Dec 2020 06:15:44 +0100 Subject: [PATCH] Add RO/RW span interfaces (#1360) * Store span data directly in the span - Nesting only some of a span's data in a `data` field (with the rest of the data living direclty in the `span` struct) is confusing. - export.SpanData is meant to be an immutable *snapshot* of a span, not the "authoritative" state of the span. - Refactor attributesMap.toSpanData into toKeyValue and make it return a []label.KeyValue which is clearer than modifying a struct passed to the function. - Read droppedCount from the attributesMap as a separate operation instead of setting it from within attributesMap.toSpanData. - Set a span's end time in the span itself rather than in the SpanData to allow reading the span's end time after a span has ended. - Set a span's end time as soon as possible within span.End so that we don't influence the span's end time with operations such as fetching span processors and generating span data. - Remove error handling for uninitialized spans. This check seems to be necessary only because we used to have an *export.SpanData field which could be nil. Now that we no longer have this field I think we can safely remove the check. The error isn't used anywhere else so remove it, too. * Store parent as trace.SpanContext The spec requires that the parent field of a Span be a Span, a SpanContext or null. Rather than extracting the parent's span ID from the trace.SpanContext which we get from the tracer, store the trace.SpanContext as is and explicitly extract the parent's span ID where necessary. * Add ReadOnlySpan interface Use this interface instead of export.SpanData in places where reading information from a span is necessary. Use export.SpanData only when exporting spans. * Add ReadWriteSpan interface Use this interface instead of export.SpanData in places where it is necessary to read information from a span and write to it at the same time. * Rename export.SpanData to SpanSnapshot SpanSnapshot represents the nature of this type as well as its intended use more accurately. Clarify the purpose of SpanSnapshot in the docs and emphasize what should and should not be done with it. * Rephrase attributesMap doc comment "refreshes" is wrong for plural ("updates"). * Refactor span.End() - Improve accuracy of span duration. Record span end time ASAP. We want to measure a user operation as accurately as possible, which means we want to mark the end time of a span as soon as possible after span.End() is called. Any operations we do inside span.End() before storing the end time affect the total duration of the span, and although these operations are rather fast at the moment they still seem to affect the duration of the span by "artificially" adding time between the start and end timestamps. This is relevant only in cases where the end time isn't explicitly specified. - Remove redundant idempotence check. Now that IsRecording() is based on the value of span.endTime, IsRecording() will always return false after span.End() had been called because span.endTime won't be zero. This means we no longer need span.endOnce. - Improve TestEndSpanTwice so that it also ensures subsequent calls to span.End() don't modify the span's end time. * Update changelog Co-authored-by: Tyler Yahn Co-authored-by: Tyler Yahn --- CHANGELOG.md | 14 + exporters/otlp/internal/transform/span.go | 7 +- .../otlp/internal/transform/span_test.go | 8 +- exporters/otlp/otlp.go | 10 +- exporters/otlp/otlp_integration_test.go | 8 +- exporters/otlp/otlp_span_test.go | 8 +- exporters/stdout/trace.go | 8 +- exporters/stdout/trace_test.go | 4 +- exporters/trace/jaeger/jaeger.go | 48 +-- exporters/trace/jaeger/jaeger_test.go | 8 +- exporters/trace/zipkin/model.go | 8 +- exporters/trace/zipkin/model_test.go | 18 +- exporters/trace/zipkin/zipkin.go | 10 +- exporters/trace/zipkin/zipkin_test.go | 2 +- sdk/export/trace/trace.go | 16 +- sdk/export/trace/tracetest/test.go | 28 +- sdk/export/trace/tracetest/test_test.go | 8 +- sdk/trace/attributesmap.go | 13 +- sdk/trace/attributesmap_test.go | 25 +- sdk/trace/batch_span_processor.go | 24 +- sdk/trace/batch_span_processor_test.go | 19 +- sdk/trace/provider_test.go | 8 +- sdk/trace/simple_span_processor.go | 15 +- sdk/trace/simple_span_processor_test.go | 6 +- sdk/trace/span.go | 294 ++++++++++++++---- sdk/trace/span_processor.go | 6 +- sdk/trace/span_processor_example_test.go | 23 +- sdk/trace/span_processor_test.go | 76 ++--- sdk/trace/trace_test.go | 178 +++++++++-- sdk/trace/tracer.go | 2 +- 30 files changed, 598 insertions(+), 304 deletions(-) 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) } }