diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bfd3779ff7..4b9d788a519 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/sdk/trace/batch_span_processor.go b/sdk/trace/batch_span_processor.go index be06324cc20..e63ff4b6dfc 100644 --- a/sdk/trace/batch_span_processor.go +++ b/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) { diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go index f240c694682..67f72ad06df 100644 --- a/sdk/trace/batch_span_processor_test.go +++ b/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()) diff --git a/sdk/trace/provider_test.go b/sdk/trace/provider_test.go index edfd8735bc9..2b7b7a69f47 100644 --- a/sdk/trace/provider_test.go +++ b/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() diff --git a/sdk/trace/simple_span_processor.go b/sdk/trace/simple_span_processor.go index ac96fe5d01a..44f20e74ee8 100644 --- a/sdk/trace/simple_span_processor.go +++ b/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. diff --git a/sdk/trace/span_processor.go b/sdk/trace/span_processor.go index 188eb614f11..79f0c61919b 100644 --- a/sdk/trace/span_processor.go +++ b/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. diff --git a/sdk/trace/span_processor_example_test.go b/sdk/trace/span_processor_example_test.go index f2240785249..2a5a8b7fcbd 100644 --- a/sdk/trace/span_processor_example_test.go +++ b/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) { diff --git a/sdk/trace/span_processor_test.go b/sdk/trace/span_processor_test.go index 081307ce90c..a607a15d1c3 100644 --- a/sdk/trace/span_processor_test.go +++ b/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) } } } diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 55e90b836ba..872c37d84c6 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(span.data) + sp.sp.OnStart(ctx, span.data) } }