Skip to content

Commit

Permalink
Refactor opentracing.Tracer interface
Browse files Browse the repository at this point in the history
Signed-off-by: afzal442 <[email protected]>
  • Loading branch information
sbin64 authored and afzal442 committed Jun 14, 2023
1 parent c582d89 commit 68fbd4b
Show file tree
Hide file tree
Showing 12 changed files with 103 additions and 36 deletions.
4 changes: 3 additions & 1 deletion cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import (
"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"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/pkg/version"
Expand Down Expand Up @@ -273,7 +274,8 @@ 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, opentracing.GlobalTracer())
jtracer := jtracer.OT(opentracing.GlobalTracer())
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))
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/query/app/grpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/model"
_ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
Expand All @@ -53,14 +54,14 @@ type GRPCHandler struct {
queryService *querysvc.QueryService
metricsQueryService querysvc.MetricsQueryService
logger *zap.Logger
tracer opentracing.Tracer
tracer jtracer.JTracer
nowFn func() time.Time
}

// GRPCHandlerOptions contains optional members of GRPCHandler.
type GRPCHandlerOptions struct {
Logger *zap.Logger
Tracer opentracing.Tracer
Tracer jtracer.JTracer
NowFn func() time.Time
}

Expand All @@ -73,8 +74,8 @@ func NewGRPCHandler(queryService *querysvc.QueryService,
options.Logger = zap.NewNop()
}

if options.Tracer == nil {
options.Tracer = opentracing.NoopTracer{}
if options.Tracer.OT == nil {
options.Tracer = jtracer.OT(opentracing.NoopTracer{})
}

if options.NowFn == nil {
Expand Down
5 changes: 3 additions & 2 deletions cmd/query/app/grpc_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
Expand Down Expand Up @@ -145,7 +146,7 @@ type grpcClient struct {
conn *grpc.ClientConn
}

func newGRPCServer(t *testing.T, q *querysvc.QueryService, mq querysvc.MetricsQueryService, logger *zap.Logger, tracer opentracing.Tracer, 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 {
Expand Down Expand Up @@ -925,7 +926,7 @@ func initializeTenantedTestServerGRPCWithOptions(t *testing.T, tm *tenancy.Manag
}

logger := zap.NewNop()
tracer := opentracing.NoopTracer{}
tracer := jtracer.OT(opentracing.NoopTracer{})

server, addr := newGRPCServer(t, q, tqs.metricsQueryService, logger, tracer, tm)

Expand Down
4 changes: 2 additions & 2 deletions cmd/query/app/handler_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ package app
import (
"time"

"github.com/opentracing/opentracing-go"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/pkg/jtracer"
)

// HandlerOption is a function that sets some option on the APIHandler
Expand Down Expand Up @@ -62,7 +62,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration)
}

// Tracer creates a HandlerOption that initializes OpenTracing tracer
func (handlerOptions) Tracer(tracer opentracing.Tracer) HandlerOption {
func (handlerOptions) Tracer(tracer jtracer.JTracer) HandlerOption {
return func(apiHandler *APIHandler) {
apiHandler.tracer = tracer
}
Expand Down
10 changes: 5 additions & 5 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ import (
"github.com/gogo/protobuf/proto"
"github.com/gorilla/mux"
"github.com/opentracing-contrib/go-stdlib/nethttp"
"github.com/opentracing/opentracing-go"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/model"
uiconv "github.com/jaegertracing/jaeger/model/converter/json"
ui "github.com/jaegertracing/jaeger/model/json"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
Expand Down Expand Up @@ -88,7 +88,7 @@ type APIHandler struct {
basePath string
apiPrefix string
logger *zap.Logger
tracer opentracing.Tracer
tracer jtracer.JTracer
}

// NewAPIHandler returns an APIHandler
Expand All @@ -111,8 +111,8 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt
if aH.logger == nil {
aH.logger = zap.NewNop()
}
if aH.tracer == nil {
aH.tracer = opentracing.NoopTracer{}
if aH.tracer.OT == nil {
aH.tracer = jtracer.NoOp()
}
return aH
}
Expand Down Expand Up @@ -147,7 +147,7 @@ func (aH *APIHandler) handleFunc(
handler = tenancy.ExtractTenantHTTPHandler(aH.tenancyMgr, handler)
}
traceMiddleware := nethttp.Middleware(
aH.tracer,
aH.tracer.OT,
handler,
nethttp.OperationNameFunc(func(r *http.Request) string {
return route
Expand Down
4 changes: 3 additions & 1 deletion cmd/query/app/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/model/adjuster"
ui "github.com/jaegertracing/jaeger/model/json"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
Expand Down Expand Up @@ -304,9 +305,10 @@ 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)
defer jaegerCloser.Close()

ts := initializeTestServer(HandlerOptions.Tracer(jaegerTracer))
ts := initializeTestServer(HandlerOptions.Tracer(jTracer))
defer ts.server.Close()

ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), model.NewTraceID(0, 0x123456abc)).
Expand Down
10 changes: 5 additions & 5 deletions cmd/query/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"time"

"github.com/gorilla/handlers"
"github.com/opentracing/opentracing-go"
"github.com/soheilhy/cmux"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
Expand All @@ -37,6 +36,7 @@ import (
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/pkg/bearertoken"
"github.com/jaegertracing/jaeger/pkg/healthcheck"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/netutils"
"github.com/jaegertracing/jaeger/pkg/recoveryhandler"
"github.com/jaegertracing/jaeger/pkg/tenancy"
Expand All @@ -51,7 +51,7 @@ type Server struct {
querySvc *querysvc.QueryService
queryOptions *QueryOptions

tracer opentracing.Tracer // TODO make part of flags.Service
tracer jtracer.JTracer // TODO make part of flags.Service

conn net.Listener
grpcConn net.Listener
Expand All @@ -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 opentracing.Tracer) (*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
Expand Down Expand Up @@ -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 opentracing.Tracer) (*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 {
Expand Down Expand Up @@ -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 opentracing.Tracer, 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),
Expand Down
26 changes: 13 additions & 13 deletions cmd/query/app/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"testing"
"time"

"github.com/opentracing/opentracing-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand All @@ -40,6 +39,7 @@ import (
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/healthcheck"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/ports"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
Expand Down Expand Up @@ -69,7 +69,7 @@ func TestCreateTLSServerSinglePortError(t *testing.T) {

_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
&QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg},
tenancy.NewManager(&tenancy.Options{}), opentracing.NoopTracer{})
tenancy.NewManager(&tenancy.Options{}), jtracer.NoOp())
assert.NotNil(t, err)
}

Expand All @@ -83,7 +83,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) {

_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
&QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg},
tenancy.NewManager(&tenancy.Options{}), opentracing.NoopTracer{})
tenancy.NewManager(&tenancy.Options{}), jtracer.NoOp())
assert.NotNil(t, err)
}

Expand All @@ -97,7 +97,7 @@ func TestCreateTLSHttpServerError(t *testing.T) {

_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
&QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg},
tenancy.NewManager(&tenancy.Options{}), opentracing.NoopTracer{})
tenancy.NewManager(&tenancy.Options{}), jtracer.NoOp())
assert.NotNil(t, err)
}

Expand Down Expand Up @@ -340,7 +340,7 @@ func TestServerHTTPTLS(t *testing.T) {
querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{})
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
serverOptions, tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{})
jtracer.NoOp())
assert.Nil(t, err)
assert.NoError(t, server.Start())

Expand Down Expand Up @@ -500,7 +500,7 @@ func TestServerGRPCTLS(t *testing.T) {
querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{})
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
serverOptions, tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{})
jtracer.NoOp())
assert.Nil(t, err)
assert.NoError(t, server.Start())

Expand Down Expand Up @@ -555,13 +555,13 @@ func TestServerBadHostPort(t *testing.T) {
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
&QueryOptions{HTTPHostPort: "8080", GRPCHostPort: "127.0.0.1:8081", BearerTokenPropagation: true},
tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{})
jtracer.NoOp())

assert.NotNil(t, err)
_, err = NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
&QueryOptions{HTTPHostPort: "127.0.0.1:8081", GRPCHostPort: "9123", BearerTokenPropagation: true},
tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{})
jtracer.NoOp())

assert.NotNil(t, err)
}
Expand Down Expand Up @@ -592,7 +592,7 @@ func TestServerInUseHostPort(t *testing.T) {
BearerTokenPropagation: true,
},
tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{},
jtracer.NoOp(),
)
assert.NoError(t, err)

Expand Down Expand Up @@ -622,7 +622,7 @@ func TestServerSinglePort(t *testing.T) {
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
&QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort, BearerTokenPropagation: true},
tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{})
jtracer.NoOp())
assert.Nil(t, err)
assert.NoError(t, server.Start())

Expand Down Expand Up @@ -668,7 +668,7 @@ func TestServerGracefulExit(t *testing.T) {
hostPort := ports.PortToHostPort(ports.QueryAdminHTTP)

querySvc := &querysvc.QueryService{}
tracer := opentracing.NoopTracer{}
tracer := jtracer.NoOp()

server, err := NewServer(flagsSvc.Logger, querySvc, nil, &QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort},
tenancy.NewManager(&tenancy.Options{}), tracer)
Expand Down Expand Up @@ -697,7 +697,7 @@ func TestServerHandlesPortZero(t *testing.T) {
flagsSvc.Logger = zap.New(zapCore)

querySvc := &querysvc.QueryService{}
tracer := opentracing.NoopTracer{}
tracer := jtracer.NoOp()
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
&QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"},
tenancy.NewManager(&tenancy.Options{}),
Expand Down Expand Up @@ -759,7 +759,7 @@ func TestServerHTTPTenancy(t *testing.T) {
querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{})
server, err := NewServer(zap.NewNop(), querySvc, nil,
serverOptions, tenancyMgr,
opentracing.NoopTracer{})
jtracer.NoOp())
require.Nil(t, err)
require.NoError(t, server.Start())

Expand Down
3 changes: 1 addition & 2 deletions cmd/query/app/token_propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"testing"

"github.com/olivere/elastic"
"github.com/opentracing/opentracing-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"
Expand Down Expand Up @@ -93,7 +92,7 @@ func runQueryService(t *testing.T, esURL string) *Server {
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
&QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0", BearerTokenPropagation: true},
tenancy.NewManager(&tenancy.Options{}),
opentracing.NoopTracer{},
noopTracer,
)
require.NoError(t, err)
require.NoError(t, server.Start())
Expand Down
4 changes: 3 additions & 1 deletion cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"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"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/pkg/version"
Expand Down Expand Up @@ -96,6 +97,7 @@ func main() {
}
defer closer.Close()
opentracing.SetGlobalTracer(tracer)
jtracer := jtracer.OT(tracer)
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 @@ -126,7 +128,7 @@ func main() {
dependencyReader,
*queryServiceOptions)
tm := tenancy.NewManager(&queryOpts.Tenancy)
server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, tracer)
server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, jtracer)
if err != nil {
logger.Fatal("Failed to create server", zap.Error(err))
}
Expand Down
Loading

0 comments on commit 68fbd4b

Please sign in to comment.