Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 10 additions & 1 deletion internal/events/rabbitmq/consumer/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,16 @@ func (r *rabbitmqConsumer) handler(msgs <-chan amqp.Delivery, handler events.Han
),
)

logger := r.logger.With().Str("topic", topic).Ctx(ctx).Logger()
logCtx := r.logger.With().Str("topic", topic).Ctx(ctx)
// Enrich with the OTEL trace_id so every log across the message
// lifecycle (decode errors, ack/nack failures, retries, and the
// application handler itself) carries the same id as the producer
// span. ExtractTrace above already restored the trace context from
// the message headers.
if sc := trace.SpanContextFromContext(ctx); sc.HasTraceID() {
logCtx = logCtx.Str("trace_id", sc.TraceID().String())
}
logger := logCtx.Logger()
ctx = logger.WithContext(ctx)
ctx = thunderContext.ContextWithMetadata(ctx, metadataFromAmqpTable(msg.Headers))
// ensures that the correlation ID is propagated or generated
Expand Down
14 changes: 10 additions & 4 deletions pkg/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,17 @@ type NewServerParams struct {
func NewServer(params NewServerParams) *BareServer {
grpcServer := &BareServer{}

// We want to add the MetadataPropagator interceptor first and
// logger interceptor last.
// MetadataPropagator first, then the base logger, then the supplied
// interceptors. The base logger MUST come before supplied interceptors:
// they enrich the context logger (e.g. audit fields, trace_id), and if
// the base logger ran after them it would replace the context logger and
// silently drop that enrichment before the handler runs.
params.Interceptors = append(
[]grpc.UnaryServerInterceptor{UnaryServerMetadataPropagator},
append(params.Interceptors, grpcLoggerInterceptor(params.Logger))...,
[]grpc.UnaryServerInterceptor{
UnaryServerMetadataPropagator,
grpcLoggerInterceptor(params.Logger),
},
params.Interceptors...,
)

// default max message size is 4MB
Expand Down
81 changes: 81 additions & 0 deletions pkg/grpc/server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package grpc

import (
"bytes"
"context"
"encoding/json"
"testing"

"github.com/rs/zerolog"
"google.golang.org/grpc"
)

// TestSuppliedInterceptorEnrichmentReachesHandler is the regression for the
// interceptor-ordering bug: the base logger interceptor used to run AFTER
// supplied interceptors, replacing the context logger and silently dropping
// any enrichment (trace_id, audit fields) before the handler executed.
//
// It simulates a supplied interceptor that enriches the context logger (the
// same pattern backend-commons logs.UnaryServerInterceptor uses) and asserts
// the handler's log line actually carries the enriched fields.
func TestSuppliedInterceptorEnrichmentReachesHandler(t *testing.T) {
var buf bytes.Buffer
logger := zerolog.New(&buf)

// Supplied interceptor: enriches the context logger, exactly like
// backend-commons logs.UnaryServerInterceptor does for trace_id.
enriching := func(
ctx context.Context,
req interface{},
info *grpc.UnaryServerInfo,
handler grpc.UnaryHandler,
) (interface{}, error) {
l := zerolog.Ctx(ctx).With().
Str("trace_id", "trace-abc-123").
Str("grpc_method", info.FullMethod).
Logger()
return handler(l.WithContext(ctx), req)
}

// Compose the chain the same way NewServer does.
interceptors := append(
[]grpc.UnaryServerInterceptor{
UnaryServerMetadataPropagator,
grpcLoggerInterceptor(&logger),
},
enriching,
)

// Handler logs from its context, like real service handlers do.
handler := func(ctx context.Context, req interface{}) (interface{}, error) {
zerolog.Ctx(ctx).Info().Msg("handled")
return "ok", nil
}

// Manually chain (mirrors grpc.ChainUnaryInterceptor semantics).
chained := handler
for i := len(interceptors) - 1; i >= 0; i-- {
ic := interceptors[i]
next := chained
info := &grpc.UnaryServerInfo{FullMethod: "/test.Service/Do"}
chained = func(ctx context.Context, req interface{}) (interface{}, error) {
return ic(ctx, req, info, next)
}
}

if _, err := chained(context.Background(), "req"); err != nil {
t.Fatalf("unexpected error: %v", err)
}

var entry map[string]any
if err := json.Unmarshal(buf.Bytes(), &entry); err != nil {
t.Fatalf("failed to parse handler log: %v\noutput: %s", err, buf.String())
}

if entry["trace_id"] != "trace-abc-123" {
t.Errorf("handler log trace_id = %v, want trace-abc-123 (enrichment was dropped)", entry["trace_id"])
}
if entry["grpc_method"] != "/test.Service/Do" {
t.Errorf("handler log grpc_method = %v, want /test.Service/Do", entry["grpc_method"])
}
}
Loading