Merge pull request #1040 from circleci/dm/cut-down-golden

Cut down the golden span API

o11y/honeycomb/honeycomb.go

@@ -370,13 +370,7 @@ func (h *honeycomb) Helpers(disableW3c ...bool) o11y.Helpers {
 }
 
 // N.B. Golden trace will not be implemented in honeycomb provider
-
-func (h *honeycomb) StartGoldenTrace(ctx context.Context, _ string) context.Context { return ctx }
-func (h *honeycomb) EndGoldenTrace(ctx context.Context)                             {}
-func (h *honeycomb) StartGoldenSpan(ctx context.Context, _ string, _ ...o11y.SpanOpt) (context.Context, o11y.Span) {
-	return ctx, nil
-}
-func (h *honeycomb) MakeSpanGolden(context.Context) {}
+func (h *honeycomb) MakeSpanGolden(ctx context.Context) context.Context { return ctx }
 
 type helpers struct {
 	disableW3c bool

o11y/o11y.go

@@ -63,23 +63,9 @@ type Provider interface {
 	// Helpers returns some specific helper functions. Temporary optional param during the cutover to otel
 	Helpers(disableW3c ...bool) Helpers
 
-	// StartGoldenTrace starts the golden trace.
-	// TODO - more notes / docs about what a golden trace is.
-	StartGoldenTrace(ctx context.Context, name string) context.Context
-
-	// EndGoldenTrace should be called when the golden trace has ended. This ends the root golden span, but the
-	// golden trace id will still be used for other golden spans.
-	// This does not need to be called, but it is better if something does call it, to avoid missing spans in the
-	// visualisation tools.
-	EndGoldenTrace(ctx context.Context)
-
-	// StartGoldenSpan Starts a normal span and an associated golden span attached to the golden trace.
+	// MakeSpanGolden Add a golden span from the span currently in the context.
 	// If the golden trace does not exist it will be started.
-	// Opts are generally expected to be set by other ex packages, rather than application code
-	StartGoldenSpan(ctx context.Context, name string, opts ...SpanOpt) (context.Context, Span)
-
-	// MakeSpanGolden Add a golden span from the span currently in the context
-	MakeSpanGolden(ctx context.Context)
+	MakeSpanGolden(ctx context.Context) context.Context
 }
 
 // PropagationContext contains trace context values that are propagated from service to service.
@@ -368,21 +354,13 @@ var defaultProvider = &noopProvider{}
 
 type noopProvider struct{}
 
-func (c *noopProvider) StartGoldenTrace(ctx context.Context, _ string) context.Context {
-	return ctx
-}
-
 func (c *noopProvider) AddGlobalField(string, interface{}) {}
 
 func (c *noopProvider) StartSpan(ctx context.Context, _ string, _ ...SpanOpt) (context.Context, Span) {
 	return ctx, &noopSpan{}
 }
 
-func (c *noopProvider) EndGoldenTrace(context.Context) {}
-func (c *noopProvider) StartGoldenSpan(ctx context.Context, _ string, _ ...SpanOpt) (context.Context, Span) {
-	return ctx, &noopSpan{}
-}
-func (c *noopProvider) MakeSpanGolden(context.Context) {}
+func (c *noopProvider) MakeSpanGolden(ctx context.Context) context.Context { return ctx }
 
 func (c *noopProvider) GetSpan(context.Context) Span {
 	return &noopSpan{}

o11y/otel/otel.go

@@ -219,7 +219,7 @@ type golden struct {
 
 const metaGolden = "meta.golden"
 
-func (o Provider) StartGoldenTrace(ctx context.Context, name string) context.Context {
+func (o Provider) startGoldenTrace(ctx context.Context, name string) context.Context {
 	spec := o.getGolden(ctx)
 	if spec != nil {
 		return ctx
@@ -234,36 +234,10 @@ func (o Provider) StartGoldenTrace(ctx context.Context, name string) context.Con
 	return context.WithValue(ctx, goldenCtxKey{}, spec)
 }
 
-func (o Provider) EndGoldenTrace(ctx context.Context) {
-	s := o.getGolden(ctx)
-	if s == nil {
-		return
-	}
-	s.span.End()
-}
-
-func (o Provider) StartGoldenSpan(ctx context.Context, name string, opts ...o11y.SpanOpt) (context.Context, o11y.Span) {
+func (o Provider) MakeSpanGolden(ctx context.Context) context.Context {
 	spec := o.getGolden(ctx)
 	if spec == nil {
-		ctx = o.StartGoldenTrace(ctx, "unknown-golden-trace")
-		spec = o.getGolden(ctx)
-	}
-	// Start the normal span
-	ctx, _ = o.StartSpan(ctx, name, opts...)
-	sp := o.getSpan(ctx)
-
-	// Start the golden span
-	spec.ctx, _ = o.StartSpan(spec.ctx, name, opts...)
-	sp.golden = o.getSpan(spec.ctx)
-	sp.golden.AddRawField(metaGolden, true)
-	sp.link(sp.golden)
-	return ctx, sp
-}
-
-func (o Provider) MakeSpanGolden(ctx context.Context) {
-	spec := o.getGolden(ctx)
-	if spec == nil {
-		ctx = o.StartGoldenTrace(ctx, "unknown-golden-trace")
+		ctx = o.startGoldenTrace(ctx, "root")
 		spec = o.getGolden(ctx)
 	}
 
@@ -275,6 +249,8 @@ func (o Provider) MakeSpanGolden(ctx context.Context) {
 	sp.golden = o.getSpan(spec.ctx)
 	sp.golden.AddRawField(metaGolden, true)
 	sp.link(sp.golden)
+
+	return ctx
 }
 
 // GetSpan returns the active span in the given context. It will return nil if there is no span available.

o11y/otel/otel_test.go

@@ -415,8 +415,8 @@ func TestHelpers(t *testing.T) {
 			"bg2": "bgv2",
 		})
 
-		ctx = provider.StartGoldenTrace(ctx, "gi")
-		ctx, gsp := provider.StartGoldenSpan(ctx, "important")
+		ctx, gsp := o11y.StartSpan(ctx, "important")
+		ctx = provider.MakeSpanGolden(ctx)
 		gsp.End()
 
 		svc1Propagation := h.ExtractPropagation(ctx)
@@ -442,11 +442,10 @@ func TestHelpers(t *testing.T) {
 		assert.Check(t, cmp.Equal(b["bg1"], "bgv1"))
 		assert.Check(t, cmp.Equal(b["bg2"], "bgv2"))
 
-		ctx, gsp2 := provider.StartGoldenSpan(service2Context, "important-2")
+		ctx, gsp2 := o11y.StartSpan(ctx, "important-2")
+		ctx = provider.MakeSpanGolden(ctx)
 		gsp2.End()
 
-		provider.EndGoldenTrace(service2Context)
-
 		span.End()
 
 		closeProvider(ctx)
@@ -738,7 +737,7 @@ func TestGolden(t *testing.T) {
 		ctx, closeProvider, err := o11yconfig.Otel(ctx, o11yconfig.OtelConfig{
 			Dataset:         "local-testing",
 			GrpcHostAndPort: "127.0.0.1:4317",
-			Service:         "app-main",
+			Service:         "app-main-gold",
 			Version:         "dev-test",
 			Statsd:          s.Addr(),
 			StatsNamespace:  "test-app",
@@ -760,17 +759,16 @@ func TestGolden(t *testing.T) {
 			// need to close the provider to be sure traces flushed
 			defer closeProvider(ctx)
 			defer func() {
-				o.EndGoldenTrace(ctx)
 				time.Sleep(time.Millisecond * 2)
 
-				_, span := o.StartGoldenSpan(ctx, "key-event")
+				ctx, span := o.StartSpan(ctx, "key-event")
+				ctx = o.MakeSpanGolden(ctx)
 				time.Sleep(time.Millisecond)
 				span.End()
 			}()
 
-			ctx = o.StartGoldenTrace(ctx, "trigger")
-
-			ctx, span = o.StartGoldenSpan(ctx, "trigger-event", o11y.WithSpanKind(o11y.SpanKindServer))
+			ctx, span = o.StartSpan(ctx, "trigger-event", o11y.WithSpanKind(o11y.SpanKindServer))
+			ctx = o.MakeSpanGolden(ctx)
 			defer span.End()
 
 			doSomething(ctx, false)
@@ -790,7 +788,7 @@ func TestGolden(t *testing.T) {
 		}()
 
 		t.Run("check", func(t *testing.T) {
-			jc := jaeger.New("http://localhost:16686", "app-main")
+			jc := jaeger.New("http://localhost:16686", "app-main-gold")
 			traces, err := jc.Traces(ctx, start)
 			assert.NilError(t, err)
 
@@ -801,7 +799,7 @@ func TestGolden(t *testing.T) {
 			for _, trc := range traces {
 				spans = append(spans, trc.Spans...)
 			}
-			assert.Check(t, cmp.Len(spans, 10))
+			assert.Check(t, cmp.Len(spans, 9))
 
 			cnt := 0
 			cntG := 0
@@ -832,8 +830,8 @@ func TestGolden(t *testing.T) {
 			}
 
 			// Check golden spans are duplicated as non-golden (except the start span)
-			assert.Check(t, cmp.Equal(cnt, 5))
-			assert.Check(t, cmp.Equal(cntG, 3))
+			assert.Check(t, cmp.Equal(cnt, 4))
+			assert.Check(t, cmp.Equal(cntG, 2))
 		})
 	})
 }