From 2152f73e6a309c665e3b93db05f4c701159249ea Mon Sep 17 00:00:00 2001 From: ale <ale@incal.net> Date: Wed, 17 Aug 2022 09:36:51 +0100 Subject: [PATCH] Drop the global Tracer instance Use the preferred otel.GetTracerProvider() idiom and create dedicated Tracer instances as needed, which seems to be best practice. --- ldap/pool.go | 17 +++++------------ tracing/tracing.go | 21 +++++++++++---------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/ldap/pool.go b/ldap/pool.go index f6831fe..15498f1 100644 --- a/ldap/pool.go +++ b/ldap/pool.go @@ -7,10 +7,10 @@ import ( "net/url" "time" - "git.autistici.org/ai3/go-common/tracing" "github.com/cenkalti/backoff/v4" "github.com/go-ldap/ldap/v3" "github.com/prometheus/client_golang/prometheus" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" @@ -158,14 +158,11 @@ func NewConnectionPool(uri, bindDN, bindPw string, cacheSize int) (*ConnectionPo func (p *ConnectionPool) doRequest(ctx context.Context, name string, attrs []attribute.KeyValue, fn func(*ldap.Conn) error) error { // Tracing: initialize a new client span. - var span trace.Span - if tracing.Enabled { - ctx, span = tracing.Tracer.Start(ctx, name, trace.WithSpanKind(trace.SpanKindClient)) - defer span.End() + ctx, span := otel.GetTracerProvider().Tracer("ldap").Start(ctx, name, trace.WithSpanKind(trace.SpanKindClient)) + defer span.End() - if len(attrs) > 0 { - span.SetAttributes(attrs...) - } + if len(attrs) > 0 { + span.SetAttributes(attrs...) } rerr := backoff.Retry(func() error { @@ -269,10 +266,6 @@ func isProtocolError(err error) bool { } func setSpanStatus(span trace.Span, err error) { - if span == nil { - return - } - switch err { case nil: span.SetStatus(codes.Ok, "OK") diff --git a/tracing/tracing.go b/tracing/tracing.go index f563090..f9cec22 100644 --- a/tracing/tracing.go +++ b/tracing/tracing.go @@ -19,16 +19,12 @@ import ( "go.opentelemetry.io/otel/sdk/resource" "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.12.0" - apitrace "go.opentelemetry.io/otel/trace" ) var ( // Enabled reports whether tracing is globally enabled or not. Enabled bool - // Global Tracer instance. - Tracer apitrace.Tracer - initOnce sync.Once ) @@ -116,6 +112,9 @@ func initTracing(serviceName string) { return } + // The sampling policy only applies to incoming requests for + // which tracing is not already enabled: in this case, we + // always pass-through. var sampler trace.Sampler switch config.Sample { case "", "always": @@ -132,13 +131,12 @@ func initTracing(serviceName string) { } tp := trace.NewTracerProvider( - trace.WithSampler(sampler), + trace.WithSampler(trace.ParentBased(sampler)), trace.WithBatcher(ze), trace.WithResource(defaultResource(serviceName)), ) otel.SetTracerProvider(tp) - Tracer = tp.Tracer(serviceName) otel.SetTextMapPropagator( propagation.NewCompositeTextMapPropagator( @@ -163,7 +161,7 @@ func Init() { // Must call Init() first. func WrapTransport(t http.RoundTripper) http.RoundTripper { if Enabled { - t = othttp.NewTransport(t, othttp.WithPublicEndpoint()) + t = othttp.NewTransport(t) } return t } @@ -178,7 +176,10 @@ func WrapHandler(h http.Handler, endpointAddr string) http.Handler { } // Format span names with the request URL path. - return othttp.NewHandler(h, serviceName, othttp.WithSpanNameFormatter(func(op string, r *http.Request) string { - return r.URL.Path - })) + return othttp.NewHandler( + h, serviceName, + othttp.WithSpanNameFormatter(func(op string, r *http.Request) string { + return r.URL.Path + }), + ) } -- GitLab