Skip to content

Commit

Permalink
Redefine ExportSpans of SpanExporter with ReadOnlySpan (open-telemetr…
Browse files Browse the repository at this point in the history
…y#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
  • Loading branch information
MrAlias authored May 4, 2021
1 parent c99d5e9 commit cbcd4b1
Show file tree
Hide file tree
Showing 34 changed files with 1,046 additions and 675 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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

Expand Down
64 changes: 33 additions & 31 deletions exporters/otlp/internal/otlptest/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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.
Expand Down
56 changes: 28 additions & 28 deletions exporters/otlp/internal/transform/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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{},
}
}
Expand All @@ -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
Expand All @@ -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[:]
}

Expand Down Expand Up @@ -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,
Expand Down
37 changes: 19 additions & 18 deletions exporters/otlp/internal/transform/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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},
Expand All @@ -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"),
},
},
},
Expand Down Expand Up @@ -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",
Expand All @@ -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))
Expand All @@ -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()
Expand All @@ -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()) })
}
6 changes: 3 additions & 3 deletions exporters/otlp/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit cbcd4b1

Please sign in to comment.