From 29a75e962f8ea33b896c13a9bbe77e7d26f8596d Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Fri, 7 Jul 2023 19:40:58 +0000 Subject: [PATCH 01/36] adds OTLP to jtracer Signed-off-by: Afzal Ansari --- pkg/jtracer/jtracer.go | 45 +++++++++++++++++++++++++++++++++++-- pkg/jtracer/jtracer_test.go | 10 ++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index d502767dc86..16c3f55a3f2 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -14,16 +14,57 @@ package jtracer -import "github.com/opentracing/opentracing-go" +import ( + "context" + "fmt" + + "github.com/opentracing/opentracing-go" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/propagation" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" +) type JTracer struct { - OT opentracing.Tracer + OT opentracing.Tracer + OTEL trace.TracerProvider } func OT(t opentracing.Tracer) JTracer { return JTracer{OT: t} } +func OTEL(tp trace.TracerProvider) JTracer { + return JTracer{OTEL: tp} +} + func NoOp() JTracer { return JTracer{OT: opentracing.NoopTracer{}} } + +func NoOpOtel() JTracer { + return JTracer{OTEL: trace.NewNoopTracerProvider()} +} + +func NewOTELProvider() (trace.TracerProvider, error) { + opts := []otlptracehttp.Option{otlptracehttp.WithInsecure()} + exporter, err := otlptrace.New( + context.Background(), + otlptracehttp.NewClient(opts...), + ) + if err != nil { + return nil, fmt.Errorf("failed to create exporter: %w", err) + } + + tracerProvider := sdktrace.NewTracerProvider( + sdktrace.WithBatcher(exporter), + sdktrace.WithSampler(sdktrace.AlwaysSample()), + ) + + otel.SetTracerProvider(tracerProvider) + otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) + + return tracerProvider, nil +} diff --git a/pkg/jtracer/jtracer_test.go b/pkg/jtracer/jtracer_test.go index e4bffa19937..31e05041306 100644 --- a/pkg/jtracer/jtracer_test.go +++ b/pkg/jtracer/jtracer_test.go @@ -19,13 +19,21 @@ import ( "github.com/opentracing/opentracing-go" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/trace" "github.com/jaegertracing/jaeger/pkg/jtracer" ) func TestOT(t *testing.T) { mockTracer := opentracing.NoopTracer{} - jtracer := jtracer.OT(opentracing.NoopTracer{}) + jtracer := jtracer.OT(mockTracer) assert.Equal(t, mockTracer, jtracer.OT) } + +func TestOTEL(t *testing.T) { + mockTracer := trace.NewNoopTracerProvider() + jtracer := jtracer.OTEL(mockTracer) + + assert.Equal(t, mockTracer, jtracer.OTEL) +} From 9dbdbe0f667cda839953764db718ad6173a4adc0 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Fri, 7 Jul 2023 19:41:44 +0000 Subject: [PATCH 02/36] replace ot with otlp Signed-off-by: Afzal Ansari --- .../metrics/prometheus/metricsstore/reader.go | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 0cf6852183d..2f601271b40 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -26,14 +26,16 @@ import ( "time" "unicode" - "github.com/opentracing/opentracing-go" - ottag "github.com/opentracing/opentracing-go/ext" - otlog "github.com/opentracing/opentracing-go/log" "github.com/prometheus/client_golang/api" promapi "github.com/prometheus/client_golang/api/prometheus/v1" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" + "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/bearertoken" + "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/prometheus/config" "github.com/jaegertracing/jaeger/plugin/metrics/prometheus/metricsstore/dbmodel" "github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics" @@ -225,7 +227,7 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( promQuery := m.buildPromQuery(p) span, ctx := startSpanForQuery(ctx, p.metricName, promQuery) - defer span.Finish() + defer span.End() queryRange := promapi.Range{ Start: p.EndTime.Add(-1 * *p.Lookback), @@ -287,17 +289,20 @@ func promqlDurationString(d *time.Duration) string { return string(b) } -func startSpanForQuery(ctx context.Context, metricName, query string) (opentracing.Span, context.Context) { - span, ctx := opentracing.StartSpanFromContext(ctx, metricName) - ottag.DBStatement.Set(span, query) - ottag.DBType.Set(span, "prometheus") - ottag.Component.Set(span, "promql") +func startSpanForQuery(ctx context.Context, metricName, query string) (trace.Span, context.Context) { + tp, _ := jtracer.NewOTELProvider() + ctx, span := tp.Tracer("").Start(ctx, metricName) + span.SetAttributes( + attribute.Key(semconv.DBStatementKey).String(query), + attribute.Key("db.type").String("prometheus"), + attribute.Key("component").String("promql"), + ) return span, ctx } -func logErrorToSpan(span opentracing.Span, err error) { - ottag.Error.Set(span, true) - span.LogFields(otlog.Error(err)) +func logErrorToSpan(span trace.Span, err error) { + // span.SetStatus(codes.Error, semconv.OTelStatusCodeError.Value.AsString()) + span.AddEvent(err.Error(), trace.WithAttributes(semconv.OTelStatusCodeError)) } func getHTTPRoundTripper(c *config.Configuration, logger *zap.Logger) (rt http.RoundTripper, err error) { From f739427e80323625419e3f7dec57f38f99bc0188 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sat, 8 Jul 2023 19:52:06 +0000 Subject: [PATCH 03/36] made singleton otel tracer Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 3 +- cmd/query/app/http_handler_test.go | 3 +- cmd/query/main.go | 3 +- pkg/jtracer/jtracer.go | 48 +++++++++++-------- pkg/jtracer/jtracer_test.go | 24 +--------- .../metrics/prometheus/metricsstore/reader.go | 11 ++--- 6 files changed, 40 insertions(+), 52 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index b40e427cfa7..d1537c53127 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -26,6 +26,7 @@ import ( "github.com/spf13/viper" jaegerClientConfig "github.com/uber/jaeger-client-go/config" jaegerClientZapLog "github.com/uber/jaeger-client-go/log/zap" + "go.opentelemetry.io/otel/trace" _ "go.uber.org/automaxprocs" "go.uber.org/zap" @@ -274,7 +275,7 @@ func startQuery( ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - jtracer := jtracer.OT(opentracing.GlobalTracer()) + jtracer := jtracer.New(opentracing.GlobalTracer(), trace.NewNoopTracerProvider()) server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jtracer) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 04604526ac6..53d2509f15c 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -36,6 +36,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/uber/jaeger-client-go" + "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -305,7 +306,7 @@ func TestGetTrace(t *testing.T) { t.Run(testCase.suffix, func(t *testing.T) { reporter := jaeger.NewInMemoryReporter() jaegerTracer, jaegerCloser := jaeger.NewTracer("test", jaeger.NewConstSampler(true), reporter) - jTracer := jtracer.OT(jaegerTracer) + jTracer := jtracer.New(jaegerTracer, trace.NewNoopTracerProvider()) defer jaegerCloser.Close() ts := initializeTestServer(HandlerOptions.Tracer(jTracer)) diff --git a/cmd/query/main.go b/cmd/query/main.go index ab88bef240e..3705a9f35c8 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/viper" jaegerClientConfig "github.com/uber/jaeger-client-go/config" jaegerClientZapLog "github.com/uber/jaeger-client-go/log/zap" + "go.opentelemetry.io/otel/trace" _ "go.uber.org/automaxprocs" "go.uber.org/zap" @@ -97,7 +98,7 @@ func main() { } defer closer.Close() opentracing.SetGlobalTracer(tracer) - jtracer := jtracer.OT(tracer) + jtracer := jtracer.New(tracer, trace.NewNoopTracerProvider()) queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 16c3f55a3f2..7b562352938 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -16,14 +16,16 @@ package jtracer import ( "context" - "fmt" + "log" "github.com/opentracing/opentracing-go" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" + semconv "go.opentelemetry.io/otel/semconv/v1.4.0" "go.opentelemetry.io/otel/trace" ) @@ -32,39 +34,45 @@ type JTracer struct { OTEL trace.TracerProvider } -func OT(t opentracing.Tracer) JTracer { - return JTracer{OT: t} -} +var globalTracerProvider trace.TracerProvider -func OTEL(tp trace.TracerProvider) JTracer { - return JTracer{OTEL: tp} +func New(t opentracing.Tracer, tp trace.TracerProvider) JTracer { + return JTracer{OT: t, OTEL: tp} } func NoOp() JTracer { - return JTracer{OT: opentracing.NoopTracer{}} -} - -func NoOpOtel() JTracer { - return JTracer{OTEL: trace.NewNoopTracerProvider()} + return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} } -func NewOTELProvider() (trace.TracerProvider, error) { +func init() { opts := []otlptracehttp.Option{otlptracehttp.WithInsecure()} - exporter, err := otlptrace.New( + traceExporter, err := otlptrace.New( context.Background(), otlptracehttp.NewClient(opts...), ) if err != nil { - return nil, fmt.Errorf("failed to create exporter: %w", err) + log.Fatal("failed to create exporter: %w", err) } - tracerProvider := sdktrace.NewTracerProvider( - sdktrace.WithBatcher(exporter), - sdktrace.WithSampler(sdktrace.AlwaysSample()), + // Register the trace exporter with a TracerProvider, using a batch + // span processor to aggregate spans before export. + bsp := sdktrace.NewBatchSpanProcessor(traceExporter) + globalTracerProvider = sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(bsp), + sdktrace.WithResource(resource.NewWithAttributes( + semconv.SchemaURL, + semconv.ServiceNameKey.String("otlp"), + )), ) - otel.SetTracerProvider(tracerProvider) - otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) + otel.SetTracerProvider(globalTracerProvider) + otel.SetTextMapPropagator( + propagation.NewCompositeTextMapPropagator( + propagation.TraceContext{}, + propagation.Baggage{}, + )) +} - return tracerProvider, nil +func GetTracerProvider() trace.TracerProvider { + return globalTracerProvider } diff --git a/pkg/jtracer/jtracer_test.go b/pkg/jtracer/jtracer_test.go index 31e05041306..e2c2d657223 100644 --- a/pkg/jtracer/jtracer_test.go +++ b/pkg/jtracer/jtracer_test.go @@ -14,26 +14,4 @@ package jtracer_test -import ( - "testing" - - "github.com/opentracing/opentracing-go" - "github.com/stretchr/testify/assert" - "go.opentelemetry.io/otel/trace" - - "github.com/jaegertracing/jaeger/pkg/jtracer" -) - -func TestOT(t *testing.T) { - mockTracer := opentracing.NoopTracer{} - jtracer := jtracer.OT(mockTracer) - - assert.Equal(t, mockTracer, jtracer.OT) -} - -func TestOTEL(t *testing.T) { - mockTracer := trace.NewNoopTracerProvider() - jtracer := jtracer.OTEL(mockTracer) - - assert.Equal(t, mockTracer, jtracer.OTEL) -} +// TODO diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 2f601271b40..bd3a549003d 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -31,7 +31,6 @@ import ( "go.opentelemetry.io/otel/attribute" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" - "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/bearertoken" @@ -226,7 +225,7 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( } promQuery := m.buildPromQuery(p) - span, ctx := startSpanForQuery(ctx, p.metricName, promQuery) + ctx, span := startSpanForQuery(ctx, p.metricName, promQuery) defer span.End() queryRange := promapi.Range{ @@ -289,15 +288,15 @@ func promqlDurationString(d *time.Duration) string { return string(b) } -func startSpanForQuery(ctx context.Context, metricName, query string) (trace.Span, context.Context) { - tp, _ := jtracer.NewOTELProvider() - ctx, span := tp.Tracer("").Start(ctx, metricName) +func startSpanForQuery(ctx context.Context, metricName, query string) (context.Context, trace.Span) { + tp := jtracer.GetTracerProvider() + ctx, span := tp.Tracer("prom-metrics-reader").Start(ctx, metricName) span.SetAttributes( attribute.Key(semconv.DBStatementKey).String(query), attribute.Key("db.type").String("prometheus"), attribute.Key("component").String("promql"), ) - return span, ctx + return ctx, span } func logErrorToSpan(span trace.Span, err error) { From d4ce5cf978b9ebe1cf5c399154ebad4c5d1978db Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sun, 9 Jul 2023 07:40:01 +0000 Subject: [PATCH 04/36] subjugate initialise method to support both tracer Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 5 +- cmd/query/app/handler_options.go | 2 +- cmd/query/app/http_handler_test.go | 3 +- cmd/query/main.go | 23 +--------- pkg/jtracer/jtracer.go | 46 +++++++++++++------ .../metrics/prometheus/metricsstore/reader.go | 2 +- 6 files changed, 39 insertions(+), 42 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index d1537c53127..4dba33f453b 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -26,7 +26,6 @@ import ( "github.com/spf13/viper" jaegerClientConfig "github.com/uber/jaeger-client-go/config" jaegerClientZapLog "github.com/uber/jaeger-client-go/log/zap" - "go.opentelemetry.io/otel/trace" _ "go.uber.org/automaxprocs" "go.uber.org/zap" @@ -275,8 +274,8 @@ func startQuery( ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - jtracer := jtracer.New(opentracing.GlobalTracer(), trace.NewNoopTracerProvider()) - server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jtracer) + tracer := jtracer.New() + server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, tracer) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go index 78d34db8221..b39db2a3a7a 100644 --- a/cmd/query/app/handler_options.go +++ b/cmd/query/app/handler_options.go @@ -64,7 +64,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration) // Tracer creates a HandlerOption that initializes OpenTracing tracer func (handlerOptions) Tracer(tracer jtracer.JTracer) HandlerOption { return func(apiHandler *APIHandler) { - apiHandler.tracer = tracer + apiHandler.tracer.OT = tracer.OT } } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 53d2509f15c..49e77eaf477 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -36,7 +36,6 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/uber/jaeger-client-go" - "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -306,7 +305,7 @@ func TestGetTrace(t *testing.T) { t.Run(testCase.suffix, func(t *testing.T) { reporter := jaeger.NewInMemoryReporter() jaegerTracer, jaegerCloser := jaeger.NewTracer("test", jaeger.NewConstSampler(true), reporter) - jTracer := jtracer.New(jaegerTracer, trace.NewNoopTracerProvider()) + jTracer := jtracer.JTracer{OT: jaegerTracer} defer jaegerCloser.Close() ts := initializeTestServer(HandlerOptions.Tracer(jTracer)) diff --git a/cmd/query/main.go b/cmd/query/main.go index 3705a9f35c8..bd263ef488d 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -20,12 +20,8 @@ import ( "log" "os" - "github.com/opentracing/opentracing-go" "github.com/spf13/cobra" "github.com/spf13/viper" - jaegerClientConfig "github.com/uber/jaeger-client-go/config" - jaegerClientZapLog "github.com/uber/jaeger-client-go/log/zap" - "go.opentelemetry.io/otel/trace" _ "go.uber.org/automaxprocs" "go.uber.org/zap" @@ -35,7 +31,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/cmd/status" - "github.com/jaegertracing/jaeger/internal/metrics/jlibadapter" "github.com/jaegertracing/jaeger/pkg/bearertoken" "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/jtracer" @@ -77,28 +72,14 @@ func main() { metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) version.NewInfoMetrics(metricsFactory) - traceCfg := &jaegerClientConfig.Configuration{ - ServiceName: "jaeger-query", - Sampler: &jaegerClientConfig.SamplerConfig{ - Type: "const", - Param: 1.0, - }, - RPCMetrics: true, - } - traceCfg, err = traceCfg.FromEnv() if err != nil { logger.Fatal("Failed to read tracer configuration", zap.Error(err)) } - tracer, closer, err := traceCfg.NewTracer( - jaegerClientConfig.Metrics(jlibadapter.NewAdapter(svc.MetricsFactory)), - jaegerClientConfig.Logger(jaegerClientZapLog.NewLogger(logger)), - ) if err != nil { logger.Fatal("Failed to initialize tracer", zap.Error(err)) } - defer closer.Close() - opentracing.SetGlobalTracer(tracer) - jtracer := jtracer.New(tracer, trace.NewNoopTracerProvider()) + + jtracer := jtracer.New() queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 7b562352938..93871a12872 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -17,9 +17,11 @@ package jtracer import ( "context" "log" + "sync" "github.com/opentracing/opentracing-go" "go.opentelemetry.io/otel" + otbridge "go.opentelemetry.io/otel/bridge/opentracing" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" "go.opentelemetry.io/otel/propagation" @@ -34,17 +36,32 @@ type JTracer struct { OTEL trace.TracerProvider } -var globalTracerProvider trace.TracerProvider +var once sync.Once -func New(t opentracing.Tracer, tp trace.TracerProvider) JTracer { - return JTracer{OT: t, OTEL: tp} +func New() JTracer { + return JTracer{OT: initOT(), OTEL: initOTEL()} } func NoOp() JTracer { return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} } -func init() { +// InitOTEL initializes OpenTelemetry SDK. +func initOTEL() trace.TracerProvider { + _, oteltp := initBoth() + + return oteltp +} + +// Init returns OTel-OpenTracing Bridge. +func initOT() opentracing.Tracer { + otTracer, _ := initBoth() + + return otTracer +} + +// initBOTH initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge +func initBoth() (opentracing.Tracer, trace.TracerProvider) { opts := []otlptracehttp.Option{otlptracehttp.WithInsecure()} traceExporter, err := otlptrace.New( context.Background(), @@ -57,7 +74,7 @@ func init() { // Register the trace exporter with a TracerProvider, using a batch // span processor to aggregate spans before export. bsp := sdktrace.NewBatchSpanProcessor(traceExporter) - globalTracerProvider = sdktrace.NewTracerProvider( + tracerProvider := sdktrace.NewTracerProvider( sdktrace.WithSpanProcessor(bsp), sdktrace.WithResource(resource.NewWithAttributes( semconv.SchemaURL, @@ -65,14 +82,15 @@ func init() { )), ) - otel.SetTracerProvider(globalTracerProvider) - otel.SetTextMapPropagator( - propagation.NewCompositeTextMapPropagator( - propagation.TraceContext{}, - propagation.Baggage{}, - )) -} + once.Do(func() { + otel.SetTextMapPropagator( + propagation.NewCompositeTextMapPropagator( + propagation.TraceContext{}, + propagation.Baggage{}, + )) + }) + + otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) -func GetTracerProvider() trace.TracerProvider { - return globalTracerProvider + return otTracer, tracerProvider } diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index bd3a549003d..7699787ff5c 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -289,7 +289,7 @@ func promqlDurationString(d *time.Duration) string { } func startSpanForQuery(ctx context.Context, metricName, query string) (context.Context, trace.Span) { - tp := jtracer.GetTracerProvider() + tp := jtracer.New().OTEL ctx, span := tp.Tracer("prom-metrics-reader").Start(ctx, metricName) span.SetAttributes( attribute.Key(semconv.DBStatementKey).String(query), From 642d77250ae20a479a2fdae94940f2b259da857a Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sun, 9 Jul 2023 19:03:46 +0000 Subject: [PATCH 05/36] adds shudown method to tp Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 2 +- pkg/jtracer/jtracer.go | 30 ++++++++----------- .../metrics/prometheus/metricsstore/reader.go | 7 ++--- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 4dba33f453b..de157d11b82 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -274,7 +274,7 @@ func startQuery( ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - tracer := jtracer.New() + tracer := jtracer.JTracer{OT: opentracing.GlobalTracer()} server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, tracer) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 93871a12872..31916f179cc 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -39,28 +39,15 @@ type JTracer struct { var once sync.Once func New() JTracer { - return JTracer{OT: initOT(), OTEL: initOTEL()} + opentracingTracer, otelTracerProvider := initBoth() + return JTracer{OT: opentracingTracer, OTEL: otelTracerProvider} } func NoOp() JTracer { return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} } -// InitOTEL initializes OpenTelemetry SDK. -func initOTEL() trace.TracerProvider { - _, oteltp := initBoth() - - return oteltp -} - -// Init returns OTel-OpenTracing Bridge. -func initOT() opentracing.Tracer { - otTracer, _ := initBoth() - - return otTracer -} - -// initBOTH initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge +// initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge func initBoth() (opentracing.Tracer, trace.TracerProvider) { opts := []otlptracehttp.Option{otlptracehttp.WithInsecure()} traceExporter, err := otlptrace.New( @@ -90,7 +77,14 @@ func initBoth() (opentracing.Tracer, trace.TracerProvider) { )) }) - otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) + // Use the bridgeTracer as your OpenTracing tracer(otTrace). + otTracer, wrapperTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) + + // Shutdown the tracerProvider to clean up resources + err = tracerProvider.Shutdown(context.Background()) + if err != nil { + log.Fatal("failed to shutdown tracerProvider: ", err) + } - return otTracer, tracerProvider + return otTracer, wrapperTracerProvider } diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 7699787ff5c..34528cc4dc3 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -225,7 +225,8 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( } promQuery := m.buildPromQuery(p) - ctx, span := startSpanForQuery(ctx, p.metricName, promQuery) + tp := jtracer.New().OTEL + ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, tp) defer span.End() queryRange := promapi.Range{ @@ -288,8 +289,7 @@ func promqlDurationString(d *time.Duration) string { return string(b) } -func startSpanForQuery(ctx context.Context, metricName, query string) (context.Context, trace.Span) { - tp := jtracer.New().OTEL +func startSpanForQuery(ctx context.Context, metricName, query string, tp trace.TracerProvider) (context.Context, trace.Span) { ctx, span := tp.Tracer("prom-metrics-reader").Start(ctx, metricName) span.SetAttributes( attribute.Key(semconv.DBStatementKey).String(query), @@ -300,7 +300,6 @@ func startSpanForQuery(ctx context.Context, metricName, query string) (context.C } func logErrorToSpan(span trace.Span, err error) { - // span.SetStatus(codes.Error, semconv.OTelStatusCodeError.Value.AsString()) span.AddEvent(err.Error(), trace.WithAttributes(semconv.OTelStatusCodeError)) } From 8425410a4c9f44a8ceaa1d95c42c8a38f4c6cefa Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sun, 9 Jul 2023 20:12:01 +0000 Subject: [PATCH 06/36] creates tracer in the parent method Signed-off-by: Afzal Ansari --- plugin/metrics/prometheus/metricsstore/reader.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 34528cc4dc3..9ad141e683d 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -55,6 +55,7 @@ type ( latencyMetricName string callsMetricName string operationLabel string + tracer jtracer.JTracer } promQueryParams struct { @@ -102,6 +103,7 @@ func NewMetricsReader(logger *zap.Logger, cfg config.Configuration) (*MetricsRea callsMetricName: buildFullCallsMetricName(cfg), latencyMetricName: buildFullLatencyMetricName(cfg), operationLabel: operationLabel, + tracer: jtracer.New(), } logger.Info("Prometheus reader initialized", zap.String("addr", cfg.ServerURL)) @@ -225,8 +227,7 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( } promQuery := m.buildPromQuery(p) - tp := jtracer.New().OTEL - ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, tp) + ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, m.tracer.OTEL) defer span.End() queryRange := promapi.Range{ From 6bcab4b364ac84fc766b4391c9b0e53149eabadb Mon Sep 17 00:00:00 2001 From: Afzal <94980910+afzalbin64@users.noreply.github.com> Date: Tue, 11 Jul 2023 22:28:49 +0000 Subject: [PATCH 07/36] transform gRPC OT to OTEL Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com> --- cmd/query/app/grpc_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/query/app/grpc_handler.go b/cmd/query/app/grpc_handler.go index 57827073000..ac428aa8691 100644 --- a/cmd/query/app/grpc_handler.go +++ b/cmd/query/app/grpc_handler.go @@ -73,7 +73,7 @@ func NewGRPCHandler(queryService *querysvc.QueryService, options.Logger = zap.NewNop() } - if options.Tracer.OT == nil { + if options.Tracer.OTEL == nil { options.Tracer = jtracer.NoOp() } From ce29456b51f89a742fe5dda81d14de3c6ea17c1b Mon Sep 17 00:00:00 2001 From: Afzal <94980910+afzalbin64@users.noreply.github.com> Date: Wed, 12 Jul 2023 18:47:51 +0000 Subject: [PATCH 08/36] update http_handler test to support OTEL sdktrace Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com> --- cmd/query/app/http_handler_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 49e77eaf477..81c7f6cae09 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -35,7 +35,9 @@ import ( testHttp "github.com/stretchr/testify/http" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "github.com/uber/jaeger-client-go" + otbridge "go.opentelemetry.io/otel/bridge/opentracing" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -303,10 +305,14 @@ func TestGetTrace(t *testing.T) { for _, tc := range testCases { testCase := tc // capture loop var t.Run(testCase.suffix, func(t *testing.T) { - reporter := jaeger.NewInMemoryReporter() - jaegerTracer, jaegerCloser := jaeger.NewTracer("test", jaeger.NewConstSampler(true), reporter) - jTracer := jtracer.JTracer{OT: jaegerTracer} - defer jaegerCloser.Close() + exporter := tracetest.NewInMemoryExporter() + tracerProvider := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(exporter), + sdktrace.WithSampler(sdktrace.AlwaysSample()), + ) + // Use the bridgeTracer as OpenTracing tracer(otTrace). + otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) + jTracer := jtracer.JTracer{OT: otTracer} ts := initializeTestServer(HandlerOptions.Tracer(jTracer)) defer ts.server.Close() @@ -319,8 +325,8 @@ func TestGetTrace(t *testing.T) { assert.NoError(t, err) assert.Len(t, response.Errors, 0) - assert.Len(t, reporter.GetSpans(), 1, "HTTP request was traced and span reported") - assert.Equal(t, "/api/traces/{traceID}", reporter.GetSpans()[0].(*jaeger.Span).OperationName()) + assert.Len(t, exporter.GetSpans(), 1, "HTTP request was traced and span reported") + assert.Equal(t, "/api/traces/{traceID}", exporter.GetSpans()[0].Name) traces := extractTraces(t, &response) assert.Len(t, traces[0].Spans, 2) From d4298614660e4b87e73e7ae5f0c10953cfa3d877 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 13 Jul 2023 15:15:05 +0000 Subject: [PATCH 09/36] updates jaeger tracer to shutdown in the main call Signed-off-by: Afzal Ansari --- cmd/query/app/handler_options.go | 1 + cmd/query/app/http_handler_test.go | 2 +- pkg/jtracer/jtracer.go | 57 +++++++++++++------ .../metrics/prometheus/metricsstore/reader.go | 3 +- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go index b39db2a3a7a..7a1459711aa 100644 --- a/cmd/query/app/handler_options.go +++ b/cmd/query/app/handler_options.go @@ -65,6 +65,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration) func (handlerOptions) Tracer(tracer jtracer.JTracer) HandlerOption { return func(apiHandler *APIHandler) { apiHandler.tracer.OT = tracer.OT + apiHandler.tracer.OTEL = tracer.OTEL } } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 81c7f6cae09..cee3b4bc800 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -312,7 +312,7 @@ func TestGetTrace(t *testing.T) { ) // Use the bridgeTracer as OpenTracing tracer(otTrace). otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) - jTracer := jtracer.JTracer{OT: otTracer} + jTracer := jtracer.JTracer{OT: otTracer, OTEL: tracerProvider} ts := initializeTestServer(HandlerOptions.Tracer(jTracer)) defer ts.server.Close() diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 31916f179cc..b426b1768cc 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -16,46 +16,54 @@ package jtracer import ( "context" + "fmt" "log" "sync" + "time" "github.com/opentracing/opentracing-go" "go.opentelemetry.io/otel" otbridge "go.opentelemetry.io/otel/bridge/opentracing" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" - "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) type JTracer struct { OT opentracing.Tracer - OTEL trace.TracerProvider + OTEL *sdktrace.TracerProvider + // logger *zap.Logger } var once sync.Once func New() JTracer { opentracingTracer, otelTracerProvider := initBoth() + + // Shutdown the tracerProvider to clean up resources + defer func() { + if err := otelTracerProvider.Shutdown(context.Background()); err != nil { + log.Fatal("Error shutting down tracer provider: %w", err) + } + }() return JTracer{OT: opentracingTracer, OTEL: otelTracerProvider} } func NoOp() JTracer { - return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} + return JTracer{OT: opentracing.NoopTracer{}, OTEL: &sdktrace.TracerProvider{}} } // initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge -func initBoth() (opentracing.Tracer, trace.TracerProvider) { - opts := []otlptracehttp.Option{otlptracehttp.WithInsecure()} - traceExporter, err := otlptrace.New( - context.Background(), - otlptracehttp.NewClient(opts...), - ) +func initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { + ctx := context.Background() + traceExporter, err := newExporter(ctx) if err != nil { - log.Fatal("failed to create exporter: %w", err) + log.Fatal("failed to create exporter", zap.Any("error", err)) } // Register the trace exporter with a TracerProvider, using a batch @@ -78,13 +86,28 @@ func initBoth() (opentracing.Tracer, trace.TracerProvider) { }) // Use the bridgeTracer as your OpenTracing tracer(otTrace). - otTracer, wrapperTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) + otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) - // Shutdown the tracerProvider to clean up resources - err = tracerProvider.Shutdown(context.Background()) + return otTracer, tracerProvider +} + +// newExporter returns a console exporter. +func newExporter(ctx context.Context) (sdktrace.SpanExporter, error) { + ctx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + conn, err := grpc.DialContext(ctx, "otel_collector:4317", + // Note the use of insecure transport here. TLS is recommended in production. + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithBlock(), + ) if err != nil { - log.Fatal("failed to shutdown tracerProvider: ", err) + return nil, fmt.Errorf("failed to create gRPC connection to collector: %w", err) } - return otTracer, wrapperTracerProvider + // Set up a trace exporter + traceExporter, err := otlptracegrpc.New(ctx, otlptracegrpc.WithGRPCConn(conn)) + if err != nil { + return nil, fmt.Errorf("failed to create trace exporter: %w", err) + } + return traceExporter, nil } diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 9ad141e683d..849eb49cb1c 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -29,6 +29,7 @@ import ( "github.com/prometheus/client_golang/api" promapi "github.com/prometheus/client_golang/api/prometheus/v1" "go.opentelemetry.io/otel/attribute" + sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -290,7 +291,7 @@ func promqlDurationString(d *time.Duration) string { return string(b) } -func startSpanForQuery(ctx context.Context, metricName, query string, tp trace.TracerProvider) (context.Context, trace.Span) { +func startSpanForQuery(ctx context.Context, metricName, query string, tp *sdktrace.TracerProvider) (context.Context, trace.Span) { ctx, span := tp.Tracer("prom-metrics-reader").Start(ctx, metricName) span.SetAttributes( attribute.Key(semconv.DBStatementKey).String(query), From d705dad63a3aa7f597bb5135aba1cbbe11d0b727 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 13 Jul 2023 15:30:09 +0000 Subject: [PATCH 10/36] adds gRPC server conn Signed-off-by: Afzal Ansari --- pkg/jtracer/jtracer.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index b426b1768cc..cce32502440 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "log" + "net" "sync" "time" @@ -95,7 +96,15 @@ func initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { func newExporter(ctx context.Context) (sdktrace.SpanExporter, error) { ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - conn, err := grpc.DialContext(ctx, "otel_collector:4317", + server := grpc.NewServer() + + lis, _ := net.Listen("tcp", ":0") + go func() { + err := server.Serve(lis) + log.Fatal("failed to create gRPC server connection: %w", err) + + }() + conn, err := grpc.DialContext(ctx, lis.Addr().String(), // Note the use of insecure transport here. TLS is recommended in production. grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithBlock(), From 29824ce30bb96fff369173400fe083caa28adafe Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 13 Jul 2023 20:31:58 +0000 Subject: [PATCH 11/36] adds logger to jtracer and adjsted the tp.shutdown Signed-off-by: Afzal Ansari --- cmd/query/main.go | 2 + pkg/jtracer/jtracer.go | 72 +++++++++---------- .../metrics/prometheus/metricsstore/reader.go | 1 + 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/cmd/query/main.go b/cmd/query/main.go index bd263ef488d..4631410c978 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -16,6 +16,7 @@ package main import ( + "context" "fmt" "log" "os" @@ -80,6 +81,7 @@ func main() { } jtracer := jtracer.New() + defer jtracer.Close(context.Background()) queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index cce32502440..dead9fe364b 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -18,41 +18,43 @@ import ( "context" "fmt" "log" - "net" "sync" - "time" + "github.com/go-logr/zapr" "github.com/opentracing/opentracing-go" "go.opentelemetry.io/otel" otbridge "go.opentelemetry.io/otel/bridge/opentracing" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" "go.uber.org/zap" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" + "go.uber.org/zap/zapcore" ) type JTracer struct { OT opentracing.Tracer OTEL *sdktrace.TracerProvider - // logger *zap.Logger + log *zap.Logger } var once sync.Once func New() JTracer { - opentracingTracer, otelTracerProvider := initBoth() - - // Shutdown the tracerProvider to clean up resources - defer func() { - if err := otelTracerProvider.Shutdown(context.Background()); err != nil { - log.Fatal("Error shutting down tracer provider: %w", err) - } - }() - return JTracer{OT: opentracingTracer, OTEL: otelTracerProvider} + jt := JTracer{} + zc := zap.NewDevelopmentConfig() + zc.Level = zap.NewAtomicLevelAt(zapcore.Level(-8)) // level used by OTEL's Debug() + logger, err := zc.Build() + if err != nil { + panic(err) + } + otel.SetLogger(zapr.NewLogger(logger)) + + opentracingTracer, otelTracerProvider := jt.initBoth() + + return JTracer{OT: opentracingTracer, OTEL: otelTracerProvider, log: logger} } func NoOp() JTracer { @@ -60,16 +62,15 @@ func NoOp() JTracer { } // initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge -func initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { - ctx := context.Background() - traceExporter, err := newExporter(ctx) +func (jt JTracer) initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { + traceExporter, err := newExporter() if err != nil { - log.Fatal("failed to create exporter", zap.Any("error", err)) + jt.log.Sugar().Fatalf("failed to create exporter", zap.Any("error", err)) } // Register the trace exporter with a TracerProvider, using a batch // span processor to aggregate spans before export. - bsp := sdktrace.NewBatchSpanProcessor(traceExporter) + bsp := sdktrace.NewBatchSpanProcessor(traceExporter, sdktrace.WithBlocking()) tracerProvider := sdktrace.NewTracerProvider( sdktrace.WithSpanProcessor(bsp), sdktrace.WithResource(resource.NewWithAttributes( @@ -93,30 +94,23 @@ func initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { } // newExporter returns a console exporter. -func newExporter(ctx context.Context) (sdktrace.SpanExporter, error) { - ctx, cancel := context.WithTimeout(ctx, time.Second) - defer cancel() - server := grpc.NewServer() - - lis, _ := net.Listen("tcp", ":0") - go func() { - err := server.Serve(lis) - log.Fatal("failed to create gRPC server connection: %w", err) - - }() - conn, err := grpc.DialContext(ctx, lis.Addr().String(), - // Note the use of insecure transport here. TLS is recommended in production. - grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBlock(), +func newExporter() (sdktrace.SpanExporter, error) { + client := otlptracegrpc.NewClient( + otlptracegrpc.WithInsecure(), ) + // Set up a otlp-trace exporter + traceExporter, err := otlptrace.New(context.Background(), client) if err != nil { - return nil, fmt.Errorf("failed to create gRPC connection to collector: %w", err) + return nil, fmt.Errorf("failed to create trace exporter: %w", err) } - // Set up a trace exporter - traceExporter, err := otlptracegrpc.New(ctx, otlptracegrpc.WithGRPCConn(conn)) + return traceExporter, nil +} + +// Shutdown the tracerProvider to clean up resources +func (jt JTracer) Close(ctx context.Context) { + err := jt.OTEL.Shutdown(ctx) if err != nil { - return nil, fmt.Errorf("failed to create trace exporter: %w", err) + log.Fatalf("Error shutting down tracer provider: %v", err) } - return traceExporter, nil } diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 849eb49cb1c..30d86c7b943 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -230,6 +230,7 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, m.tracer.OTEL) defer span.End() + defer m.tracer.Close(ctx) queryRange := promapi.Range{ Start: p.EndTime.Add(-1 * *p.Lookback), From 20188574117e2d6c09cf36a7323ca9f3a369a03e Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 13 Jul 2023 23:08:54 +0000 Subject: [PATCH 12/36] adds Close method to jtracer pkg Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 7 ++- cmd/query/app/handler_options.go | 3 +- cmd/query/main.go | 4 +- pkg/jtracer/jtracer.go | 59 ++++++++++--------- .../metrics/prometheus/metricsstore/reader.go | 7 ++- 5 files changed, 46 insertions(+), 34 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index de157d11b82..16f07bd9404 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -16,6 +16,7 @@ package main import ( + "context" "fmt" "io" "log" @@ -274,7 +275,7 @@ func startQuery( ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - tracer := jtracer.JTracer{OT: opentracing.GlobalTracer()} + tracer := jtracer.New() server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, tracer) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) @@ -287,6 +288,10 @@ func startQuery( if err := server.Start(); err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } + if err = tracer.Close(context.Background()); err != nil { + svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + } + return server } diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go index 7a1459711aa..78d34db8221 100644 --- a/cmd/query/app/handler_options.go +++ b/cmd/query/app/handler_options.go @@ -64,8 +64,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration) // Tracer creates a HandlerOption that initializes OpenTracing tracer func (handlerOptions) Tracer(tracer jtracer.JTracer) HandlerOption { return func(apiHandler *APIHandler) { - apiHandler.tracer.OT = tracer.OT - apiHandler.tracer.OTEL = tracer.OTEL + apiHandler.tracer = tracer } } diff --git a/cmd/query/main.go b/cmd/query/main.go index 4631410c978..69f21e1c7a4 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -81,7 +81,6 @@ func main() { } jtracer := jtracer.New() - defer jtracer.Close(context.Background()) queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) @@ -133,6 +132,9 @@ func main() { logger.Error("Failed to close storage factory", zap.Error(err)) } }) + if err = jtracer.Close(context.Background()); err != nil { + svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + } return nil }, } diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index dead9fe364b..75b8afc86d0 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -17,7 +17,6 @@ package jtracer import ( "context" "fmt" - "log" "sync" "github.com/go-logr/zapr" @@ -30,20 +29,38 @@ import ( "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) type JTracer struct { - OT opentracing.Tracer - OTEL *sdktrace.TracerProvider - log *zap.Logger + OT opentracing.Tracer + OTEL trace.TracerProvider + log *zap.Logger + closer func() error } var once sync.Once func New() JTracer { jt := JTracer{} + opentracingTracer, otelTracerProvider, logger, closed := jt.initBoth() + + return JTracer{ + OT: opentracingTracer, + OTEL: otelTracerProvider, + log: logger, + closer: closed, + } +} + +func NoOp() JTracer { + return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} +} + +// initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge +func (jt JTracer) initBoth() (opentracing.Tracer, trace.TracerProvider, *zap.Logger, func() error) { zc := zap.NewDevelopmentConfig() zc.Level = zap.NewAtomicLevelAt(zapcore.Level(-8)) // level used by OTEL's Debug() logger, err := zc.Build() @@ -52,20 +69,9 @@ func New() JTracer { } otel.SetLogger(zapr.NewLogger(logger)) - opentracingTracer, otelTracerProvider := jt.initBoth() - - return JTracer{OT: opentracingTracer, OTEL: otelTracerProvider, log: logger} -} - -func NoOp() JTracer { - return JTracer{OT: opentracing.NoopTracer{}, OTEL: &sdktrace.TracerProvider{}} -} - -// initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge -func (jt JTracer) initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { - traceExporter, err := newExporter() + traceExporter, err := otelExporter() if err != nil { - jt.log.Sugar().Fatalf("failed to create exporter", zap.Any("error", err)) + logger.Sugar().Fatalf("failed to create exporter", zap.Any("error", err)) } // Register the trace exporter with a TracerProvider, using a batch @@ -88,17 +94,19 @@ func (jt JTracer) initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) { }) // Use the bridgeTracer as your OpenTracing tracer(otTrace). - otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) + otTracer, wrapperTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) - return otTracer, tracerProvider + closer := func() error { + return tracerProvider.Shutdown(context.Background()) + } + + return otTracer, wrapperTracerProvider, logger, closer } -// newExporter returns a console exporter. -func newExporter() (sdktrace.SpanExporter, error) { +func otelExporter() (sdktrace.SpanExporter, error) { client := otlptracegrpc.NewClient( otlptracegrpc.WithInsecure(), ) - // Set up a otlp-trace exporter traceExporter, err := otlptrace.New(context.Background(), client) if err != nil { return nil, fmt.Errorf("failed to create trace exporter: %w", err) @@ -108,9 +116,6 @@ func newExporter() (sdktrace.SpanExporter, error) { } // Shutdown the tracerProvider to clean up resources -func (jt JTracer) Close(ctx context.Context) { - err := jt.OTEL.Shutdown(ctx) - if err != nil { - log.Fatalf("Error shutting down tracer provider: %v", err) - } +func (jt JTracer) Close(ctx context.Context) error { + return jt.closer() } diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index 30d86c7b943..d1493614f88 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -29,7 +29,6 @@ import ( "github.com/prometheus/client_golang/api" promapi "github.com/prometheus/client_golang/api/prometheus/v1" "go.opentelemetry.io/otel/attribute" - sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -230,7 +229,9 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, m.tracer.OTEL) defer span.End() - defer m.tracer.Close(ctx) + if err := m.tracer.Close(context.Background()); err != nil { + m.logger.Error("Error shutting down tracer provider", zap.Error(err)) + } queryRange := promapi.Range{ Start: p.EndTime.Add(-1 * *p.Lookback), @@ -292,7 +293,7 @@ func promqlDurationString(d *time.Duration) string { return string(b) } -func startSpanForQuery(ctx context.Context, metricName, query string, tp *sdktrace.TracerProvider) (context.Context, trace.Span) { +func startSpanForQuery(ctx context.Context, metricName, query string, tp trace.TracerProvider) (context.Context, trace.Span) { ctx, span := tp.Tracer("prom-metrics-reader").Start(ctx, metricName) span.SetAttributes( attribute.Key(semconv.DBStatementKey).String(query), From b54e87283555f342ea09a758f21d465349223be7 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Fri, 14 Jul 2023 06:39:30 +0000 Subject: [PATCH 13/36] enscapulate err field in jtracer Signed-off-by: Afzal Ansari --- cmd/query/main.go | 6 +++--- pkg/jtracer/jtracer.go | 23 ++++++----------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/cmd/query/main.go b/cmd/query/main.go index 69f21e1c7a4..b8935c06f26 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -131,10 +131,10 @@ func main() { if err := storageFactory.Close(); err != nil { logger.Error("Failed to close storage factory", zap.Error(err)) } + if err = jtracer.Close(context.Background()); err != nil { + svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + } }) - if err = jtracer.Close(context.Background()); err != nil { - svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) - } return nil }, } diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 75b8afc86d0..8490b1fe6a3 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -19,7 +19,6 @@ import ( "fmt" "sync" - "github.com/go-logr/zapr" "github.com/opentracing/opentracing-go" "go.opentelemetry.io/otel" otbridge "go.opentelemetry.io/otel/bridge/opentracing" @@ -30,28 +29,26 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" "go.opentelemetry.io/otel/trace" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" ) type JTracer struct { OT opentracing.Tracer OTEL trace.TracerProvider - log *zap.Logger closer func() error + err error } var once sync.Once func New() JTracer { jt := JTracer{} - opentracingTracer, otelTracerProvider, logger, closed := jt.initBoth() + opentracingTracer, otelTracerProvider, closed, error := jt.initBoth() return JTracer{ OT: opentracingTracer, OTEL: otelTracerProvider, - log: logger, closer: closed, + err: error, } } @@ -60,18 +57,10 @@ func NoOp() JTracer { } // initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge -func (jt JTracer) initBoth() (opentracing.Tracer, trace.TracerProvider, *zap.Logger, func() error) { - zc := zap.NewDevelopmentConfig() - zc.Level = zap.NewAtomicLevelAt(zapcore.Level(-8)) // level used by OTEL's Debug() - logger, err := zc.Build() - if err != nil { - panic(err) - } - otel.SetLogger(zapr.NewLogger(logger)) - +func (jt JTracer) initBoth() (opentracing.Tracer, trace.TracerProvider, func() error, error) { traceExporter, err := otelExporter() if err != nil { - logger.Sugar().Fatalf("failed to create exporter", zap.Any("error", err)) + return nil, nil, nil, fmt.Errorf("failed to create exporter: %w", err) } // Register the trace exporter with a TracerProvider, using a batch @@ -100,7 +89,7 @@ func (jt JTracer) initBoth() (opentracing.Tracer, trace.TracerProvider, *zap.Log return tracerProvider.Shutdown(context.Background()) } - return otTracer, wrapperTracerProvider, logger, closer + return otTracer, wrapperTracerProvider, closer, nil } func otelExporter() (sdktrace.SpanExporter, error) { From fd6d707b0c045b8491e55302084ea637e1926757 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Fri, 14 Jul 2023 17:02:10 +0000 Subject: [PATCH 14/36] reform tracer in cmd app Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 12 ++++++------ cmd/query/main.go | 8 -------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 16f07bd9404..d9fddf5bb35 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -195,7 +195,7 @@ by default uses only in-memory database.`, agent := startAgent(cp, aOpts, logger, metricsFactory) // query - querySrv := startQuery( + querySrv, tracer := startQuery( svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger), spanReader, dependencyReader, metricsQueryService, metricsFactory, tm, @@ -214,6 +214,9 @@ by default uses only in-memory database.`, if err := storageFactory.Close(); err != nil { logger.Error("Failed to close storage factory", zap.Error(err)) } + if err = tracer.Close(context.Background()); err != nil { + svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + } _ = tracerCloser.Close() }) return nil @@ -272,7 +275,7 @@ func startQuery( metricsQueryService querysvc.MetricsQueryService, baseFactory metrics.Factory, tm *tenancy.Manager, -) *queryApp.Server { +) (*queryApp.Server, jtracer.JTracer) { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) tracer := jtracer.New() @@ -288,11 +291,8 @@ func startQuery( if err := server.Start(); err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } - if err = tracer.Close(context.Background()); err != nil { - svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) - } - return server + return server, tracer } func initTracer(svc *flags.Service) io.Closer { diff --git a/cmd/query/main.go b/cmd/query/main.go index b8935c06f26..a529e3e5ccc 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -72,14 +72,6 @@ func main() { baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"}) metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) version.NewInfoMetrics(metricsFactory) - - if err != nil { - logger.Fatal("Failed to read tracer configuration", zap.Error(err)) - } - if err != nil { - logger.Fatal("Failed to initialize tracer", zap.Error(err)) - } - jtracer := jtracer.New() queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { From b01349e5c2e784ac6f79384c2ace7792f65007b3 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Fri, 14 Jul 2023 19:14:21 +0000 Subject: [PATCH 15/36] modifies jtracer in cmd/app Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 44 +++++-------------- cmd/query/main.go | 9 ++-- pkg/jtracer/jtracer.go | 22 +++++----- .../metrics/prometheus/metricsstore/reader.go | 9 +++- 4 files changed, 34 insertions(+), 50 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index d9fddf5bb35..4aabf3c2ea4 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -22,11 +22,8 @@ import ( "log" "os" - "github.com/opentracing/opentracing-go" "github.com/spf13/cobra" "github.com/spf13/viper" - jaegerClientConfig "github.com/uber/jaeger-client-go/config" - jaegerClientZapLog "github.com/uber/jaeger-client-go/log/zap" _ "go.uber.org/automaxprocs" "go.uber.org/zap" @@ -44,7 +41,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/status" "github.com/jaegertracing/jaeger/internal/metrics/expvar" "github.com/jaegertracing/jaeger/internal/metrics/fork" - "github.com/jaegertracing/jaeger/internal/metrics/jlibadapter" "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/metrics" @@ -104,7 +100,7 @@ by default uses only in-memory database.`, svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})) version.NewInfoMetrics(metricsFactory) - tracerCloser := initTracer(svc) + tracerCloser := initTracer() storageFactory.InitFromViper(v, logger) if err := storageFactory.Initialize(metricsFactory, logger); err != nil { @@ -195,7 +191,7 @@ by default uses only in-memory database.`, agent := startAgent(cp, aOpts, logger, metricsFactory) // query - querySrv, tracer := startQuery( + querySrv := startQuery( svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger), spanReader, dependencyReader, metricsQueryService, metricsFactory, tm, @@ -214,10 +210,9 @@ by default uses only in-memory database.`, if err := storageFactory.Close(); err != nil { logger.Error("Failed to close storage factory", zap.Error(err)) } - if err = tracer.Close(context.Background()); err != nil { - svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + if tracerCloser != nil { + logger.Fatal("Error shutting down tracer provider", zap.Error(err)) } - _ = tracerCloser.Close() }) return nil }, @@ -275,10 +270,10 @@ func startQuery( metricsQueryService querysvc.MetricsQueryService, baseFactory metrics.Factory, tm *tenancy.Manager, -) (*queryApp.Server, jtracer.JTracer) { +) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - tracer := jtracer.New() + tracer := jtracer.NoOp() server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, tracer) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) @@ -292,32 +287,15 @@ func startQuery( svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } - return server, tracer + return server } -func initTracer(svc *flags.Service) io.Closer { - logger := svc.Logger - traceCfg := &jaegerClientConfig.Configuration{ - ServiceName: "jaeger-query", - Sampler: &jaegerClientConfig.SamplerConfig{ - Type: "const", - Param: 1.0, - }, - RPCMetrics: true, - } - traceCfg, err := traceCfg.FromEnv() - if err != nil { - logger.Fatal("Failed to read tracer configuration", zap.Error(err)) - } - tracer, closer, err := traceCfg.NewTracer( - jaegerClientConfig.Metrics(jlibadapter.NewAdapter(svc.MetricsFactory)), - jaegerClientConfig.Logger(jaegerClientZapLog.NewLogger(logger)), - ) +func initTracer() error { + tracer, err := jtracer.New() if err != nil { - logger.Fatal("Failed to initialize tracer", zap.Error(err)) + log.Fatal("Failed to initialize tracer", zap.Error(err)) } - opentracing.SetGlobalTracer(tracer) - return closer + return tracer.Close(context.Background()) } func createMetricsQueryService( diff --git a/cmd/query/main.go b/cmd/query/main.go index a529e3e5ccc..13e5ef22c99 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -72,7 +72,10 @@ func main() { baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"}) metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) version.NewInfoMetrics(metricsFactory) - jtracer := jtracer.New() + jtracer, err := jtracer.New() + if err != nil { + logger.Fatal("Failed to create exporter:", zap.Error(err)) + } queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { logger.Fatal("Failed to configure query service", zap.Error(err)) @@ -103,7 +106,7 @@ func main() { dependencyReader, *queryServiceOptions) tm := tenancy.NewManager(&queryOpts.Tenancy) - server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, jtracer) + server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, *jtracer) if err != nil { logger.Fatal("Failed to create server", zap.Error(err)) } @@ -124,7 +127,7 @@ func main() { logger.Error("Failed to close storage factory", zap.Error(err)) } if err = jtracer.Close(context.Background()); err != nil { - svc.Logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + logger.Fatal("Error shutting down tracer provider", zap.Error(err)) } }) return nil diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 8490b1fe6a3..088da50edaf 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -35,21 +35,19 @@ type JTracer struct { OT opentracing.Tracer OTEL trace.TracerProvider closer func() error - err error } var once sync.Once -func New() JTracer { +func New() (*JTracer, error) { jt := JTracer{} - opentracingTracer, otelTracerProvider, closed, error := jt.initBoth() + opentracingTracer, otelTracerProvider, closed, error := jt.initBoth(context.Background()) - return JTracer{ + return &JTracer{ OT: opentracingTracer, OTEL: otelTracerProvider, closer: closed, - err: error, - } + }, error } func NoOp() JTracer { @@ -57,10 +55,10 @@ func NoOp() JTracer { } // initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge -func (jt JTracer) initBoth() (opentracing.Tracer, trace.TracerProvider, func() error, error) { - traceExporter, err := otelExporter() +func (jt JTracer) initBoth(ctx context.Context) (opentracing.Tracer, trace.TracerProvider, func() error, error) { + traceExporter, err := otelExporter(ctx) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to create exporter: %w", err) + return nil, nil, nil, err } // Register the trace exporter with a TracerProvider, using a batch @@ -86,17 +84,17 @@ func (jt JTracer) initBoth() (opentracing.Tracer, trace.TracerProvider, func() e otTracer, wrapperTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) closer := func() error { - return tracerProvider.Shutdown(context.Background()) + return tracerProvider.Shutdown(ctx) } return otTracer, wrapperTracerProvider, closer, nil } -func otelExporter() (sdktrace.SpanExporter, error) { +func otelExporter(ctx context.Context) (sdktrace.SpanExporter, error) { client := otlptracegrpc.NewClient( otlptracegrpc.WithInsecure(), ) - traceExporter, err := otlptrace.New(context.Background(), client) + traceExporter, err := otlptrace.New(ctx, client) if err != nil { return nil, fmt.Errorf("failed to create trace exporter: %w", err) } diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index d1493614f88..be433185ce7 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -95,6 +95,11 @@ func NewMetricsReader(logger *zap.Logger, cfg config.Configuration) (*MetricsRea operationLabel = "span_name" } + jtracer, err := jtracer.New() + if err != nil { + logger.Fatal("Failed to create exporter:", zap.Error(err)) + } + mr := &MetricsReader{ client: promapi.NewAPI(client), logger: logger, @@ -103,7 +108,7 @@ func NewMetricsReader(logger *zap.Logger, cfg config.Configuration) (*MetricsRea callsMetricName: buildFullCallsMetricName(cfg), latencyMetricName: buildFullLatencyMetricName(cfg), operationLabel: operationLabel, - tracer: jtracer.New(), + tracer: *jtracer, } logger.Info("Prometheus reader initialized", zap.String("addr", cfg.ServerURL)) @@ -229,7 +234,7 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, m.tracer.OTEL) defer span.End() - if err := m.tracer.Close(context.Background()); err != nil { + if err := m.tracer.Close(ctx); err != nil { m.logger.Error("Error shutting down tracer provider", zap.Error(err)) } From 9447bcfb0b5adc7cd7cddd32279ca77788c6d047 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sat, 15 Jul 2023 08:13:58 +0000 Subject: [PATCH 16/36] moves the tracer to main Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 4aabf3c2ea4..43c38395f30 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -100,7 +100,7 @@ by default uses only in-memory database.`, svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})) version.NewInfoMetrics(metricsFactory) - tracerCloser := initTracer() + tracer := initTracer() storageFactory.InitFromViper(v, logger) if err := storageFactory.Initialize(metricsFactory, logger); err != nil { @@ -194,7 +194,7 @@ by default uses only in-memory database.`, querySrv := startQuery( svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger), spanReader, dependencyReader, metricsQueryService, - metricsFactory, tm, + metricsFactory, tm, tracer, ) svc.RunAndThen(func() { @@ -210,8 +210,8 @@ by default uses only in-memory database.`, if err := storageFactory.Close(); err != nil { logger.Error("Failed to close storage factory", zap.Error(err)) } - if tracerCloser != nil { - logger.Fatal("Error shutting down tracer provider", zap.Error(err)) + if tracer.Close(context.Background()) != nil { + logger.Error("Error shutting down tracer provider", zap.Error(err)) } }) return nil @@ -269,12 +269,11 @@ func startQuery( depReader dependencystore.Reader, metricsQueryService querysvc.MetricsQueryService, baseFactory metrics.Factory, - tm *tenancy.Manager, + tm *tenancy.Manager, jt jtracer.JTracer, ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - tracer := jtracer.NoOp() - server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, tracer) + server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jt) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } @@ -290,12 +289,12 @@ func startQuery( return server } -func initTracer() error { +func initTracer() jtracer.JTracer { tracer, err := jtracer.New() if err != nil { log.Fatal("Failed to initialize tracer", zap.Error(err)) } - return tracer.Close(context.Background()) + return *tracer } func createMetricsQueryService( From 68ec355d79b9cc19a0db1fb89225980ecf6ddf73 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sat, 15 Jul 2023 18:27:16 +0000 Subject: [PATCH 17/36] undo reader file changes Signed-off-by: Afzal Ansari --- .../metrics/prometheus/metricsstore/reader.go | 40 +++++++------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/plugin/metrics/prometheus/metricsstore/reader.go b/plugin/metrics/prometheus/metricsstore/reader.go index be433185ce7..0cf6852183d 100644 --- a/plugin/metrics/prometheus/metricsstore/reader.go +++ b/plugin/metrics/prometheus/metricsstore/reader.go @@ -26,15 +26,14 @@ import ( "time" "unicode" + "github.com/opentracing/opentracing-go" + ottag "github.com/opentracing/opentracing-go/ext" + otlog "github.com/opentracing/opentracing-go/log" "github.com/prometheus/client_golang/api" promapi "github.com/prometheus/client_golang/api/prometheus/v1" - "go.opentelemetry.io/otel/attribute" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" - "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/bearertoken" - "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/prometheus/config" "github.com/jaegertracing/jaeger/plugin/metrics/prometheus/metricsstore/dbmodel" "github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics" @@ -55,7 +54,6 @@ type ( latencyMetricName string callsMetricName string operationLabel string - tracer jtracer.JTracer } promQueryParams struct { @@ -95,11 +93,6 @@ func NewMetricsReader(logger *zap.Logger, cfg config.Configuration) (*MetricsRea operationLabel = "span_name" } - jtracer, err := jtracer.New() - if err != nil { - logger.Fatal("Failed to create exporter:", zap.Error(err)) - } - mr := &MetricsReader{ client: promapi.NewAPI(client), logger: logger, @@ -108,7 +101,6 @@ func NewMetricsReader(logger *zap.Logger, cfg config.Configuration) (*MetricsRea callsMetricName: buildFullCallsMetricName(cfg), latencyMetricName: buildFullLatencyMetricName(cfg), operationLabel: operationLabel, - tracer: *jtracer, } logger.Info("Prometheus reader initialized", zap.String("addr", cfg.ServerURL)) @@ -232,11 +224,8 @@ func (m MetricsReader) executeQuery(ctx context.Context, p metricsQueryParams) ( } promQuery := m.buildPromQuery(p) - ctx, span := startSpanForQuery(ctx, p.metricName, promQuery, m.tracer.OTEL) - defer span.End() - if err := m.tracer.Close(ctx); err != nil { - m.logger.Error("Error shutting down tracer provider", zap.Error(err)) - } + span, ctx := startSpanForQuery(ctx, p.metricName, promQuery) + defer span.Finish() queryRange := promapi.Range{ Start: p.EndTime.Add(-1 * *p.Lookback), @@ -298,18 +287,17 @@ func promqlDurationString(d *time.Duration) string { return string(b) } -func startSpanForQuery(ctx context.Context, metricName, query string, tp trace.TracerProvider) (context.Context, trace.Span) { - ctx, span := tp.Tracer("prom-metrics-reader").Start(ctx, metricName) - span.SetAttributes( - attribute.Key(semconv.DBStatementKey).String(query), - attribute.Key("db.type").String("prometheus"), - attribute.Key("component").String("promql"), - ) - return ctx, span +func startSpanForQuery(ctx context.Context, metricName, query string) (opentracing.Span, context.Context) { + span, ctx := opentracing.StartSpanFromContext(ctx, metricName) + ottag.DBStatement.Set(span, query) + ottag.DBType.Set(span, "prometheus") + ottag.Component.Set(span, "promql") + return span, ctx } -func logErrorToSpan(span trace.Span, err error) { - span.AddEvent(err.Error(), trace.WithAttributes(semconv.OTelStatusCodeError)) +func logErrorToSpan(span opentracing.Span, err error) { + ottag.Error.Set(span, true) + span.LogFields(otlog.Error(err)) } func getHTTPRoundTripper(c *config.Configuration, logger *zap.Logger) (rt http.RoundTripper, err error) { From a2eb44a6ba9ac8a4c6fed3ea1dbdd62fdb4a4186 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Sat, 15 Jul 2023 18:31:56 +0000 Subject: [PATCH 18/36] reformats the jtracer pkg Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 20 ++++++++----------- cmd/query/main.go | 2 +- pkg/jtracer/jtracer.go | 45 ++++++++++++++++++++++-------------------- 3 files changed, 33 insertions(+), 34 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 43c38395f30..75151515b54 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -100,7 +100,10 @@ by default uses only in-memory database.`, svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})) version.NewInfoMetrics(metricsFactory) - tracer := initTracer() + tracer, err := jtracer.New("query-service") + if err != nil { + logger.Fatal("Failed to initialize tracer", zap.Error(err)) + } storageFactory.InitFromViper(v, logger) if err := storageFactory.Initialize(metricsFactory, logger); err != nil { @@ -194,7 +197,7 @@ by default uses only in-memory database.`, querySrv := startQuery( svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger), spanReader, dependencyReader, metricsQueryService, - metricsFactory, tm, tracer, + metricsFactory, tm, *tracer, ) svc.RunAndThen(func() { @@ -210,7 +213,7 @@ by default uses only in-memory database.`, if err := storageFactory.Close(); err != nil { logger.Error("Failed to close storage factory", zap.Error(err)) } - if tracer.Close(context.Background()) != nil { + if err := tracer.Close(context.Background()); err != nil { logger.Error("Error shutting down tracer provider", zap.Error(err)) } }) @@ -269,7 +272,8 @@ func startQuery( depReader dependencystore.Reader, metricsQueryService querysvc.MetricsQueryService, baseFactory metrics.Factory, - tm *tenancy.Manager, jt jtracer.JTracer, + tm *tenancy.Manager, + jt jtracer.JTracer, ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) @@ -289,14 +293,6 @@ func startQuery( return server } -func initTracer() jtracer.JTracer { - tracer, err := jtracer.New() - if err != nil { - log.Fatal("Failed to initialize tracer", zap.Error(err)) - } - return *tracer -} - func createMetricsQueryService( metricsReaderFactory *metricsPlugin.Factory, v *viper.Viper, diff --git a/cmd/query/main.go b/cmd/query/main.go index 13e5ef22c99..7cd0c219009 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -72,7 +72,7 @@ func main() { baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"}) metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"}) version.NewInfoMetrics(metricsFactory) - jtracer, err := jtracer.New() + jtracer, err := jtracer.New("jaeger-query") if err != nil { logger.Fatal("Failed to create exporter:", zap.Error(err)) } diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 088da50edaf..bf49f61e571 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -34,31 +34,41 @@ import ( type JTracer struct { OT opentracing.Tracer OTEL trace.TracerProvider - closer func() error + closer func(ctx context.Context) error } var once sync.Once -func New() (*JTracer, error) { - jt := JTracer{} - opentracingTracer, otelTracerProvider, closed, error := jt.initBoth(context.Background()) +func New(serviceName string) (*JTracer, error) { + ctx := context.Background() + tracerProvider, err := initOTEL(ctx, serviceName) + if err != nil { + return nil, err + } + // Use the bridgeTracer as your OpenTracing tracer(otTrace). + otelTracer := tracerProvider.Tracer("github.com/jaegertracing/jaeger/pkg/jtracer") + otTracer, wrappedTracerProvider := otbridge.NewTracerPair(otelTracer) + + closer := func(ctx context.Context) error { + return tracerProvider.Shutdown(ctx) + } return &JTracer{ - OT: opentracingTracer, - OTEL: otelTracerProvider, - closer: closed, - }, error + OT: otTracer, + OTEL: wrappedTracerProvider, + closer: closer, + }, nil } func NoOp() JTracer { return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} } -// initBoth initializes OpenTelemetry SDK and uses OTel-OpenTracing Bridge -func (jt JTracer) initBoth(ctx context.Context) (opentracing.Tracer, trace.TracerProvider, func() error, error) { +// initOTEL initializes OTEL Tracer +func initOTEL(ctx context.Context, svc string) (*sdktrace.TracerProvider, error) { traceExporter, err := otelExporter(ctx) if err != nil { - return nil, nil, nil, err + return nil, err } // Register the trace exporter with a TracerProvider, using a batch @@ -68,7 +78,7 @@ func (jt JTracer) initBoth(ctx context.Context) (opentracing.Tracer, trace.Trace sdktrace.WithSpanProcessor(bsp), sdktrace.WithResource(resource.NewWithAttributes( semconv.SchemaURL, - semconv.ServiceNameKey.String("otlp"), + semconv.ServiceNameKey.String(svc), )), ) @@ -80,14 +90,7 @@ func (jt JTracer) initBoth(ctx context.Context) (opentracing.Tracer, trace.Trace )) }) - // Use the bridgeTracer as your OpenTracing tracer(otTrace). - otTracer, wrapperTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) - - closer := func() error { - return tracerProvider.Shutdown(ctx) - } - - return otTracer, wrapperTracerProvider, closer, nil + return tracerProvider, nil } func otelExporter(ctx context.Context) (sdktrace.SpanExporter, error) { @@ -104,5 +107,5 @@ func otelExporter(ctx context.Context) (sdktrace.SpanExporter, error) { // Shutdown the tracerProvider to clean up resources func (jt JTracer) Close(ctx context.Context) error { - return jt.closer() + return jt.closer(ctx) } From 9b2be206110a4a8ad1c27929fb038b34b297c3a2 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Mon, 17 Jul 2023 06:52:26 +0000 Subject: [PATCH 19/36] modifies svc name Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 75151515b54..a9c24f0976b 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -100,7 +100,7 @@ by default uses only in-memory database.`, svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})) version.NewInfoMetrics(metricsFactory) - tracer, err := jtracer.New("query-service") + tracer, err := jtracer.New("jaeger-query") if err != nil { logger.Fatal("Failed to initialize tracer", zap.Error(err)) } From 2e037433d8bd8e5a3bdbef77f0f6d8fcd4f91c71 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Mon, 17 Jul 2023 19:04:34 +0000 Subject: [PATCH 20/36] adds middleware for tracer response Signed-off-by: Afzal Ansari --- cmd/query/app/http_handler.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 9714483a4f2..70c5248f889 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -27,7 +27,8 @@ import ( "github.com/gogo/protobuf/proto" "github.com/gorilla/mux" - "github.com/opentracing-contrib/go-stdlib/nethttp" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + "go.opentelemetry.io/otel/propagation" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" @@ -111,7 +112,7 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt if aH.logger == nil { aH.logger = zap.NewNop() } - if aH.tracer.OT == nil { + if aH.tracer.OTEL == nil { aH.tracer = jtracer.NoOp() } return aH @@ -146,12 +147,10 @@ func (aH *APIHandler) handleFunc( if aH.tenancyMgr.Enabled { handler = tenancy.ExtractTenantHTTPHandler(aH.tenancyMgr, handler) } - traceMiddleware := nethttp.Middleware( - aH.tracer.OT, - handler, - nethttp.OperationNameFunc(func(r *http.Request) string { - return route - })) + traceMiddleware := otelhttp.NewHandler( + otelhttp.WithRouteTag(route, traceResponseHandler(handler)), + route, + otelhttp.WithTracerProvider(aH.tracer.OTEL)) return router.HandleFunc(route, traceMiddleware.ServeHTTP) } @@ -523,3 +522,18 @@ func (aH *APIHandler) writeJSON(w http.ResponseWriter, r *http.Request, response aH.handleError(w, fmt.Errorf("failed writing HTTP response: %w", err), http.StatusInternalServerError) } } + +// Returns a handler that generates a traceresponse header. +// https://github.com/w3c/trace-context/blob/main/spec/21-http_response_header_format.md +func traceResponseHandler(handler http.Handler) http.Handler { + // We use the standard TraceContext propagator, since the formats are identical. + // But the propagator uses "traceparent" header name, so we inject it into a map + // `carrier` and then use the result to set the "tracereponse" header. + var prop propagation.TraceContext + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + carrier := make(map[string]string) + prop.Inject(r.Context(), propagation.MapCarrier(carrier)) + w.Header().Add("traceresponse", carrier["traceparent"]) + handler.ServeHTTP(w, r) + }) +} From 056b81dc8cf6fef33b5c32993ff291b5862da42b Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Mon, 17 Jul 2023 19:05:04 +0000 Subject: [PATCH 21/36] adds trace-id url Signed-off-by: Afzal Ansari --- cmd/all-in-one/all_in_one_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index deb89a04e25..b4336acb511 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -23,6 +23,7 @@ import ( "encoding/json" "io" "net/http" + "strings" "testing" "time" @@ -46,12 +47,13 @@ const ( agentURL = "http://" + agentHostPort getServicesURL = queryURL + "/api/services" - getTraceURL = queryURL + "/api/traces?service=jaeger-query&tag=jaeger-debug-id:debug" getSamplingStrategyURL = agentURL + "/sampling?service=whatever" getServicesAPIV3URL = queryURL + "/api/v3/services" ) +var getTraceURL = queryURL + "/api/traces/" + var httpClient = &http.Client{ Timeout: time.Second, } @@ -70,10 +72,15 @@ func TestAllInOne(t *testing.T) { func createTrace(t *testing.T) { req, err := http.NewRequest(http.MethodGet, getServicesURL, nil) require.NoError(t, err) - req.Header.Add("jaeger-debug-id", "debug") resp, err := httpClient.Do(req) require.NoError(t, err) + traceResponse := resp.Header.Get("traceresponse") + parts := strings.Split(traceResponse, "-") + traceID := parts[1] + req.Header.Set("trace-id", traceID) + // Update the URL with the traceID + getTraceURL += traceID resp.Body.Close() } @@ -121,7 +128,7 @@ func getSamplingStrategy(t *testing.T) { resp.Body.Close() assert.NotNil(t, queryResponse.ProbabilisticSampling) - assert.EqualValues(t, 1.0, queryResponse.ProbabilisticSampling.SamplingRate) + assert.EqualValues(t, 0.001, queryResponse.ProbabilisticSampling.SamplingRate) } func healthCheck(t *testing.T) { From 7e157ebc923b3d1fd4e9d1c414d3b43f4ef7072a Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Mon, 17 Jul 2023 19:39:14 +0000 Subject: [PATCH 22/36] reverts prob value Signed-off-by: Afzal Ansari --- cmd/all-in-one/all_in_one_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index b4336acb511..6d1675f56fa 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -76,9 +76,7 @@ func createTrace(t *testing.T) { resp, err := httpClient.Do(req) require.NoError(t, err) traceResponse := resp.Header.Get("traceresponse") - parts := strings.Split(traceResponse, "-") - traceID := parts[1] - req.Header.Set("trace-id", traceID) + traceID := strings.Split(traceResponse, "-")[1] // Update the URL with the traceID getTraceURL += traceID resp.Body.Close() @@ -128,7 +126,7 @@ func getSamplingStrategy(t *testing.T) { resp.Body.Close() assert.NotNil(t, queryResponse.ProbabilisticSampling) - assert.EqualValues(t, 0.001, queryResponse.ProbabilisticSampling.SamplingRate) + assert.EqualValues(t, 1, queryResponse.ProbabilisticSampling.SamplingRate) } func healthCheck(t *testing.T) { From 8e96af8c9f99b50f918d8400259fcfede93a0868 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Tue, 18 Jul 2023 05:26:20 +0000 Subject: [PATCH 23/36] minor modification around pointers and versions Signed-off-by: Afzal Ansari --- cmd/all-in-one/all_in_one_test.go | 2 +- cmd/all-in-one/main.go | 6 +++--- cmd/query/app/http_handler_test.go | 4 ++-- pkg/jtracer/jtracer.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index 6d1675f56fa..c71914837b2 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -126,7 +126,7 @@ func getSamplingStrategy(t *testing.T) { resp.Body.Close() assert.NotNil(t, queryResponse.ProbabilisticSampling) - assert.EqualValues(t, 1, queryResponse.ProbabilisticSampling.SamplingRate) + assert.EqualValues(t, 1.0, queryResponse.ProbabilisticSampling.SamplingRate) } func healthCheck(t *testing.T) { diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index a9c24f0976b..64a584edce6 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -197,7 +197,7 @@ by default uses only in-memory database.`, querySrv := startQuery( svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger), spanReader, dependencyReader, metricsQueryService, - metricsFactory, tm, *tracer, + metricsFactory, tm, tracer, ) svc.RunAndThen(func() { @@ -273,11 +273,11 @@ func startQuery( metricsQueryService querysvc.MetricsQueryService, baseFactory metrics.Factory, tm *tenancy.Manager, - jt jtracer.JTracer, + jt *jtracer.JTracer, ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jt) + server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, *jt) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index cee3b4bc800..e1d2f091bea 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -311,8 +311,8 @@ func TestGetTrace(t *testing.T) { sdktrace.WithSampler(sdktrace.AlwaysSample()), ) // Use the bridgeTracer as OpenTracing tracer(otTrace). - otTracer, _ := otbridge.NewTracerPair(tracerProvider.Tracer("")) - jTracer := jtracer.JTracer{OT: otTracer, OTEL: tracerProvider} + otTracer, wrappedTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) + jTracer := jtracer.JTracer{OT: otTracer, OTEL: wrappedTracerProvider} ts := initializeTestServer(HandlerOptions.Tracer(jTracer)) defer ts.server.Close() diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index bf49f61e571..55a5e84ab24 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -27,7 +27,7 @@ import ( "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" ) @@ -106,6 +106,6 @@ func otelExporter(ctx context.Context) (sdktrace.SpanExporter, error) { } // Shutdown the tracerProvider to clean up resources -func (jt JTracer) Close(ctx context.Context) error { +func (jt *JTracer) Close(ctx context.Context) error { return jt.closer(ctx) } From 88d5f2565cc5a1e5a6c18ea1b40c8154db625827 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Wed, 19 Jul 2023 19:40:06 +0000 Subject: [PATCH 24/36] pass OTEL tracer into the tracer method Signed-off-by: Afzal Ansari --- cmd/query/app/handler_options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go index 78d34db8221..e4555fe6380 100644 --- a/cmd/query/app/handler_options.go +++ b/cmd/query/app/handler_options.go @@ -64,7 +64,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration) // Tracer creates a HandlerOption that initializes OpenTracing tracer func (handlerOptions) Tracer(tracer jtracer.JTracer) HandlerOption { return func(apiHandler *APIHandler) { - apiHandler.tracer = tracer + apiHandler.tracer.OTEL = tracer.OTEL } } From 55918b434cf6ab8800363539126d1c0d3949b459 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Wed, 19 Jul 2023 20:03:24 +0000 Subject: [PATCH 25/36] excludes pkg from testing Signed-off-by: Afzal Ansari --- pkg/jtracer/testutils/.nocover | 1 + 1 file changed, 1 insertion(+) create mode 100644 pkg/jtracer/testutils/.nocover diff --git a/pkg/jtracer/testutils/.nocover b/pkg/jtracer/testutils/.nocover new file mode 100644 index 00000000000..3c8a0c31ed2 --- /dev/null +++ b/pkg/jtracer/testutils/.nocover @@ -0,0 +1 @@ +requires connection to OTLP receiver From f26e1b2415bb122197e26df05c0480132458728f Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 01:48:13 +0000 Subject: [PATCH 26/36] rmvs jtracer_test file Signed-off-by: Afzal Ansari --- pkg/jtracer/jtracer_test.go | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 pkg/jtracer/jtracer_test.go diff --git a/pkg/jtracer/jtracer_test.go b/pkg/jtracer/jtracer_test.go deleted file mode 100644 index e2c2d657223..00000000000 --- a/pkg/jtracer/jtracer_test.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) 2023 The Jaeger Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package jtracer_test - -// TODO From 0eba87ab7ced4453864d871376c9fb018ad92cfe Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 01:56:25 +0000 Subject: [PATCH 27/36] modifies .nocover Signed-off-by: Afzal Ansari --- pkg/jtracer/testutils/.nocover | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jtracer/testutils/.nocover b/pkg/jtracer/testutils/.nocover index 3c8a0c31ed2..9d6cf4b7fb6 100644 --- a/pkg/jtracer/testutils/.nocover +++ b/pkg/jtracer/testutils/.nocover @@ -1 +1 @@ -requires connection to OTLP receiver +FIXME From 3600c09c0a7a890eaf1cb28a109e44d51481653e Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 01:56:47 +0000 Subject: [PATCH 28/36] makes minor modification around *jtracer pointer Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 2 +- cmd/query/app/grpc_handler.go | 2 +- cmd/query/app/grpc_handler_test.go | 2 +- cmd/query/app/handler_options.go | 4 ++-- cmd/query/app/http_handler.go | 2 +- cmd/query/app/http_handler_test.go | 3 ++- cmd/query/app/server.go | 10 +++++----- cmd/query/main.go | 4 ++-- pkg/jtracer/empty_test.go | 17 +++++++++++++++++ pkg/jtracer/jtracer.go | 6 +++--- 10 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 pkg/jtracer/empty_test.go diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 64a584edce6..6917849bc6d 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -277,7 +277,7 @@ func startQuery( ) *queryApp.Server { spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"})) qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) - server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, *jt) + server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jt) if err != nil { svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) } diff --git a/cmd/query/app/grpc_handler.go b/cmd/query/app/grpc_handler.go index ac428aa8691..558f4d8c6fe 100644 --- a/cmd/query/app/grpc_handler.go +++ b/cmd/query/app/grpc_handler.go @@ -74,7 +74,7 @@ func NewGRPCHandler(queryService *querysvc.QueryService, } if options.Tracer.OTEL == nil { - options.Tracer = jtracer.NoOp() + options.Tracer = *jtracer.NoOp() } if options.NowFn == nil { diff --git a/cmd/query/app/grpc_handler_test.go b/cmd/query/app/grpc_handler_test.go index a82ebe94afd..8f303d93d98 100644 --- a/cmd/query/app/grpc_handler_test.go +++ b/cmd/query/app/grpc_handler_test.go @@ -927,7 +927,7 @@ func initializeTenantedTestServerGRPCWithOptions(t *testing.T, tm *tenancy.Manag logger := zap.NewNop() tracer := jtracer.NoOp() - server, addr := newGRPCServer(t, q, tqs.metricsQueryService, logger, tracer, tm) + server, addr := newGRPCServer(t, q, tqs.metricsQueryService, logger, *tracer, tm) return &grpcServer{ server: server, diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go index e4555fe6380..8f9ef33b3ca 100644 --- a/cmd/query/app/handler_options.go +++ b/cmd/query/app/handler_options.go @@ -62,9 +62,9 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration) } // Tracer creates a HandlerOption that initializes OpenTracing tracer -func (handlerOptions) Tracer(tracer jtracer.JTracer) HandlerOption { +func (handlerOptions) Tracer(tracer *jtracer.JTracer) HandlerOption { return func(apiHandler *APIHandler) { - apiHandler.tracer.OTEL = tracer.OTEL + apiHandler.tracer = *tracer } } diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 70c5248f889..2a9f331fd3b 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -113,7 +113,7 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt aH.logger = zap.NewNop() } if aH.tracer.OTEL == nil { - aH.tracer = jtracer.NoOp() + aH.tracer = *jtracer.NoOp() } return aH } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index e1d2f091bea..8ea3e77a31a 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -313,8 +313,9 @@ func TestGetTrace(t *testing.T) { // Use the bridgeTracer as OpenTracing tracer(otTrace). otTracer, wrappedTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) jTracer := jtracer.JTracer{OT: otTracer, OTEL: wrappedTracerProvider} + defer tracerProvider.Shutdown(context.Background()) - ts := initializeTestServer(HandlerOptions.Tracer(jTracer)) + ts := initializeTestServer(HandlerOptions.Tracer(&jTracer)) defer ts.server.Close() ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), model.NewTraceID(0, 0x123456abc)). diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 2c9e834f324..f15f32f9467 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -51,7 +51,7 @@ type Server struct { querySvc *querysvc.QueryService queryOptions *QueryOptions - tracer jtracer.JTracer // TODO make part of flags.Service + tracer *jtracer.JTracer // TODO make part of flags.Service conn net.Listener grpcConn net.Listener @@ -65,7 +65,7 @@ type Server struct { } // NewServer creates and initializes Server -func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, tracer jtracer.JTracer) (*Server, error) { +func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, tracer *jtracer.JTracer) (*Server, error) { _, httpPort, err := net.SplitHostPort(options.HTTPHostPort) if err != nil { return nil, err @@ -107,7 +107,7 @@ func (s Server) HealthCheckStatus() chan healthcheck.Status { return s.unavailableChannel } -func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, logger *zap.Logger, tracer jtracer.JTracer) (*grpc.Server, error) { +func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, logger *zap.Logger, tracer *jtracer.JTracer) (*grpc.Server, error) { var grpcOpts []grpc.ServerOption if options.TLSGRPC.Enabled { @@ -132,7 +132,7 @@ func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc. handler := NewGRPCHandler(querySvc, metricsQuerySvc, GRPCHandlerOptions{ Logger: logger, - Tracer: tracer, + Tracer: *tracer, }) healthServer := health.NewServer() @@ -148,7 +148,7 @@ func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc. return server, nil } -func createHTTPServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, queryOpts *QueryOptions, tm *tenancy.Manager, tracer jtracer.JTracer, logger *zap.Logger) (*http.Server, context.CancelFunc, error) { +func createHTTPServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, queryOpts *QueryOptions, tm *tenancy.Manager, tracer *jtracer.JTracer, logger *zap.Logger) (*http.Server, context.CancelFunc, error) { apiHandlerOptions := []HandlerOption{ HandlerOptions.Logger(logger), HandlerOptions.Tracer(tracer), diff --git a/cmd/query/main.go b/cmd/query/main.go index 7cd0c219009..aadb7915787 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -74,7 +74,7 @@ func main() { version.NewInfoMetrics(metricsFactory) jtracer, err := jtracer.New("jaeger-query") if err != nil { - logger.Fatal("Failed to create exporter:", zap.Error(err)) + logger.Fatal("Failed to create tracer:", zap.Error(err)) } queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger) if err != nil { @@ -106,7 +106,7 @@ func main() { dependencyReader, *queryServiceOptions) tm := tenancy.NewManager(&queryOpts.Tenancy) - server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, *jtracer) + server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, jtracer) if err != nil { logger.Fatal("Failed to create server", zap.Error(err)) } diff --git a/pkg/jtracer/empty_test.go b/pkg/jtracer/empty_test.go new file mode 100644 index 00000000000..e2c2d657223 --- /dev/null +++ b/pkg/jtracer/empty_test.go @@ -0,0 +1,17 @@ +// Copyright (c) 2023 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package jtracer_test + +// TODO diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 55a5e84ab24..18ddecd6209 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -60,8 +60,8 @@ func New(serviceName string) (*JTracer, error) { }, nil } -func NoOp() JTracer { - return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} +func NoOp() *JTracer { + return &JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()} } // initOTEL initializes OTEL Tracer @@ -73,7 +73,7 @@ func initOTEL(ctx context.Context, svc string) (*sdktrace.TracerProvider, error) // Register the trace exporter with a TracerProvider, using a batch // span processor to aggregate spans before export. - bsp := sdktrace.NewBatchSpanProcessor(traceExporter, sdktrace.WithBlocking()) + bsp := sdktrace.NewBatchSpanProcessor(traceExporter) tracerProvider := sdktrace.NewTracerProvider( sdktrace.WithSpanProcessor(bsp), sdktrace.WithResource(resource.NewWithAttributes( From 7f81f627c515ae5d644775f4226aaf4531537ae2 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 02:34:21 +0000 Subject: [PATCH 29/36] updates pointer receivers for jtracer Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 2 +- cmd/query/app/grpc_handler.go | 8 ++++---- cmd/query/app/grpc_handler_test.go | 4 ++-- cmd/query/app/handler_options.go | 2 +- cmd/query/app/http_handler.go | 6 +++--- cmd/query/app/http_handler_test.go | 1 - cmd/query/app/server.go | 2 +- pkg/jtracer/{testutils => }/.nocover | 0 8 files changed, 12 insertions(+), 13 deletions(-) rename pkg/jtracer/{testutils => }/.nocover (100%) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 6917849bc6d..97c8ca83256 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -100,7 +100,7 @@ by default uses only in-memory database.`, svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})) version.NewInfoMetrics(metricsFactory) - tracer, err := jtracer.New("jaeger-query") + tracer, err := jtracer.New("jaeger-all-in-one") if err != nil { logger.Fatal("Failed to initialize tracer", zap.Error(err)) } diff --git a/cmd/query/app/grpc_handler.go b/cmd/query/app/grpc_handler.go index 558f4d8c6fe..39f656df723 100644 --- a/cmd/query/app/grpc_handler.go +++ b/cmd/query/app/grpc_handler.go @@ -60,7 +60,7 @@ type GRPCHandler struct { // GRPCHandlerOptions contains optional members of GRPCHandler. type GRPCHandlerOptions struct { Logger *zap.Logger - Tracer jtracer.JTracer + Tracer *jtracer.JTracer NowFn func() time.Time } @@ -73,8 +73,8 @@ func NewGRPCHandler(queryService *querysvc.QueryService, options.Logger = zap.NewNop() } - if options.Tracer.OTEL == nil { - options.Tracer = *jtracer.NoOp() + if options.Tracer == nil { + options.Tracer = jtracer.NoOp() } if options.NowFn == nil { @@ -85,7 +85,7 @@ func NewGRPCHandler(queryService *querysvc.QueryService, queryService: queryService, metricsQueryService: metricsQueryService, logger: options.Logger, - tracer: options.Tracer, + tracer: *options.Tracer, nowFn: options.NowFn, } } diff --git a/cmd/query/app/grpc_handler_test.go b/cmd/query/app/grpc_handler_test.go index 8f303d93d98..19c02502efa 100644 --- a/cmd/query/app/grpc_handler_test.go +++ b/cmd/query/app/grpc_handler_test.go @@ -145,7 +145,7 @@ type grpcClient struct { conn *grpc.ClientConn } -func newGRPCServer(t *testing.T, q *querysvc.QueryService, mq querysvc.MetricsQueryService, logger *zap.Logger, tracer jtracer.JTracer, tenancyMgr *tenancy.Manager) (*grpc.Server, net.Addr) { +func newGRPCServer(t *testing.T, q *querysvc.QueryService, mq querysvc.MetricsQueryService, logger *zap.Logger, tracer *jtracer.JTracer, tenancyMgr *tenancy.Manager) (*grpc.Server, net.Addr) { lis, _ := net.Listen("tcp", ":0") var grpcOpts []grpc.ServerOption if tenancyMgr.Enabled { @@ -927,7 +927,7 @@ func initializeTenantedTestServerGRPCWithOptions(t *testing.T, tm *tenancy.Manag logger := zap.NewNop() tracer := jtracer.NoOp() - server, addr := newGRPCServer(t, q, tqs.metricsQueryService, logger, *tracer, tm) + server, addr := newGRPCServer(t, q, tqs.metricsQueryService, logger, tracer, tm) return &grpcServer{ server: server, diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go index 8f9ef33b3ca..1b87fd6c524 100644 --- a/cmd/query/app/handler_options.go +++ b/cmd/query/app/handler_options.go @@ -64,7 +64,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration) // Tracer creates a HandlerOption that initializes OpenTracing tracer func (handlerOptions) Tracer(tracer *jtracer.JTracer) HandlerOption { return func(apiHandler *APIHandler) { - apiHandler.tracer = *tracer + apiHandler.tracer = tracer } } diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 2a9f331fd3b..d3c10e4344f 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -89,7 +89,7 @@ type APIHandler struct { basePath string apiPrefix string logger *zap.Logger - tracer jtracer.JTracer + tracer *jtracer.JTracer } // NewAPIHandler returns an APIHandler @@ -112,8 +112,8 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt if aH.logger == nil { aH.logger = zap.NewNop() } - if aH.tracer.OTEL == nil { - aH.tracer = *jtracer.NoOp() + if aH.tracer == nil { + aH.tracer = jtracer.NoOp() } return aH } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 8ea3e77a31a..fa268b79ca1 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -310,7 +310,6 @@ func TestGetTrace(t *testing.T) { sdktrace.WithSyncer(exporter), sdktrace.WithSampler(sdktrace.AlwaysSample()), ) - // Use the bridgeTracer as OpenTracing tracer(otTrace). otTracer, wrappedTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) jTracer := jtracer.JTracer{OT: otTracer, OTEL: wrappedTracerProvider} defer tracerProvider.Shutdown(context.Background()) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index f15f32f9467..d12f1770cbb 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -132,7 +132,7 @@ func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc. handler := NewGRPCHandler(querySvc, metricsQuerySvc, GRPCHandlerOptions{ Logger: logger, - Tracer: *tracer, + Tracer: tracer, }) healthServer := health.NewServer() diff --git a/pkg/jtracer/testutils/.nocover b/pkg/jtracer/.nocover similarity index 100% rename from pkg/jtracer/testutils/.nocover rename to pkg/jtracer/.nocover From eab67351545c9f57374197f5cfaa2eeaa60c8a14 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 04:51:46 +0000 Subject: [PATCH 30/36] rmv pointer dereferrencing Signed-off-by: Afzal Ansari --- cmd/query/app/grpc_handler.go | 4 ++-- pkg/jtracer/empty_test.go | 17 ----------------- 2 files changed, 2 insertions(+), 19 deletions(-) delete mode 100644 pkg/jtracer/empty_test.go diff --git a/cmd/query/app/grpc_handler.go b/cmd/query/app/grpc_handler.go index 39f656df723..84e18506d6b 100644 --- a/cmd/query/app/grpc_handler.go +++ b/cmd/query/app/grpc_handler.go @@ -53,7 +53,7 @@ type GRPCHandler struct { queryService *querysvc.QueryService metricsQueryService querysvc.MetricsQueryService logger *zap.Logger - tracer jtracer.JTracer + tracer *jtracer.JTracer nowFn func() time.Time } @@ -85,7 +85,7 @@ func NewGRPCHandler(queryService *querysvc.QueryService, queryService: queryService, metricsQueryService: metricsQueryService, logger: options.Logger, - tracer: *options.Tracer, + tracer: options.Tracer, nowFn: options.NowFn, } } diff --git a/pkg/jtracer/empty_test.go b/pkg/jtracer/empty_test.go deleted file mode 100644 index e2c2d657223..00000000000 --- a/pkg/jtracer/empty_test.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) 2023 The Jaeger Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package jtracer_test - -// TODO From e4194172e475ce1a41cf09aa65205e8d00b4789a Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 04:52:19 +0000 Subject: [PATCH 31/36] replces with jaeger-all-in-one Signed-off-by: Afzal Ansari --- cmd/all-in-one/all_in_one_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index c71914837b2..c0971265a79 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -162,5 +162,5 @@ func getServicesAPIV3(t *testing.T) { jsonpb := runtime.JSONPb{} err = jsonpb.Unmarshal(body, &servicesResponse) require.NoError(t, err) - assert.Equal(t, []string{"jaeger-query"}, servicesResponse.GetServices()) + assert.Equal(t, []string{"jaeger-all-in-one"}, servicesResponse.GetServices()) } From 22fb80aa14f58864fd8af84f6d050acf415ddd1a Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 04:55:45 +0000 Subject: [PATCH 32/36] updates the log err Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 97c8ca83256..de571c06a50 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -279,7 +279,7 @@ func startQuery( qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jt) if err != nil { - svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) + svc.Logger.Fatal("Could not start jaeger-all-in-one service", zap.Error(err)) } go func() { for s := range server.HealthCheckStatus() { @@ -287,7 +287,7 @@ func startQuery( } }() if err := server.Start(); err != nil { - svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err)) + svc.Logger.Fatal("Could not start jaeger-all-in-one service", zap.Error(err)) } return server From 43e5032987db241f45691541a91eb7787bb4125b Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 05:40:52 +0000 Subject: [PATCH 33/36] updates logger to log err jaeger-query Signed-off-by: Afzal Ansari --- cmd/all-in-one/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index de571c06a50..34d86f534ce 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -279,7 +279,7 @@ func startQuery( qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts) server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, tm, jt) if err != nil { - svc.Logger.Fatal("Could not start jaeger-all-in-one service", zap.Error(err)) + svc.Logger.Fatal("Could not create jaeger-query", zap.Error(err)) } go func() { for s := range server.HealthCheckStatus() { @@ -287,7 +287,7 @@ func startQuery( } }() if err := server.Start(); err != nil { - svc.Logger.Fatal("Could not start jaeger-all-in-one service", zap.Error(err)) + svc.Logger.Fatal("Could not start jaeger-query", zap.Error(err)) } return server From c64e90c9e800af2b78c05d34ee0bb1939f404ae8 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 10:30:27 +0000 Subject: [PATCH 34/36] simplifies the traceRes obj Signed-off-by: Afzal Ansari --- cmd/all-in-one/all_in_one_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index c0971265a79..d26f61ef8cf 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -76,8 +76,9 @@ func createTrace(t *testing.T) { resp, err := httpClient.Do(req) require.NoError(t, err) traceResponse := resp.Header.Get("traceresponse") - traceID := strings.Split(traceResponse, "-")[1] - // Update the URL with the traceID + parts := strings.Split(traceResponse, "-") + require.Len(t, parts, 4) // [version] [trace-id] [child-id] [trace-flags] + traceID := parts[1] getTraceURL += traceID resp.Body.Close() } From c2960e3e29487d72d3fd103d0c0494f72f6dc26f Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 18:12:25 +0000 Subject: [PATCH 35/36] adds defer to resp from Do() Signed-off-by: Afzal Ansari --- cmd/all-in-one/all_in_one_test.go | 2 +- pkg/jtracer/jtracer.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index d26f61ef8cf..6bb20c12560 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -75,12 +75,12 @@ func createTrace(t *testing.T) { resp, err := httpClient.Do(req) require.NoError(t, err) + defer resp.Body.Close() traceResponse := resp.Header.Get("traceresponse") parts := strings.Split(traceResponse, "-") require.Len(t, parts, 4) // [version] [trace-id] [child-id] [trace-flags] traceID := parts[1] getTraceURL += traceID - resp.Body.Close() } type response struct { diff --git a/pkg/jtracer/jtracer.go b/pkg/jtracer/jtracer.go index 18ddecd6209..32e9c56c241 100644 --- a/pkg/jtracer/jtracer.go +++ b/pkg/jtracer/jtracer.go @@ -99,7 +99,7 @@ func otelExporter(ctx context.Context) (sdktrace.SpanExporter, error) { ) traceExporter, err := otlptrace.New(ctx, client) if err != nil { - return nil, fmt.Errorf("failed to create trace exporter: %w", err) + return nil, fmt.Errorf("Failed to create trace exporter: %w", err) } return traceExporter, nil From f905480687934f1474e4b2d09b558d4e3712690d Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Thu, 20 Jul 2023 18:22:30 +0000 Subject: [PATCH 36/36] rmvs OT tracer Signed-off-by: Afzal Ansari --- cmd/query/app/http_handler_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index fa268b79ca1..6cbfa41c3da 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -35,7 +35,6 @@ import ( testHttp "github.com/stretchr/testify/http" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - otbridge "go.opentelemetry.io/otel/bridge/opentracing" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.uber.org/zap" @@ -310,8 +309,7 @@ func TestGetTrace(t *testing.T) { sdktrace.WithSyncer(exporter), sdktrace.WithSampler(sdktrace.AlwaysSample()), ) - otTracer, wrappedTracerProvider := otbridge.NewTracerPair(tracerProvider.Tracer("")) - jTracer := jtracer.JTracer{OT: otTracer, OTEL: wrappedTracerProvider} + jTracer := jtracer.JTracer{OTEL: tracerProvider} defer tracerProvider.Shutdown(context.Background()) ts := initializeTestServer(HandlerOptions.Tracer(&jTracer))