Add parent context to SpanProcessor.OnStart (#1333)

* Add parent context to SpanProcessor.OnStart

The spec requires doing so. Right now SpanProcessor implementations
aren't doing anything with this argument.

* Update changelog

* Fix typo in test name

CHANGELOG.md

@@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - A `TextMapPropagator` and associated `TextMapCarrier` are added to the `go.opentelemetry.io/otel/oteltest` package to test TextMap type propagators and their use. (#1259)
 - `SpanContextFromContext` returns `SpanContext` from context. (#1255)
 - Add an opencensus to opentelemetry tracing bridge. (#1305)
+- Add a parent context argument to `SpanProcessor.OnStart` to follow the specification. (#1333)
 
 ### Changed
 

sdk/trace/batch_span_processor.go

@@ -112,7 +112,7 @@ func NewBatchSpanProcessor(exporter export.SpanExporter, options ...BatchSpanPro
 }
 
 // OnStart method does nothing.
-func (bsp *BatchSpanProcessor) OnStart(sd *export.SpanData) {}
+func (bsp *BatchSpanProcessor) OnStart(parent context.Context, sd *export.SpanData) {}
 
 // OnEnd method enqueues export.SpanData for later processing.
 func (bsp *BatchSpanProcessor) OnEnd(sd *export.SpanData) {

sdk/trace/batch_span_processor_test.go

@@ -63,7 +63,7 @@ var _ export.SpanExporter = (*testBatchExporter)(nil)
 func TestNewBatchSpanProcessorWithNilExporter(t *testing.T) {
 	bsp := sdktrace.NewBatchSpanProcessor(nil)
 	// These should not panic.
-	bsp.OnStart(&export.SpanData{})
+	bsp.OnStart(context.Background(), &export.SpanData{})
 	bsp.OnEnd(&export.SpanData{})
 	bsp.ForceFlush()
 	err := bsp.Shutdown(context.Background())

sdk/trace/provider_test.go

@@ -30,9 +30,9 @@ func (t *basicSpanProcesor) Shutdown(context.Context) error {
 	return nil
 }
 
-func (t *basicSpanProcesor) OnStart(s *export.SpanData) {}
-func (t *basicSpanProcesor) OnEnd(s *export.SpanData)   {}
-func (t *basicSpanProcesor) ForceFlush()                {}
+func (t *basicSpanProcesor) OnStart(parent context.Context, s *export.SpanData) {}
+func (t *basicSpanProcesor) OnEnd(s *export.SpanData)                           {}
+func (t *basicSpanProcesor) ForceFlush()                                        {}
 
 func TestShutdownTraceProvider(t *testing.T) {
 	stp := NewTracerProvider()

sdk/trace/simple_span_processor.go

@@ -39,7 +39,7 @@ func NewSimpleSpanProcessor(exporter export.SpanExporter) *SimpleSpanProcessor {
 }
 
 // OnStart method does nothing.
-func (ssp *SimpleSpanProcessor) OnStart(sd *export.SpanData) {
+func (ssp *SimpleSpanProcessor) OnStart(parent context.Context, sd *export.SpanData) {
 }
 
 // OnEnd method exports SpanData using associated export.

sdk/trace/span_processor.go

@@ -26,7 +26,7 @@ type SpanProcessor interface {
 
 	// OnStart method is invoked when span is started. It is a synchronous call
 	// and hence should not block.
-	OnStart(sd *export.SpanData)
+	OnStart(parent context.Context, sd *export.SpanData)
 
 	// OnEnd method is invoked when span is finished. It is a synchronous call
 	// and hence should not block.

sdk/trace/span_processor_example_test.go

@@ -34,7 +34,9 @@ type DurationFilter struct {
 	Max time.Duration
 }
 
-func (f DurationFilter) OnStart(sd *export.SpanData)        { f.Next.OnStart(sd) }
+func (f DurationFilter) OnStart(parent context.Context, sd *export.SpanData) {
+	f.Next.OnStart(parent, sd)
+}
 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) {
@@ -60,7 +62,9 @@ type InstrumentationBlacklist struct {
 	Blacklist map[string]bool
 }
 
-func (f InstrumentationBlacklist) OnStart(sd *export.SpanData)        { f.Next.OnStart(sd) }
+func (f InstrumentationBlacklist) OnStart(parent context.Context, sd *export.SpanData) {
+	f.Next.OnStart(parent, sd)
+}
 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) {

sdk/trace/span_processor_test.go

@@ -20,6 +20,7 @@ import (
 
 	"go.opentelemetry.io/otel/label"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/trace"
 )
 
 type testSpanProcessor struct {
@@ -29,12 +30,27 @@ type testSpanProcessor struct {
 	shutdownCount int
 }
 
-func (t *testSpanProcessor) OnStart(s *export.SpanData) {
-	kv := label.KeyValue{
-		Key:   "OnStart",
-		Value: label.StringValue(t.name),
+func (t *testSpanProcessor) OnStart(parent context.Context, s *export.SpanData) {
+	psc := trace.RemoteSpanContextFromContext(parent)
+	kv := []label.KeyValue{
+		{
+			Key:   "SpanProcessorName",
+			Value: label.StringValue(t.name),
+		},
+		// Store parent trace ID and span ID as attributes to be read later in
+		// tests so that we "do something" with the parent argument. Real
+		// SpanProcessor implementations will likely use the parent argument in
+		// a more meaningful way.
+		{
+			Key:   "ParentTraceID",
+			Value: label.StringValue(psc.TraceID.String()),
+		},
+		{
+			Key:   "ParentSpanID",
+			Value: label.StringValue(psc.SpanID.String()),
+		},
 	}
-	s.Attributes = append(s.Attributes, kv)
+	s.Attributes = append(s.Attributes, kv...)
 	t.spansStarted = append(t.spansStarted, s)
 }
 
@@ -55,7 +71,7 @@ func (t *testSpanProcessor) Shutdown(_ context.Context) error {
 func (t *testSpanProcessor) ForceFlush() {
 }
 
-func TestRegisterSpanProcessort(t *testing.T) {
+func TestRegisterSpanProcessor(t *testing.T) {
 	name := "Register span processor before span starts"
 	tp := basicTracerProvider(t)
 	spNames := []string{"sp1", "sp2", "sp3"}
@@ -65,8 +81,16 @@ func TestRegisterSpanProcessort(t *testing.T) {
 		tp.RegisterSpanProcessor(sp)
 	}
 
+	tid, _ := trace.TraceIDFromHex("01020304050607080102040810203040")
+	sid, _ := trace.SpanIDFromHex("0102040810203040")
+	parent := trace.SpanContext{
+		TraceID: tid,
+		SpanID:  sid,
+	}
+	ctx := trace.ContextWithRemoteSpanContext(context.Background(), parent)
+
 	tr := tp.Tracer("SpanProcessor")
-	_, span := tr.Start(context.Background(), "OnStart")
+	_, span := tr.Start(ctx, "OnStart")
 	span.End()
 	wantCount := 1
 
@@ -81,18 +105,40 @@ func TestRegisterSpanProcessort(t *testing.T) {
 		}
 
 		c := 0
+		tidOK := false
+		sidOK := false
 		for _, kv := range sp.spansStarted[0].Attributes {
-			if kv.Key != "OnStart" {
+			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
 			}
-			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(OnStart): got %d, want %d\n", name, c, len(spNames))
+			t.Errorf("%s: expected attributes(SpanProcessorName): got %d, want %d\n", name, c, len(spNames))
+		}
+		if !tidOK {
+			t.Errorf("%s: expected attributes(ParentTraceID)\n", name)
+		}
+		if !sidOK {
+			t.Errorf("%s: expected attributes(ParentSpanID)\n", name)
 		}
 	}
 }

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(span.data)
+			sp.sp.OnStart(ctx, span.data)
 		}
 	}