Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the enable tracing opt-in flag #4685

Merged
merged 2 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/spf13/cobra"
"github.com/spf13/viper"
"go.opentelemetry.io/otel"
_ "go.uber.org/automaxprocs"
"go.uber.org/zap"

Expand Down Expand Up @@ -105,7 +104,6 @@ by default uses only in-memory database.`,
if err != nil {
logger.Fatal("Failed to initialize tracer", zap.Error(err))
}
otel.SetTracerProvider(tracer.OTEL)

storageFactory.InitFromViper(v, logger)
if err := storageFactory.Initialize(metricsFactory, logger); err != nil {
Expand Down
5 changes: 5 additions & 0 deletions cmd/query/app/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const (
queryTokenPropagation = "query.bearer-token-propagation"
queryAdditionalHeaders = "query.additional-headers"
queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment"
queryEnableTracing = "query.enable-tracing"
)

var tlsGRPCFlagsConfig = tlscfg.ServerFlagsConfig{
Expand Down Expand Up @@ -85,6 +86,8 @@ type QueryOptions struct {
MaxClockSkewAdjust time.Duration
// Tenancy configures tenancy for query
Tenancy tenancy.Options
// EnableTracing determines whether traces will be emitted by jaeger-query.
EnableTracing bool
}

// AddFlags adds flags for QueryOptions
Expand All @@ -98,6 +101,7 @@ func AddFlags(flagSet *flag.FlagSet) {
flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format")
flagSet.Bool(queryTokenPropagation, false, "Allow propagation of bearer token to be used by storage plugins")
flagSet.Duration(queryMaxClockSkewAdjust, 0, "The maximum delta by which span timestamps may be adjusted in the UI due to clock skew; set to 0s to disable clock skew adjustments")
flagSet.Bool(queryEnableTracing, false, "Enables emitting jaeger-query traces")
tlsGRPCFlagsConfig.AddFlags(flagSet)
tlsHTTPFlagsConfig.AddFlags(flagSet)
}
Expand Down Expand Up @@ -131,6 +135,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
qOpts.AdditionalHeaders = headers
}
qOpts.Tenancy = tenancy.InitFromViper(v)
qOpts.EnableTracing = v.GetBool(queryEnableTracing)
return qOpts, nil
}

Expand Down
20 changes: 12 additions & 8 deletions cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/spf13/cobra"
"github.com/spf13/viper"
"go.opentelemetry.io/otel"
_ "go.uber.org/automaxprocs"
"go.uber.org/zap"

Expand Down Expand Up @@ -73,15 +72,20 @@ func main() {
baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"})
version.NewInfoMetrics(metricsFactory)
jtracer, err := jtracer.New("jaeger-query")
if err != nil {
logger.Fatal("Failed to create tracer:", zap.Error(err))
}
otel.SetTracerProvider(jtracer.OTEL)

queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger)
if err != nil {
logger.Fatal("Failed to configure query service", zap.Error(err))
}

jt := jtracer.NoOp()
if queryOpts.EnableTracing {
jt, err = jtracer.New("jaeger-query")
if err != nil {
logger.Fatal("Failed to create tracer", zap.Error(err))
}
}

// TODO: Need to figure out set enable/disable propagation on storage plugins.
v.Set(bearertoken.StoragePropagationKey, queryOpts.BearerTokenPropagation)
storageFactory.InitFromViper(v, logger)
Expand All @@ -108,7 +112,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, jt)
Comment on lines -111 to +115
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to jt to avoid clashing names with the jtracer package.

if err != nil {
logger.Fatal("Failed to create server", zap.Error(err))
}
Expand All @@ -128,7 +132,7 @@ 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 {
if err = jt.Close(context.Background()); err != nil {
logger.Fatal("Error shutting down tracer provider", zap.Error(err))
}
})
Expand Down
2 changes: 0 additions & 2 deletions examples/memstore-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/hashicorp/go-plugin"
"github.com/spf13/viper"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"go.opentelemetry.io/otel"
googleGRPC "google.golang.org/grpc"

"github.com/jaegertracing/jaeger/pkg/jtracer"
Expand Down Expand Up @@ -62,7 +61,6 @@ func main() {
panic(fmt.Errorf("failed to initialize tracer: %w", err))
}
defer tracer.Close(context.Background())
otel.SetTracerProvider(tracer.OTEL)

memStorePlugin := grpcMemory.NewStoragePlugin(memory.NewStore(), memory.NewStore())
service := &shared.PluginServices{
Expand Down
2 changes: 2 additions & 0 deletions pkg/jtracer/jtracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ func initOTEL(ctx context.Context, svc string) (*sdktrace.TracerProvider, error)
))
})

otel.SetTracerProvider(tracerProvider)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved here so we just set the global tracer in one place, and removes the otel dependencies from the respective files that use them.

I didn't see, or could think of, a use case where we wouldn't want to set the global tracer when instantiating a jtracer.

Let me know if that's not the case.


return tracerProvider, nil
}

Expand Down