Redefine ExportSpans of SpanExporter with ReadOnlySpan (#1873)

* Remove TODO from ReadOnlySpan interface

* Remove the Tracer method from the ReadOnlySpan

This is not required by the specification nor the use of this interface.

* Remove IsRecording from the ReadOnlySpan interface

A read-only span value does not need to know if updates to it will be
recorded. It by definition cannot be updated so no point in
communicating if an update would be recorded.

* Document the ReadOnlySpan interface

* Rename messageEvent* to just event*

* Move the SpanSnapshot into its own file

* Update ReadOnlySpan interface with meta info methods

Add the DroppedAttributes, DroppedLinks, DroppedEvents, and
ChildSpanCount methods to the interface to return additional information
about the span not specified by the specification, but that we are
already providing.

* Add SpanStub to the sdk/trace/tracetest pkg

* Redefine ExportSpans of SpanExporter with ReadOnlySpan

* Rename SpanSnapshot to snapshot and purge docs

* Remove Snapshot method from snapshot type

This method is a hold-over from previous version of the ReadOnlySpan
interface is not needed.

* Update CHANGELOG with changes

CHANGELOG.md

@@ -22,6 +22,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
       | 14 | Unavailable |
       | 15 | Data Loss |
 - The `Status` type was added to the `go.opentelemetry.io/otel/sdk/trace` package to represent the status of a span. (#1874)
+- The `SpanStub` type and its associated functions were added to the `go.opentelemetry.io/otel/sdk/trace/tracetest` package.
+  This type can be used as a testing replacement for the `SpanSnapshot` that was removed from the `go.opentelemetry.io/otel/sdk/trace` package. (#1873)
 
 ### Changed
 
@@ -34,6 +36,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Renamed `CloudZoneKey` to `CloudAvailabilityZoneKey` in Resource semantic conventions according to spec. (#1871)
 - The `StatusCode` and `StatusMessage` methods of the `ReadOnlySpan` interface and the `Span` produced by the `go.opentelemetry.io/otel/sdk/trace` package have been replaced with a single `Status` method.
   This method returns the status of a span using the new `Status` type. (#1874)
+- The `ExportSpans` method of the`SpanExporter` interface type was updated to accept `ReadOnlySpan`s instead of the removed `SpanSnapshot`.
+  This brings the export interface into compliance with the specification in that it now accepts an explicitly immutable type instead of just an implied one. (#1873)
 - Unembed `SpanContext` in `Link`. (#1877)
 
 ### Deprecated
@@ -42,6 +46,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Remove `resource.WithoutBuiltin()`. Use `resource.New()`. (#1810)
 - Unexported types `resource.FromEnv`, `resource.Host`, and `resource.TelemetrySDK`, Use the corresponding `With*()` to use individually. (#1810)
+- Removed the `Tracer` and `IsRecording` method from the `ReadOnlySpan` in the `go.opentelemetry.io/otel/sdk/trace`.
+  The `Tracer` method is not a required to be included in this interface and given the mutable nature of the tracer that is associated with a span, this method is not appropriate.
+  The `IsRecording` method returns if the span is recording or not.
+  A read-only span value does not need to know if updates to it will be recorded or not.
+  By definition, it cannot be updated so there is no point in communicating if an update is recorded. (#1873)
+- Removed the `SpanSnapshot` type from the `go.opentelemetry.io/otel/sdk/trace` package.
+  The use of this type has been replaced with the use of the explicitly immutable `ReadOnlySpan` type.
+  When a concrete representation of a read-only span is needed for testing, the newly added `SpanStub` in the `go.opentelemetry.io/otel/sdk/trace/tracetest` package should be used. (#1873)
 
 ### Fixed
 

exporters/otlp/internal/otlptest/data.go

@@ -28,6 +28,7 @@ import (
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/sum"
 	"go.opentelemetry.io/otel/sdk/resource"
 	tracesdk "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/sdk/trace/tracetest"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -78,39 +79,40 @@ func (OneRecordCheckpointSet) ForEach(kindSelector exportmetric.ExportKindSelect
 	return recordFunc(rec)
 }
 
-// SingleSpanSnapshot returns a one-element slice with a snapshot. It
+// SingleReadOnlySpan returns a one-element slice with a read-only span. It
 // may be useful for testing driver's trace export.
-func SingleSpanSnapshot() []*tracesdk.SpanSnapshot {
-	sd := &tracesdk.SpanSnapshot{
-		SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
-			TraceID:    trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9},
-			SpanID:     trace.SpanID{3, 4, 5, 6, 7, 8, 9, 0},
-			TraceFlags: trace.FlagsSampled,
-		}),
-		Parent: trace.NewSpanContext(trace.SpanContextConfig{
-			TraceID:    trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9},
-			SpanID:     trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8},
-			TraceFlags: trace.FlagsSampled,
-		}),
-		SpanKind:                 trace.SpanKindInternal,
-		Name:                     "foo",
-		StartTime:                time.Date(2020, time.December, 8, 20, 23, 0, 0, time.UTC),
-		EndTime:                  time.Date(2020, time.December, 0, 20, 24, 0, 0, time.UTC),
-		Attributes:               []attribute.KeyValue{},
-		MessageEvents:            []tracesdk.Event{},
-		Links:                    []trace.Link{},
-		Status:                   tracesdk.Status{Code: codes.Ok},
-		DroppedAttributeCount:    0,
-		DroppedMessageEventCount: 0,
-		DroppedLinkCount:         0,
-		ChildSpanCount:           0,
-		Resource:                 resource.NewWithAttributes(attribute.String("a", "b")),
-		InstrumentationLibrary: instrumentation.Library{
-			Name:    "bar",
-			Version: "0.0.0",
+func SingleReadOnlySpan() []tracesdk.ReadOnlySpan {
+	return tracetest.SpanStubs{
+		{
+			SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID:    trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9},
+				SpanID:     trace.SpanID{3, 4, 5, 6, 7, 8, 9, 0},
+				TraceFlags: trace.FlagsSampled,
+			}),
+			Parent: trace.NewSpanContext(trace.SpanContextConfig{
+				TraceID:    trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9},
+				SpanID:     trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8},
+				TraceFlags: trace.FlagsSampled,
+			}),
+			SpanKind:          trace.SpanKindInternal,
+			Name:              "foo",
+			StartTime:         time.Date(2020, time.December, 8, 20, 23, 0, 0, time.UTC),
+			EndTime:           time.Date(2020, time.December, 0, 20, 24, 0, 0, time.UTC),
+			Attributes:        []attribute.KeyValue{},
+			Events:            []tracesdk.Event{},
+			Links:             []trace.Link{},
+			Status:            tracesdk.Status{Code: codes.Ok},
+			DroppedAttributes: 0,
+			DroppedEvents:     0,
+			DroppedLinks:      0,
+			ChildSpanCount:    0,
+			Resource:          resource.NewWithAttributes(attribute.String("a", "b")),
+			InstrumentationLibrary: instrumentation.Library{
+				Name:    "bar",
+				Version: "0.0.0",
+			},
 		},
-	}
-	return []*tracesdk.SpanSnapshot{sd}
+	}.Snapshots()
 }
 
 // EmptyCheckpointSet is a checkpointer that has no records at all.

exporters/otlp/internal/transform/span.go

@@ -25,12 +25,12 @@ import (
 )
 
 const (
-	maxMessageEventsPerSpan = 128
+	maxEventsPerSpan = 128
 )
 
-// SpanData transforms a slice of SpanSnapshot into a slice of OTLP
+// Spans transforms a slice of OpenTelemetry spans into a slice of OTLP
 // ResourceSpans.
-func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans {
+func Spans(sdl []tracesdk.ReadOnlySpan) []*tracepb.ResourceSpans {
 	if len(sdl) == 0 {
 		return nil
 	}
@@ -49,16 +49,16 @@ func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans {
 			continue
 		}
 
-		rKey := sd.Resource.Equivalent()
+		rKey := sd.Resource().Equivalent()
 		iKey := ilsKey{
 			r:  rKey,
-			il: sd.InstrumentationLibrary,
+			il: sd.InstrumentationLibrary(),
 		}
 		ils, iOk := ilsm[iKey]
 		if !iOk {
 			// Either the resource or instrumentation library were unknown.
 			ils = &tracepb.InstrumentationLibrarySpans{
-				InstrumentationLibrary: instrumentationLibrary(sd.InstrumentationLibrary),
+				InstrumentationLibrary: instrumentationLibrary(sd.InstrumentationLibrary()),
 				Spans:                  []*tracepb.Span{},
 			}
 		}
@@ -70,7 +70,7 @@ func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans {
 			resources++
 			// The resource was unknown.
 			rs = &tracepb.ResourceSpans{
-				Resource:                    Resource(sd.Resource),
+				Resource:                    Resource(sd.Resource()),
 				InstrumentationLibrarySpans: []*tracepb.InstrumentationLibrarySpans{ils},
 			}
 			rsm[rKey] = rs
@@ -96,32 +96,32 @@ func SpanData(sdl []*tracesdk.SpanSnapshot) []*tracepb.ResourceSpans {
 }
 
 // span transforms a Span into an OTLP span.
-func span(sd *tracesdk.SpanSnapshot) *tracepb.Span {
+func span(sd tracesdk.ReadOnlySpan) *tracepb.Span {
 	if sd == nil {
 		return nil
 	}
 
-	tid := sd.SpanContext.TraceID()
-	sid := sd.SpanContext.SpanID()
+	tid := sd.SpanContext().TraceID()
+	sid := sd.SpanContext().SpanID()
 
 	s := &tracepb.Span{
 		TraceId:                tid[:],
 		SpanId:                 sid[:],
-		TraceState:             sd.SpanContext.TraceState().String(),
-		Status:                 status(sd.Status.Code, sd.Status.Description),
-		StartTimeUnixNano:      uint64(sd.StartTime.UnixNano()),
-		EndTimeUnixNano:        uint64(sd.EndTime.UnixNano()),
-		Links:                  links(sd.Links),
-		Kind:                   spanKind(sd.SpanKind),
-		Name:                   sd.Name,
-		Attributes:             Attributes(sd.Attributes),
-		Events:                 spanEvents(sd.MessageEvents),
-		DroppedAttributesCount: uint32(sd.DroppedAttributeCount),
-		DroppedEventsCount:     uint32(sd.DroppedMessageEventCount),
-		DroppedLinksCount:      uint32(sd.DroppedLinkCount),
+		TraceState:             sd.SpanContext().TraceState().String(),
+		Status:                 status(sd.Status().Code, sd.Status().Description),
+		StartTimeUnixNano:      uint64(sd.StartTime().UnixNano()),
+		EndTimeUnixNano:        uint64(sd.EndTime().UnixNano()),
+		Links:                  links(sd.Links()),
+		Kind:                   spanKind(sd.SpanKind()),
+		Name:                   sd.Name(),
+		Attributes:             Attributes(sd.Attributes()),
+		Events:                 spanEvents(sd.Events()),
+		DroppedAttributesCount: uint32(sd.DroppedAttributes()),
+		DroppedEventsCount:     uint32(sd.DroppedEvents()),
+		DroppedLinksCount:      uint32(sd.DroppedLinks()),
 	}
 
-	if psid := sd.Parent.SpanID(); psid.IsValid() {
+	if psid := sd.Parent().SpanID(); psid.IsValid() {
 		s.ParentSpanId = psid[:]
 	}
 
@@ -174,18 +174,18 @@ func spanEvents(es []tracesdk.Event) []*tracepb.Span_Event {
 	}
 
 	evCount := len(es)
-	if evCount > maxMessageEventsPerSpan {
-		evCount = maxMessageEventsPerSpan
+	if evCount > maxEventsPerSpan {
+		evCount = maxEventsPerSpan
 	}
 	events := make([]*tracepb.Span_Event, 0, evCount)
-	messageEvents := 0
+	nEvents := 0
 
 	// Transform message events
 	for _, e := range es {
-		if messageEvents >= maxMessageEventsPerSpan {
+		if nEvents >= maxEventsPerSpan {
 			break
 		}
-		messageEvents++
+		nEvents++
 		events = append(events,
 			&tracepb.Span_Event{
 				Name:         e.Name,

exporters/otlp/internal/transform/span_test.go

@@ -32,6 +32,7 @@ import (
 	"go.opentelemetry.io/otel/sdk/instrumentation"
 	"go.opentelemetry.io/otel/sdk/resource"
 	tracesdk "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/sdk/trace/tracetest"
 )
 
 func TestSpanKind(t *testing.T) {
@@ -101,15 +102,15 @@ func TestSpanEvent(t *testing.T) {
 }
 
 func TestExcessiveSpanEvents(t *testing.T) {
-	e := make([]tracesdk.Event, maxMessageEventsPerSpan+1)
-	for i := 0; i < maxMessageEventsPerSpan+1; i++ {
+	e := make([]tracesdk.Event, maxEventsPerSpan+1)
+	for i := 0; i < maxEventsPerSpan+1; i++ {
 		e[i] = tracesdk.Event{Name: strconv.Itoa(i)}
 	}
-	assert.Len(t, e, maxMessageEventsPerSpan+1)
+	assert.Len(t, e, maxEventsPerSpan+1)
 	got := spanEvents(e)
-	assert.Len(t, got, maxMessageEventsPerSpan)
+	assert.Len(t, got, maxEventsPerSpan)
 	// Ensure the drop order.
-	assert.Equal(t, strconv.Itoa(maxMessageEventsPerSpan-1), got[len(got)-1].Name)
+	assert.Equal(t, strconv.Itoa(maxEventsPerSpan-1), got[len(got)-1].Name)
 }
 
 func TestNilLinks(t *testing.T) {
@@ -185,11 +186,11 @@ func TestNilSpan(t *testing.T) {
 }
 
 func TestNilSpanData(t *testing.T) {
-	assert.Nil(t, SpanData(nil))
+	assert.Nil(t, Spans(nil))
 }
 
 func TestEmptySpanData(t *testing.T) {
-	assert.Nil(t, SpanData(nil))
+	assert.Nil(t, Spans(nil))
 }
 
 func TestSpanData(t *testing.T) {
@@ -199,7 +200,7 @@ func TestSpanData(t *testing.T) {
 	startTime := time.Unix(1585674086, 1234)
 	endTime := startTime.Add(10 * time.Second)
 	traceState, _ := trace.TraceStateFromKeyValues(attribute.String("key1", "val1"), attribute.String("key2", "val2"))
-	spanData := &tracesdk.SpanSnapshot{
+	spanData := tracetest.SpanStub{
 		SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
 			TraceID:    trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
 			SpanID:     trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8},
@@ -215,15 +216,15 @@ func TestSpanData(t *testing.T) {
 		Name:      "span data to span data",
 		StartTime: startTime,
 		EndTime:   endTime,
-		MessageEvents: []tracesdk.Event{
+		Events: []tracesdk.Event{
 			{Time: startTime,
 				Attributes: []attribute.KeyValue{
 					attribute.Int64("CompressedByteSize", 512),
 				},
 			},
 			{Time: endTime,
 				Attributes: []attribute.KeyValue{
-					attribute.String("MessageEventType", "Recv"),
+					attribute.String("EventType", "Recv"),
 				},
 			},
 		},
@@ -256,10 +257,10 @@ func TestSpanData(t *testing.T) {
 		Attributes: []attribute.KeyValue{
 			attribute.Int64("timeout_ns", 12e9),
 		},
-		DroppedAttributeCount:    1,
-		DroppedMessageEventCount: 2,
-		DroppedLinkCount:         3,
-		Resource:                 resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)),
+		DroppedAttributes: 1,
+		DroppedEvents:     2,
+		DroppedLinks:      3,
+		Resource:          resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)),
 		InstrumentationLibrary: instrumentation.Library{
 			Name:    "go.opentelemetry.io/test/otel",
 			Version: "v0.0.1",
@@ -279,15 +280,15 @@ func TestSpanData(t *testing.T) {
 		StartTimeUnixNano:      uint64(startTime.UnixNano()),
 		EndTimeUnixNano:        uint64(endTime.UnixNano()),
 		Status:                 status(spanData.Status.Code, spanData.Status.Description),
-		Events:                 spanEvents(spanData.MessageEvents),
+		Events:                 spanEvents(spanData.Events),
 		Links:                  links(spanData.Links),
 		Attributes:             Attributes(spanData.Attributes),
 		DroppedAttributesCount: 1,
 		DroppedEventsCount:     2,
 		DroppedLinksCount:      3,
 	}
 
-	got := SpanData([]*tracesdk.SpanSnapshot{spanData})
+	got := Spans(tracetest.SpanStubs{spanData}.Snapshots())
 	require.Len(t, got, 1)
 
 	assert.Equal(t, got[0].GetResource(), Resource(spanData.Resource))
@@ -304,7 +305,7 @@ func TestSpanData(t *testing.T) {
 
 // Empty parent span ID should be treated as root span.
 func TestRootSpanData(t *testing.T) {
-	sd := SpanData([]*tracesdk.SpanSnapshot{{}})
+	sd := Spans(tracetest.SpanStubs{{}}.Snapshots())
 	require.Len(t, sd, 1)
 	rs := sd[0]
 	got := rs.GetInstrumentationLibrarySpans()[0].GetSpans()[0].GetParentSpanId()
@@ -314,5 +315,5 @@ func TestRootSpanData(t *testing.T) {
 }
 
 func TestSpanDataNilResource(t *testing.T) {
-	assert.NotPanics(t, func() { SpanData([]*tracesdk.SpanSnapshot{{}}) })
+	assert.NotPanics(t, func() { Spans(tracetest.SpanStubs{{}}.Snapshots()) })
 }

exporters/otlp/otlp.go

@@ -129,10 +129,10 @@ func (e *Exporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind)
 	return e.cfg.exportKindSelector.ExportKindFor(desc, kind)
 }
 
-// ExportSpans transforms and batches trace SpanSnapshots into OTLP Trace and
+// ExportSpans transforms and batches OpenTelemetry spans into OTLP Trace and
 // transmits them to the configured collector.
-func (e *Exporter) ExportSpans(ctx context.Context, ss []*tracesdk.SpanSnapshot) error {
-	return e.driver.ExportTraces(ctx, ss)
+func (e *Exporter) ExportSpans(ctx context.Context, spans []tracesdk.ReadOnlySpan) error {
+	return e.driver.ExportTraces(ctx, spans)
 }
 
 // NewExportPipeline sets up a complete export pipeline