Merge branch 'johananl/span-interfaces' of github.com:johananl/opente…

…lemetry-go into johananl-johananl/span-interfaces

CHANGELOG.md

@@ -10,19 +10,27 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Added
 
+- Add the `ReadOnlySpan` and `ReadWriteSpan` interfaces to provide better control for accessing span data. (#1360)
 - `trace.WithIDGenerator()` `TracerProviderOption`. (#1363)
 
 ### Changed
 
+- Zipkin exporter relies on the status code for success rather than body read but still read the response body. (#1328)
 - Move the OpenCensus example into `example` directory. (#1359)
 - Moved the SDK's `internal.IDGenerator` interface in to the `sdk/trace` package to enable support for externally-defined ID generators. (#1363)
 - `NewExporter` and `Start` functions in `go.opentelemetry.io/otel/exporters/otlp` now receive `context.Context` as a first parameter. (#1357)
-- Zipkin exporter relies on the status code for success rather than body read but still read the response body. (#1328)
+- 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)
 
 ### Fixed
 
 - Metric SDK `SumObserver` and `UpDownSumObserver` instruments correctness fixes. (#1381)
 
+### Removed
+
+- Remove `errUninitializedSpan` as its only usage is now obsolete. (#1360)
+
 ## [0.14.0] - 2020-11-19
 
 ### Added

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
 	}

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{{}}) })
 }

exporters/otlp/otlp.go

@@ -197,20 +197,20 @@ 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()
 
 	if !e.cc.connected() {
 		return nil
 	}
 
-	protoSpans := transform.SpanData(sdl)
+	protoSpans := transform.SpanData(ss)
 	if len(protoSpans) == 0 {
 		return nil
 	}

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)

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}),

exporters/stdout/trace.go

@@ -31,19 +31,19 @@ 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()
 	if stopped {
 		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
 	}

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)
 	}
 

exporters/trace/jaeger/jaeger.go

@@ -210,18 +210,18 @@ 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()
 	if stopped {
 		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,34 +273,34 @@ 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))
 		}
 	}
 
 	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,

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
 			})

exporters/trace/zipkin/model.go

@@ -31,15 +31,15 @@ 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))
 	}
 	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()