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
Show file tree
Hide file tree
Changes from 31 commits
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
18 changes: 12 additions & 6 deletions cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/jaegertracing/jaeger/pkg/config"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/pkg/telemetery"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/pkg/version"
metricsPlugin "github.com/jaegertracing/jaeger/plugin/metrics"
Expand Down Expand Up @@ -193,12 +194,17 @@ by default uses only in-memory database.`,
logger.Fatal("Could not create collector proxy", zap.Error(err))
}
agent := startAgent(cp, aOpts, logger, agentMetricsFactory)

telset := telemetery.Setting{
Logger: svc.Logger,
TracerProvider: tracer.OTEL,
Metrics: queryMetricsFactory,
ReportStatus: telemetery.HCAdapter(svc.HC()),
}
// query
querySrv := startQuery(
svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger),
spanReader, dependencyReader, metricsQueryService,
queryMetricsFactory, tm, tracer,
tm, telset,
)

svc.RunAndThen(func() {
Expand Down Expand Up @@ -273,13 +279,13 @@ func startQuery(
spanReader spanstore.Reader,
depReader dependencystore.Reader,
metricsQueryService querysvc.MetricsQueryService,
metricsFactory metrics.Factory,
tm *tenancy.Manager,
jt *jtracer.JTracer,
telset telemetery.Setting,
) *queryApp.Server {
spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, metricsFactory)
spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, telset.Metrics)
qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts)
server, err := queryApp.NewServer(svc.Logger, svc.HC(), qs, metricsQueryService, qOpts, tm, jt)

server, err := queryApp.NewServer(qs, metricsQueryService, qOpts, tm, telset)
if err != nil {
svc.Logger.Fatal("Could not create jaeger-query", zap.Error(err))
}
Expand Down
35 changes: 19 additions & 16 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ import (

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

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
queryApp "github.com/jaegertracing/jaeger/cmd/query/app"
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/pkg/healthcheck"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/telemetery"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/ports"
Expand All @@ -28,16 +27,16 @@ var (
)

type server struct {
config *Config
logger *zap.Logger
server *queryApp.Server
jtracer *jtracer.JTracer
config *Config
server *queryApp.Server
telset component.TelemetrySettings
closeTracer func(ctx context.Context) error
}

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

Expand Down Expand Up @@ -75,22 +74,26 @@ func (s *server) Start(_ context.Context, host component.Host) error {
// TODO OTel-collector does not initialize the tracer currently
// https://github.com/open-telemetry/opentelemetry-collector/issues/7532
//nolint
s.jtracer, err = jtracer.New("jaeger")
tracerProvider, err := jtracer.New("jaeger")
if err != nil {
Wise-Wizard marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("could not initialize a tracer: %w", err)
}
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
s.closeTracer = tracerProvider.Close
telset := telemetery.Setting{
Logger: s.telset.Logger,
TracerProvider: tracerProvider.OTEL,
ReportStatus: s.telset.ReportStatus,
}

// TODO contextcheck linter complains about next line that context is not passed. It is not wrong.
//nolint
s.server, err = queryApp.NewServer(
s.logger,
// TODO propagate healthcheck updates up to the collector's runtime
healthcheck.New(),
qs,
metricsQueryService,
s.makeQueryOptions(),
tm,
s.jtracer,
telset,
)
if err != nil {
return fmt.Errorf("could not create jaeger-query: %w", err)
Expand All @@ -105,7 +108,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
}

Expand All @@ -114,8 +117,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
}
Expand All @@ -135,8 +138,8 @@ func (s *server) Shutdown(ctx context.Context) error {
if s.server != nil {
errs = append(errs, s.server.Close())
}
if s.jtracer != nil {
Wise-Wizard marked this conversation as resolved.
Show resolved Hide resolved
errs = append(errs, s.jtracer.Close(ctx))
if s.server.TracerProvider != nil {
Wise-Wizard marked this conversation as resolved.
Show resolved Hide resolved
errs = append(errs, s.closeTracer(ctx))
}
return errors.Join(errs...)
}
6 changes: 3 additions & 3 deletions cmd/query/app/apiv3/http_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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.
Expand All @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/apiv3/http_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
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"

"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
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 4 additions & 3 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -90,7 +91,7 @@ type APIHandler struct {
basePath string
apiPrefix string
logger *zap.Logger
tracer *jtracer.JTracer
tracer trace.TracerProvider
}

// NewAPIHandler returns an APIHandler
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand Down
Loading
Loading