From 39323bf187748576d39ed7d7d4b96ecdc9bdf290 Mon Sep 17 00:00:00 2001 From: Kleonikos Kyriakis Date: Fri, 17 May 2024 09:35:41 +0300 Subject: [PATCH] Fix missing requestID due to empty traceID --- internal/rpc/server/server.go | 8 +++++--- internal/tracing/nooptracer.go | 11 +++++++++++ internal/tracing/nooptracer_test.go | 24 ++++++++++++++++++++++++ internal/tracing/tracer.go | 8 +++++++- internal/tracing/tracer_test.go | 21 +++++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 internal/tracing/nooptracer_test.go create mode 100644 internal/tracing/tracer_test.go diff --git a/internal/rpc/server/server.go b/internal/rpc/server/server.go index e9d722c5..b4439830 100644 --- a/internal/rpc/server/server.go +++ b/internal/rpc/server/server.go @@ -7,6 +7,8 @@ import ( "log" "net" + "github.com/chain4travel/camino-messenger-bot/internal/tracing" + "buf.build/gen/go/chain4travel/camino-messenger-protocol/grpc/go/cmp/services/activity/v1alpha/activityv1alphagrpc" "buf.build/gen/go/chain4travel/camino-messenger-protocol/grpc/go/cmp/services/book/v1alpha/bookv1alphagrpc" bookv1alpha "buf.build/gen/go/chain4travel/camino-messenger-protocol/protocolbuffers/go/cmp/services/book/v1alpha" @@ -59,7 +61,7 @@ type server struct { grpcServer *grpc.Server cfg *config.RPCServerConfig logger *zap.SugaredLogger - tracer trace.Tracer + tracer tracing.Tracer processor messaging.Processor serviceRegistry *messaging.ServiceRegistry } @@ -68,7 +70,7 @@ func (s *server) Checkpoint() string { return "request-gateway" } -func NewServer(cfg *config.RPCServerConfig, logger *zap.SugaredLogger, tracer trace.Tracer, processor messaging.Processor, serviceRegistry *messaging.ServiceRegistry) *server { +func NewServer(cfg *config.RPCServerConfig, logger *zap.SugaredLogger, tracer tracing.Tracer, processor messaging.Processor, serviceRegistry *messaging.ServiceRegistry) *server { var opts []grpc.ServerOption if cfg.Unencrypted { logger.Warn("Running gRPC server without TLS!") @@ -182,7 +184,7 @@ func (s *server) processInternalRequest(ctx context.Context, requestType messagi func (s *server) processExternalRequest(ctx context.Context, requestType messaging.MessageType, request *messaging.RequestContent) (messaging.ResponseContent, error) { ctx, span := s.tracer.Start(ctx, "server.processExternalRequest", trace.WithSpanKind(trace.SpanKindServer)) defer span.End() - err, md := s.processMetadata(ctx, span.SpanContext().TraceID()) + err, md := s.processMetadata(ctx, s.tracer.TraceIDForSpan(span)) if err != nil { return messaging.ResponseContent{}, fmt.Errorf("error processing metadata: %v", err) } diff --git a/internal/tracing/nooptracer.go b/internal/tracing/nooptracer.go index 265c77c6..7eec09d2 100644 --- a/internal/tracing/nooptracer.go +++ b/internal/tracing/nooptracer.go @@ -7,6 +7,8 @@ package tracing import ( "context" + "crypto/rand" + "go.opentelemetry.io/otel/trace" ) @@ -19,9 +21,18 @@ type noopTracer struct { func NewNoOpTracer() (Tracer, error) { return &noopTracer{tp: trace.NewNoopTracerProvider()}, nil } + func (n *noopTracer) Start(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { return n.tp.Tracer("").Start(ctx, spanName, opts...) } + func (n *noopTracer) Shutdown() error { return nil // nothing to do here } + +// TraceIDForSpan returns a random trace ID in tha case of noopTracer. A non-empty trace ID is required for the span to be exported. +func (n *noopTracer) TraceIDForSpan(trace.Span) trace.TraceID { + traceID := trace.TraceID{} + rand.Read(traceID[:]) + return traceID +} diff --git a/internal/tracing/nooptracer_test.go b/internal/tracing/nooptracer_test.go new file mode 100644 index 00000000..ea26e525 --- /dev/null +++ b/internal/tracing/nooptracer_test.go @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2024, Chain4Travel AG. All rights reserved. + * See the file LICENSE for licensing terms. + */ + +package tracing + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/otel/trace" +) + +func TestNoopTraceIDForSpan(t *testing.T) { + tracer, err := NewNoOpTracer() + require.NoError(t, err) + _, span := tracer.Start(context.Background(), "test") + got := tracer.TraceIDForSpan(span) + emptyTraceID := trace.TraceID{} + require.NotEqual(t, emptyTraceID, got) +} diff --git a/internal/tracing/tracer.go b/internal/tracing/tracer.go index 247913e9..a97436a4 100644 --- a/internal/tracing/tracer.go +++ b/internal/tracing/tracer.go @@ -7,13 +7,14 @@ package tracing import ( "context" + "time" + "github.com/chain4travel/camino-messenger-bot/config" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.12.0" "go.opentelemetry.io/otel/trace" - "time" ) const tracerProviderShutdownTimeout = exportTimeout + 5*time.Second @@ -22,6 +23,7 @@ var _ Tracer = (*tracer)(nil) type Tracer interface { trace.Tracer + TraceIDForSpan(trace.Span) trace.TraceID // TraceIDForSpan returns the trace ID of the given span. Shutdown() error } @@ -38,6 +40,10 @@ func (t *tracer) Shutdown() error { return t.tp.Shutdown(ctx) } +func (t *tracer) TraceIDForSpan(span trace.Span) trace.TraceID { + return span.SpanContext().TraceID() +} + func NewTracer(tracingConfig *config.TracingConfig, name string) (Tracer, error) { exporter, err := newExporter(tracingConfig) if err != nil { diff --git a/internal/tracing/tracer_test.go b/internal/tracing/tracer_test.go new file mode 100644 index 00000000..2fb02e80 --- /dev/null +++ b/internal/tracing/tracer_test.go @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2024, Chain4Travel AG. All rights reserved. + * See the file LICENSE for licensing terms. + */ + +package tracing + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestTraceIDForSpan(t *testing.T) { + tracer, err := NewNoOpTracer() + require.NoError(t, err) + _, span := tracer.Start(context.Background(), "test") + got := tracer.TraceIDForSpan(span) + require.NotEqual(t, span.SpanContext().TraceID(), got) +}