Skip to content

Commit

Permalink
Implement Telemetery struct for V1 Components Initialization (#5695)
Browse files Browse the repository at this point in the history
**Which problem is this PR solving?**

This PR addresses a part of the issue [#5633
](#5633)

**Description of the changes**
This is a Draft PR to achieve Observability Parity between V1 and V2
components by creating an unified telemetery container to pass
observability clients to V1 components.
**How was this change tested?**

The changes were tested by running the following command:

```bash
make test
```

**Checklist**

- [x] I have read
[CONTRIBUTING_GUIDELINES.md](https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md)
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - `for jaeger: make lint test`
  - `for jaeger-ui: yarn lint` and `yarn test`

---------

Signed-off-by: Wise-Wizard <saransh.shankar@gmail.com>
Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
Wise-Wizard and yurishkuro committed Jul 9, 2024
1 parent 295293c commit a5fd223
Show file tree
Hide file tree
Showing 13 changed files with 268 additions and 116 deletions.
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 {
return fmt.Errorf("could not initialize a tracer: %w", err)
}
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 {
errs = append(errs, s.jtracer.Close(ctx))
if s.closeTracer != nil {
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

0 comments on commit a5fd223

Please sign in to comment.