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

Implement Telemetery struct for V1 Components Initialization #5695

Merged
merged 35 commits into from
Jul 9, 2024
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
51487a4
Draft Changes to Query using telset
Wise-Wizard Jul 2, 2024
59aa43a
Changed all-in-one code
Wise-Wizard Jul 2, 2024
59111ab
Update pkg/telemetery/settings.go
Wise-Wizard Jul 3, 2024
5ba546b
Made sugessted Changes
Wise-Wizard Jul 3, 2024
1894931
Merge branch 'main' into OTEL/Telemetery
Wise-Wizard Jul 3, 2024
1cfca71
Fix error
Wise-Wizard Jul 3, 2024
5a63b73
Update pkg/telemetery/settings.go
Wise-Wizard Jul 3, 2024
bba59f8
Merge branch 'main' into OTEL/Telemetery
Wise-Wizard Jul 3, 2024
18fdc2d
Refactored telset init in all-in-one
Wise-Wizard Jul 3, 2024
0c933b0
Merge branch 'main' into OTEL/Telemetery
Wise-Wizard Jul 3, 2024
c769b44
Created Healthcheck Adapter
Wise-Wizard Jul 3, 2024
e132adb
Updated ReportStatus to use OTEL's StatusEvent
Wise-Wizard Jul 4, 2024
69876c7
Prototype Impl of Tracer
Wise-Wizard Jul 4, 2024
3ec9594
Added suggested Changes
Wise-Wizard Jul 4, 2024
0002dc6
Changed V2 to accept OTEL's Telset
Wise-Wizard Jul 4, 2024
32d921c
Refactored to accept trace.TracerProvider
Wise-Wizard Jul 5, 2024
57602b5
Merge branch 'main' into OTEL/Telemetery
Wise-Wizard Jul 5, 2024
e5fa271
Temporary fix to failing CI
Wise-Wizard Jul 5, 2024
2396ca5
Made suggested changes
Wise-Wizard Jul 5, 2024
2abe801
Reduced number of ReportStatus calls
Wise-Wizard Jul 5, 2024
ca9950c
Update cmd/query/app/server.go
Wise-Wizard Jul 5, 2024
44638d9
Merge branch 'main' into OTEL/Telemetery
Wise-Wizard Jul 5, 2024
a297feb
Cleaned up COde
Wise-Wizard Jul 5, 2024
6680ea3
Pass telset to struct
Wise-Wizard Jul 5, 2024
bc958c2
Made suggested changes
Wise-Wizard Jul 5, 2024
8654a32
Worked on Test
Wise-Wizard Jul 5, 2024
db18701
Cleaned up Test File
Wise-Wizard Jul 6, 2024
45a1ba3
Fix Linter Errors
Wise-Wizard Jul 6, 2024
65ecbd3
Fix goleak
Wise-Wizard Jul 6, 2024
74403ee
Added Tests
Wise-Wizard Jul 7, 2024
3d9ba04
Ran make lint
Wise-Wizard Jul 7, 2024
160663e
Update cmd/jaeger/internal/extension/jaegerquery/server.go
Wise-Wizard Jul 9, 2024
3de30a1
Merge branch 'main' into OTEL/Telemetery
Wise-Wizard Jul 9, 2024
70d4b10
Fixed Error
Wise-Wizard Jul 9, 2024
25f8d10
Fix nil pointer error
Wise-Wizard Jul 9, 2024
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
Prev Previous commit
Next Next commit
Refactored to accept trace.TracerProvider
Signed-off-by: Wise-Wizard <[email protected]>
Wise-Wizard committed Jul 5, 2024
commit 32d921c48c501b278b32e7620ab5a518a65caed1
40 changes: 11 additions & 29 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
@@ -5,13 +5,10 @@ package jaegerquery

import (
"context"
"errors"
"fmt"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/extension"
"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
queryApp "github.com/jaegertracing/jaeger/cmd/query/app"
@@ -29,18 +26,14 @@ var (

type server struct {
config *Config
logger *zap.Logger
server *queryApp.Server
tracer trace.TracerProvider
hcFunc func(*component.StatusEvent)
telset component.TelemetrySettings
}

func newServer(config *Config, otel component.TelemetrySettings) *server {
return &server{
config: config,
logger: otel.Logger,
tracer: otel.TracerProvider,
hcFunc: otel.ReportStatus,
telset: otel,
}
}

@@ -79,14 +72,10 @@ func (s *server) Start(_ context.Context, host component.Host) error {
// https://github.com/open-telemetry/opentelemetry-collector/issues/7532
//nolint

if err != nil {
return fmt.Errorf("could not initialize a tracer: %w", err)
}

telset := telemetery.Setting{
Logger: s.logger,
TracerProvider: s.tracer,
ReportStatus: s.hcFunc,
internalTelset := telemetery.Setting{
Logger: s.telset.Logger,
TracerProvider: s.telset.TracerProvider,
ReportStatus: s.telset.ReportStatus,
}
// TODO contextcheck linter complains about next line that context is not passed. It is not wrong.
//nolint
@@ -96,7 +85,7 @@ func (s *server) Start(_ context.Context, host component.Host) error {
metricsQueryService,
s.makeQueryOptions(),
tm,
telset,
internalTelset,
)
if err != nil {
return fmt.Errorf("could not create jaeger-query: %w", err)
@@ -111,7 +100,7 @@ func (s *server) Start(_ context.Context, host component.Host) error {

func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host component.Host) error {
if s.config.TraceStorageArchive == "" {
s.logger.Info("Archive storage not configured")
s.telset.Logger.Info("Archive storage not configured")
return nil
}

@@ -120,8 +109,8 @@ func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host comp
return fmt.Errorf("cannot find archive storage factory: %w", err)
}

if !opts.InitArchiveStorage(f, s.logger) {
s.logger.Info("Archive storage not initialized")
if !opts.InitArchiveStorage(f, s.telset.Logger) {
s.telset.Logger.Info("Archive storage not initialized")
}
return nil
}
@@ -137,12 +126,5 @@ func (s *server) makeQueryOptions() *queryApp.QueryOptions {
}

func (s *server) Shutdown(ctx context.Context) error {
var errs []error
if s.server != nil {
errs = append(errs, s.server.Close())
}
// if s.tracer != nil {
// errs = append(errs, s.tracer.Shutdown(ctx))
// }
return errors.Join(errs...)
return s.server.Close()
}
6 changes: 3 additions & 3 deletions cmd/query/app/apiv3/http_gateway.go
Original file line number Diff line number Diff line change
@@ -17,12 +17,12 @@ import (
"github.com/gorilla/mux"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/internal/api_v3"
"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/storage/spanstore"
)
@@ -48,7 +48,7 @@ type HTTPGateway struct {
QueryService *querysvc.QueryService
TenancyMgr *tenancy.Manager
Logger *zap.Logger
Tracer *jtracer.JTracer
Tracer trace.TracerProvider
}

// RegisterRoutes registers HTTP endpoints for APIv3 into provided mux.
@@ -75,7 +75,7 @@ func (h *HTTPGateway) addRoute(
traceMiddleware := otelhttp.NewHandler(
otelhttp.WithRouteTag(route, handler),
route,
otelhttp.WithTracerProvider(h.Tracer.OTEL))
otelhttp.WithTracerProvider(h.Tracer))
return router.HandleFunc(route, traceMiddleware.ServeHTTP)
}

2 changes: 1 addition & 1 deletion cmd/query/app/apiv3/http_gateway_test.go
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@ func setupHTTPGatewayNoServer(
QueryService: q,
TenancyMgr: tenancy.NewManager(&tenancyOptions),
Logger: zap.NewNop(),
Tracer: jtracer.NoOp(),
Tracer: jtracer.NoOp().OTEL,
}

gw.router = &mux.Router{}
4 changes: 2 additions & 2 deletions cmd/query/app/handler_options.go
Original file line number Diff line number Diff line change
@@ -18,10 +18,10 @@ package app
import (
"time"

"go.opentelemetry.io/otel/trace"
"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
@@ -62,7 +62,7 @@ func (handlerOptions) QueryLookbackDuration(queryLookbackDuration time.Duration)
}

// Tracer creates a HandlerOption that passes the tracer to the handler
func (handlerOptions) Tracer(tracer *jtracer.JTracer) HandlerOption {
func (handlerOptions) Tracer(tracer trace.TracerProvider) HandlerOption {
return func(apiHandler *APIHandler) {
apiHandler.tracer = tracer
}
7 changes: 4 additions & 3 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@ import (
"github.com/gorilla/mux"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
@@ -90,7 +91,7 @@ type APIHandler struct {
basePath string
apiPrefix string
logger *zap.Logger
tracer *jtracer.JTracer
tracer trace.TracerProvider
}

// NewAPIHandler returns an APIHandler
@@ -114,7 +115,7 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt
aH.logger = zap.NewNop()
}
if aH.tracer == nil {
aH.tracer = jtracer.NoOp()
aH.tracer = jtracer.NoOp().OTEL
}
return aH
}
@@ -151,7 +152,7 @@ func (aH *APIHandler) handleFunc(
traceMiddleware := otelhttp.NewHandler(
otelhttp.WithRouteTag(route, traceResponseHandler(handler)),
route,
otelhttp.WithTracerProvider(aH.tracer.OTEL))
otelhttp.WithTracerProvider(aH.tracer))
return router.HandleFunc(route, traceMiddleware.ServeHTTP)
}

2 changes: 1 addition & 1 deletion cmd/query/app/http_handler_test.go
Original file line number Diff line number Diff line change
@@ -321,7 +321,7 @@ func TestGetTrace(t *testing.T) {
jTracer := jtracer.JTracer{OTEL: tracerProvider}
defer tracerProvider.Shutdown(context.Background())

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

ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), model.NewTraceID(0, 0x123456abc)).
33 changes: 15 additions & 18 deletions cmd/query/app/server.go
Original file line number Diff line number Diff line change
@@ -27,7 +27,6 @@ import (
"github.com/gorilla/handlers"
"github.com/soheilhy/cmux"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"google.golang.org/grpc"
@@ -84,12 +83,12 @@ func NewServer(querySvc *querysvc.QueryService,
return nil, errors.New("server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead")
}

grpcServer, err := createGRPCServer(querySvc, metricsQuerySvc, options, tm, telset.Logger, telset.TracerProvider)
grpcServer, err := createGRPCServer(querySvc, metricsQuerySvc, options, tm, telset)
if err != nil {
return nil, err
}

httpServer, err := createHTTPServer(querySvc, metricsQuerySvc, options, tm, telset.TracerProvider, telset.Logger)
httpServer, err := createHTTPServer(querySvc, metricsQuerySvc, options, tm, telset)
if err != nil {
return nil, err
}
@@ -104,11 +103,11 @@ func NewServer(querySvc *querysvc.QueryService,
}, nil
}

func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, logger *zap.Logger, tracer trace.TracerProvider) (*grpc.Server, error) {
func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, telset telemetery.Setting) (*grpc.Server, error) {
var grpcOpts []grpc.ServerOption

if options.TLSGRPC.Enabled {
tlsCfg, err := options.TLSGRPC.Config(logger)
tlsCfg, err := options.TLSGRPC.Config(telset.Logger)
if err != nil {
return nil, err
}
@@ -128,8 +127,7 @@ func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.
reflection.Register(server)

handler := NewGRPCHandler(querySvc, metricsQuerySvc, GRPCHandlerOptions{
Logger: logger,
Tracer: tracer,
Logger: telset.Logger,
})
healthServer := health.NewServer()

@@ -157,12 +155,11 @@ func createHTTPServer(
metricsQuerySvc querysvc.MetricsQueryService,
queryOpts *QueryOptions,
tm *tenancy.Manager,
tracer trace.TracerProvider,
logger *zap.Logger,
telset telemetery.Setting,
) (*httpServer, error) {
apiHandlerOptions := []HandlerOption{
HandlerOptions.Logger(logger),
HandlerOptions.Tracer(tracer),
HandlerOptions.Logger(telset.Logger),
HandlerOptions.Tracer(telset.TracerProvider),
HandlerOptions.MetricsQueryService(metricsQuerySvc),
}

@@ -178,20 +175,20 @@ func createHTTPServer(
(&apiv3.HTTPGateway{
QueryService: querySvc,
TenancyMgr: tm,
Logger: logger,
Tracer: tracer,
Logger: telset.Logger,
Tracer: telset.TracerProvider,
}).RegisterRoutes(r)

apiHandler.RegisterRoutes(r)
var handler http.Handler = r
handler = additionalHeadersHandler(handler, queryOpts.AdditionalHeaders)
if queryOpts.BearerTokenPropagation {
handler = bearertoken.PropagationHandler(logger, handler)
handler = bearertoken.PropagationHandler(telset.Logger, handler)
}
handler = handlers.CompressHandler(handler)
recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true)
recoveryHandler := recoveryhandler.NewRecoveryHandler(telset.Logger, true)

errorLog, _ := zap.NewStdLogAt(logger, zapcore.ErrorLevel)
errorLog, _ := zap.NewStdLogAt(telset.Logger, zapcore.ErrorLevel)
server := &httpServer{
Server: &http.Server{
Handler: recoveryHandler(handler),
@@ -201,14 +198,14 @@ func createHTTPServer(
}

if queryOpts.TLSHTTP.Enabled {
tlsCfg, err := queryOpts.TLSHTTP.Config(logger) // This checks if the certificates are correctly provided
tlsCfg, err := queryOpts.TLSHTTP.Config(telset.Logger) // This checks if the certificates are correctly provided
if err != nil {
return nil, err
}
server.TLSConfig = tlsCfg
}

server.staticHandlerCloser = RegisterStaticHandler(r, logger, queryOpts, querySvc.GetCapabilities())
server.staticHandlerCloser = RegisterStaticHandler(r, telset.Logger, queryOpts, querySvc.GetCapabilities())

return server, nil
}