Skip to content

Commit

Permalink
adds Close method to jtracer pkg
Browse files Browse the repository at this point in the history
Signed-off-by: Afzal Ansari <[email protected]>
  • Loading branch information
afzal442 committed Jul 13, 2023
1 parent 29824ce commit 2018857
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 34 deletions.
7 changes: 6 additions & 1 deletion cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package main

import (
"context"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -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))
Expand All @@ -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
}

Expand Down
3 changes: 1 addition & 2 deletions cmd/query/app/handler_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
4 changes: 3 additions & 1 deletion cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
},
}
Expand Down
59 changes: 32 additions & 27 deletions pkg/jtracer/jtracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package jtracer
import (
"context"
"fmt"
"log"
"sync"

"github.com/go-logr/zapr"
Expand All @@ -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,
}

Check warning on line 55 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L46-L55

Added lines #L46 - L55 were not covered by tests
}

func NoOp() JTracer {
return JTracer{OT: opentracing.NoopTracer{}, OTEL: trace.NewNoopTracerProvider()}

Check warning on line 59 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L59

Added line #L59 was not covered by tests
}

// 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()
Expand All @@ -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))
}

Check warning on line 75 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L70-L75

Added lines #L70 - L75 were not covered by tests

// Register the trace exporter with a TracerProvider, using a batch
Expand All @@ -88,17 +94,19 @@ func (jt JTracer) initBoth() (opentracing.Tracer, *sdktrace.TracerProvider) {
})

Check warning on line 94 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L79-L94

Added lines #L79 - L94 were not covered by tests

// 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())
}

Check warning on line 101 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L97-L101

Added lines #L97 - L101 were not covered by tests

return otTracer, wrapperTracerProvider, logger, closer

Check warning on line 103 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L103

Added line #L103 was not covered by tests
}

// 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)
Expand All @@ -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()

Check warning on line 120 in pkg/jtracer/jtracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/jtracer/jtracer.go#L119-L120

Added lines #L119 - L120 were not covered by tests
}
7 changes: 4 additions & 3 deletions plugin/metrics/prometheus/metricsstore/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {

Check failure on line 232 in plugin/metrics/prometheus/metricsstore/reader.go

View workflow job for this annotation

GitHub Actions / unit-tests

Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
m.logger.Error("Error shutting down tracer provider", zap.Error(err))
}

Check warning on line 234 in plugin/metrics/prometheus/metricsstore/reader.go

View check run for this annotation

Codecov / codecov/patch

plugin/metrics/prometheus/metricsstore/reader.go#L233-L234

Added lines #L233 - L234 were not covered by tests

queryRange := promapi.Range{
Start: p.EndTime.Add(-1 * *p.Lookback),
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit 2018857

Please sign in to comment.