Skip to content

Commit d9b367e

Browse files
committed
fix(atenet): propagate trace context through router and namespace span/metric attrs
Two bugs broke the trace chain through the router, creating two separate traces instead of one connected trace for each ResumeActor request. 1. Singleflight detaches trace context (primary): resumer.go used context.Background() inside singleflight.DoChan(). Fixed by propagating the caller's span context into the background context via trace.ContextWithSpanContext. 2. No traceparent injected into upstream request (secondary): extproc.go only rewrote the :authority header. Fixed by calling injectTraceContext(ctx, mutation) which uses otel.GetTextMapPropagator() to write traceparent/tracestate with OVERWRITE_IF_EXISTS_OR_ADD AppendAction. 3. Non-namespaced attribute keys: Renamed span and metric attributes to ate.* convention (#412 style). Replaced custom target_addr attribute with stable OTel semconv server.address + server.port.
1 parent c1ab095 commit d9b367e

5 files changed

Lines changed: 242 additions & 16 deletions

File tree

cmd/atenet/internal/router/extproc.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"go.opentelemetry.io/otel/attribute"
3131
"go.opentelemetry.io/otel/metric"
3232
"go.opentelemetry.io/otel/propagation"
33+
"go.opentelemetry.io/otel/semconv/v1.40.0"
3334
"google.golang.org/grpc"
3435

3536
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
@@ -147,6 +148,10 @@ func (s *ExtProcServer) handleRequestHeaders(
147148
// Host is invalid, respond with 404.
148149
return nil, metadata, "", "", "", invalidHostErr(metadata.host, err)
149150
}
151+
span.SetAttributes(
152+
attribute.String("ate.atespace", atespace),
153+
attribute.String("ate.actor.name", actorName),
154+
)
150155

151156
slog.InfoContext(ctx, "ResumeActor", slog.String("atespace", atespace), slog.String("actor", actorName))
152157
actor, err := s.resumer.ResumeActor(ctx, atespace, actorName)
@@ -176,9 +181,18 @@ func (s *ExtProcServer) handleRequestHeaders(
176181

177182
slog.InfoContext(ctx, "Route ok", slog.String("actor", actorName), slog.String("targetAddr", targetAddr))
178183

179-
// Route by rewriting the :authority header.
184+
span.SetAttributes(
185+
semconv.ServerAddress(workerIP),
186+
semconv.ServerPort(80),
187+
attribute.String("ate.actor.template.namespace", tmplNs),
188+
attribute.String("ate.actor.template.name", tmplName),
189+
)
190+
191+
// Route by rewriting the :authority header and injecting trace context
192+
// so the upstream worker receives the same trace as the ingress path.
180193
mutation := &extprocv3.HeaderMutation{}
181194
addAuthorityMutation(targetAddr, mutation)
195+
injectTraceContext(ctx, mutation)
182196

183197
return &extprocv3.HeadersResponse{
184198
Response: &extprocv3.CommonResponse{
@@ -192,9 +206,9 @@ func (s *ExtProcServer) recordRouteDuration(ctx context.Context, d time.Duration
192206
return
193207
}
194208
s.routeDuration.Record(ctx, d.Seconds(), metric.WithAttributes(
195-
attribute.String("actor_template_namespace", tmplNs),
196-
attribute.String("actor_template_name", tmplName),
197-
attribute.String("outcome", outcome),
209+
attribute.String("ate.actor.template.namespace", tmplNs),
210+
attribute.String("ate.actor.template.name", tmplName),
211+
attribute.String("ate.outcome", outcome),
198212
))
199213
}
200214

cmd/atenet/internal/router/extproc_out.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,13 @@
1515
package router
1616

1717
import (
18+
"context"
19+
1820
corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
1921
extproc "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
2022
envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
23+
"go.opentelemetry.io/otel"
24+
"go.opentelemetry.io/otel/propagation"
2125
)
2226

2327
// reqError carries an HTTP-mappable status code and a client-safe message.
@@ -43,6 +47,23 @@ func addAuthorityMutation(auth string, mut *extproc.HeaderMutation) {
4347
)
4448
}
4549

50+
// injectTraceContext injects the trace context from ctx into the header mutation,
51+
// so that Envoy forwards traceparent/tracestate to the upstream worker and spans
52+
// are connected across the full request path.
53+
func injectTraceContext(ctx context.Context, mutation *extproc.HeaderMutation) {
54+
headers := make(map[string]string)
55+
otel.GetTextMapPropagator().Inject(ctx, propagation.MapCarrier(headers))
56+
for k, v := range headers {
57+
mutation.SetHeaders = append(mutation.SetHeaders, &corev3.HeaderValueOption{
58+
Header: &corev3.HeaderValue{
59+
Key: k,
60+
Value: v,
61+
},
62+
AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD,
63+
})
64+
}
65+
}
66+
4667
func immediateResponse(statusCode envoy_type.StatusCode, message string) *extproc.ProcessingResponse {
4768
return &extproc.ProcessingResponse{
4869
Response: &extproc.ProcessingResponse_ImmediateResponse{

cmd/atenet/internal/router/extproc_test.go

Lines changed: 113 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@ import (
2121
"errors"
2222
"log/slog"
2323
"strings"
24+
"sync"
2425
"testing"
2526
"time"
2627

2728
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
2829
corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
2930
extprocv3 "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
3031
envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
32+
"go.opentelemetry.io/otel"
33+
"go.opentelemetry.io/otel/propagation"
34+
sdktrace "go.opentelemetry.io/otel/sdk/trace"
3135
"google.golang.org/grpc"
3236
"google.golang.org/grpc/codes"
3337
"google.golang.org/grpc/status"
@@ -230,17 +234,19 @@ func TestExtProcHeadersEvaluation(t *testing.T) {
230234
}
231235

232236
mutation := res.Response.GetHeaderMutation()
233-
if len(mutation.GetSetHeaders()) != 1 {
234-
t.Fatalf("expected exactly one Header option set, found: %v", mutation.GetSetHeaders())
235-
}
236-
237-
headerOption := mutation.GetSetHeaders()[0]
238-
if strings.ToLower(headerOption.Header.Key) != ":authority" {
239-
t.Errorf("invalid resulting dynamic parameter key: %s", headerOption.Header.Key)
237+
headers := mutation.GetSetHeaders()
238+
var found bool
239+
for _, h := range headers {
240+
if strings.ToLower(h.Header.Key) == ":authority" {
241+
found = true
242+
if string(h.Header.RawValue) != tc.expectedTarget {
243+
t.Errorf("invalid destination mapping found: %s, expected: %s", h.Header.RawValue, tc.expectedTarget)
244+
}
245+
break
246+
}
240247
}
241-
242-
if string(headerOption.Header.RawValue) != tc.expectedTarget {
243-
t.Errorf("invalid destination mapping found: %s, expected: %s", headerOption.Header.RawValue, tc.expectedTarget)
248+
if !found {
249+
t.Errorf(":authority header not found in mutation, got: %v", headers)
244250
}
245251

246252
// Confirm that query logs recorded metric trace details
@@ -252,3 +258,100 @@ func TestExtProcHeadersEvaluation(t *testing.T) {
252258
})
253259
}
254260
}
261+
262+
// inMemoryExporter stores ended spans for test inspection.
263+
type inMemoryExporter struct {
264+
mu sync.Mutex
265+
spans []sdktrace.ReadOnlySpan
266+
}
267+
268+
func (e *inMemoryExporter) ExportSpans(_ context.Context, spans []sdktrace.ReadOnlySpan) error {
269+
e.mu.Lock()
270+
defer e.mu.Unlock()
271+
e.spans = append(e.spans, spans...)
272+
return nil
273+
}
274+
275+
func (e *inMemoryExporter) Shutdown(context.Context) error {
276+
e.mu.Lock()
277+
defer e.mu.Unlock()
278+
e.spans = nil
279+
return nil
280+
}
281+
282+
func (e *inMemoryExporter) Ended() []sdktrace.ReadOnlySpan {
283+
e.mu.Lock()
284+
defer e.mu.Unlock()
285+
ret := make([]sdktrace.ReadOnlySpan, len(e.spans))
286+
copy(ret, e.spans)
287+
return ret
288+
}
289+
290+
func TestInjectTraceContext_AddsTraceparentMatchingParentSpan(t *testing.T) {
291+
prevProp := otel.GetTextMapPropagator()
292+
otel.SetTextMapPropagator(propagation.TraceContext{})
293+
t.Cleanup(func() { otel.SetTextMapPropagator(prevProp) })
294+
295+
exporter := &inMemoryExporter{}
296+
tp := sdktrace.NewTracerProvider(
297+
sdktrace.WithSampler(sdktrace.AlwaysSample()),
298+
sdktrace.WithSpanProcessor(sdktrace.NewSimpleSpanProcessor(exporter)),
299+
)
300+
301+
ctx, span := tp.Tracer("test").Start(context.Background(), "parent")
302+
parentTraceID := span.SpanContext().TraceID()
303+
304+
mutation := &extprocv3.HeaderMutation{}
305+
injectTraceContext(ctx, mutation)
306+
span.End()
307+
308+
var traceparent string
309+
for _, h := range mutation.GetSetHeaders() {
310+
if strings.ToLower(h.Header.Key) == "traceparent" {
311+
traceparent = h.Header.Value
312+
if h.GetAppendAction() != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD {
313+
t.Errorf("AppendAction = %v, want OVERWRITE_IF_EXISTS_OR_ADD", h.GetAppendAction())
314+
}
315+
break
316+
}
317+
}
318+
if traceparent == "" {
319+
t.Fatal("traceparent not found in header mutation")
320+
}
321+
322+
parts := strings.Split(traceparent, "-")
323+
if len(parts) < 2 {
324+
t.Fatalf("invalid traceparent format: %s", traceparent)
325+
}
326+
if parts[1] != parentTraceID.String() {
327+
t.Errorf("trace ID in traceparent = %s, want parent trace ID = %s", parts[1], parentTraceID.String())
328+
}
329+
}
330+
331+
func TestInjectTraceContext_AppendActionDefaultsToOverwrite(t *testing.T) {
332+
prevProp := otel.GetTextMapPropagator()
333+
otel.SetTextMapPropagator(propagation.TraceContext{})
334+
t.Cleanup(func() { otel.SetTextMapPropagator(prevProp) })
335+
336+
exporter := &inMemoryExporter{}
337+
tp := sdktrace.NewTracerProvider(
338+
sdktrace.WithSampler(sdktrace.AlwaysSample()),
339+
sdktrace.WithSpanProcessor(sdktrace.NewSimpleSpanProcessor(exporter)),
340+
)
341+
342+
ctx, span := tp.Tracer("test").Start(context.Background(), "parent")
343+
defer span.End()
344+
345+
mutation := &extprocv3.HeaderMutation{}
346+
injectTraceContext(ctx, mutation)
347+
348+
for _, h := range mutation.GetSetHeaders() {
349+
if strings.ToLower(h.Header.Key) == "traceparent" {
350+
if h.GetAppendAction() != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD {
351+
t.Errorf("AppendAction = %v, want OVERWRITE_IF_EXISTS_OR_ADD", h.GetAppendAction())
352+
}
353+
return
354+
}
355+
}
356+
t.Error("traceparent not found in mutation")
357+
}

cmd/atenet/internal/router/resumer.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ func NewActorResumer(apiClient ateapipb.ControlClient) *ActorResumer {
4646
func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorName string) (*ateapipb.Actor, error) {
4747
ctx, span := otel.Tracer(routerServiceName).Start(ctx, "ResumeActor",
4848
trace.WithAttributes(
49-
attribute.String("atespace", atespace),
50-
attribute.String("actor", actorName),
49+
attribute.String("ate.atespace", atespace),
50+
attribute.String("ate.actor.name", actorName),
5151
))
5252
defer span.End()
5353

@@ -57,6 +57,9 @@ func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorName stri
5757
// resume operation continues running for Caller 2 and Caller 3 without failing.
5858
bgCtx, bgCancel := context.WithTimeout(context.Background(), 15*time.Second)
5959
defer bgCancel()
60+
// Propagate the caller's span context so the gRPC spans are children
61+
// of ResumeActor rather than appearing as a separate trace.
62+
bgCtx = trace.ContextWithSpanContext(bgCtx, trace.SpanContextFromContext(ctx))
6063

6164
backoff := wait.Backoff{
6265
Steps: 7,

cmd/atenet/internal/router/resumer_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"time"
2222

2323
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
24+
"go.opentelemetry.io/otel"
25+
sdktrace "go.opentelemetry.io/otel/sdk/trace"
2426
"google.golang.org/grpc"
2527
"google.golang.org/grpc/codes"
2628
"google.golang.org/grpc/status"
@@ -168,3 +170,86 @@ func TestActorResumer_ResumeActor(t *testing.T) {
168170
}
169171
})
170172
}
173+
174+
type resumerSpanExporter struct {
175+
mu sync.Mutex
176+
spans []sdktrace.ReadOnlySpan
177+
}
178+
179+
func (e *resumerSpanExporter) ExportSpans(_ context.Context, spans []sdktrace.ReadOnlySpan) error {
180+
e.mu.Lock()
181+
defer e.mu.Unlock()
182+
e.spans = append(e.spans, spans...)
183+
return nil
184+
}
185+
186+
func (e *resumerSpanExporter) Shutdown(context.Context) error {
187+
e.mu.Lock()
188+
defer e.mu.Unlock()
189+
e.spans = nil
190+
return nil
191+
}
192+
193+
func (e *resumerSpanExporter) Ended() []sdktrace.ReadOnlySpan {
194+
e.mu.Lock()
195+
defer e.mu.Unlock()
196+
ret := make([]sdktrace.ReadOnlySpan, len(e.spans))
197+
copy(ret, e.spans)
198+
return ret
199+
}
200+
201+
func TestActorResumer_ResumeChildSharesParentTraceID(t *testing.T) {
202+
const testActorName = "actor-a"
203+
const testAtespace = "team-a"
204+
const expectedIP = "10.0.0.52"
205+
206+
exporter := &resumerSpanExporter{}
207+
tp := sdktrace.NewTracerProvider(
208+
sdktrace.WithSampler(sdktrace.AlwaysSample()),
209+
sdktrace.WithSpanProcessor(sdktrace.NewSimpleSpanProcessor(exporter)),
210+
)
211+
prevTP := otel.GetTracerProvider()
212+
otel.SetTracerProvider(tp)
213+
t.Cleanup(func() { otel.SetTracerProvider(prevTP) })
214+
215+
ctx, parent := tp.Tracer("test").Start(context.Background(), "parent")
216+
parentTraceID := parent.SpanContext().TraceID()
217+
parentSpanID := parent.SpanContext().SpanID()
218+
219+
mock := &resumerMockClient{
220+
resumeFn: func(ctx context.Context, in *ateapipb.ResumeActorRequest, opts ...grpc.CallOption) (*ateapipb.ResumeActorResponse, error) {
221+
return &ateapipb.ResumeActorResponse{
222+
Actor: &ateapipb.Actor{
223+
Metadata: &ateapipb.ResourceMetadata{Name: testActorName},
224+
Status: ateapipb.Actor_STATUS_RUNNING,
225+
AteomPodIp: expectedIP,
226+
},
227+
}, nil
228+
},
229+
}
230+
231+
resumer := NewActorResumer(mock)
232+
_, err := resumer.ResumeActor(ctx, testAtespace, testActorName)
233+
if err != nil {
234+
t.Fatalf("unexpected error: %v", err)
235+
}
236+
parent.End()
237+
238+
var resumeSpan sdktrace.ReadOnlySpan
239+
for _, s := range exporter.Ended() {
240+
if s.Name() == "ResumeActor" {
241+
resumeSpan = s
242+
break
243+
}
244+
}
245+
if resumeSpan == nil {
246+
t.Fatal("ResumeActor span not found in exporter")
247+
}
248+
249+
if got, want := resumeSpan.SpanContext().TraceID(), parentTraceID; got != want {
250+
t.Errorf("ResumeActor trace ID = %s, want parent trace ID = %s", got, want)
251+
}
252+
if got, want := resumeSpan.Parent().SpanID(), parentSpanID; got != want {
253+
t.Errorf("ResumeActor parent span ID = %s, want %s", got, want)
254+
}
255+
}

0 commit comments

Comments
 (0)