From c68f95e0e7ded02e26ebbca6778bc00e5500720f Mon Sep 17 00:00:00 2001 From: Dan Mullineux Date: Thu, 14 Nov 2024 09:36:10 +0000 Subject: [PATCH] Cut down the golden span API --- o11y/honeycomb/honeycomb.go | 8 +------- o11y/o11y.go | 28 +++------------------------- o11y/otel/otel.go | 34 +++++----------------------------- o11y/otel/otel_test.go | 28 +++++++++++++--------------- 4 files changed, 22 insertions(+), 76 deletions(-) diff --git a/o11y/honeycomb/honeycomb.go b/o11y/honeycomb/honeycomb.go index c339f36f0..8b7bacb17 100644 --- a/o11y/honeycomb/honeycomb.go +++ b/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 diff --git a/o11y/o11y.go b/o11y/o11y.go index e8dff8571..dca043a88 100644 --- a/o11y/o11y.go +++ b/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{} diff --git a/o11y/otel/otel.go b/o11y/otel/otel.go index c1f22c8da..9940e261b 100644 --- a/o11y/otel/otel.go +++ b/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. diff --git a/o11y/otel/otel_test.go b/o11y/otel/otel_test.go index 6d3fc3967..de27eb89d 100644 --- a/o11y/otel/otel_test.go +++ b/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)) }) }) }