Skip to content

Commit 8599e6e

Browse files
Copilotlpcox
andauthored
test: improve ParentContext and serviceName defaults tests with more precise assertions
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/68c6c224-9436-4b1d-9289-79e3af4c4d53 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent c1325b7 commit 8599e6e

2 files changed

Lines changed: 37 additions & 26 deletions

File tree

internal/config/config_tracing_test.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -161,18 +161,23 @@ func TestOTEL009_SpanIDWithoutTraceID_NoError(t *testing.T) {
161161
}
162162

163163
// T-OTEL-010: serviceName defaults to "mcp-gateway" when not specified.
164+
// Tests the actual registered defaults setter applied via applyDefaults.
164165
func TestOTEL010_ServiceNameDefaults(t *testing.T) {
165-
cfg := &TracingConfig{
166-
Endpoint: "https://otel-collector.example.com",
167-
// ServiceName intentionally absent
168-
}
169-
assert.Equal(t, "", cfg.ServiceName, "ServiceName should be empty before defaults are applied")
170-
171-
// Simulate defaults application
172-
if cfg.ServiceName == "" {
173-
cfg.ServiceName = DefaultTracingServiceName
174-
}
175-
assert.Equal(t, "mcp-gateway", cfg.ServiceName, "T-OTEL-010: default serviceName must be 'mcp-gateway'")
166+
// Test the constant
167+
assert.Equal(t, "mcp-gateway", DefaultTracingServiceName, "T-OTEL-010: DefaultTracingServiceName must be 'mcp-gateway'")
168+
169+
// Test that the defaults setter correctly applies the default service name
170+
cfg := &Config{
171+
Gateway: &GatewayConfig{
172+
Tracing: &TracingConfig{
173+
Endpoint: "https://otel-collector.example.com",
174+
// ServiceName intentionally absent
175+
},
176+
},
177+
}
178+
applyDefaults(cfg)
179+
assert.Equal(t, "mcp-gateway", cfg.Gateway.Tracing.ServiceName,
180+
"T-OTEL-010: default serviceName must be 'mcp-gateway' after applyDefaults")
176181
}
177182

178183
// TestValidateOpenTelemetryConfig_VarExpressions verifies that ${VAR} expressions

internal/tracing/provider_test.go

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -359,29 +359,35 @@ func TestInitProvider_WithHeaders(t *testing.T) {
359359
func TestParentContext_WithValidTraceIDAndSpanID(t *testing.T) {
360360
ctx := context.Background()
361361

362+
// Initialize noop provider to set up the W3C propagator globally
363+
provider, err := tracing.InitProvider(ctx, nil)
364+
require.NoError(t, err)
365+
defer provider.Shutdown(ctx)
366+
362367
cfg := &config.TracingConfig{
363368
Endpoint: "https://example.com",
364369
TraceID: "4bf92f3577b34da6a3ce929d0e0e4736",
365370
SpanID: "00f067aa0ba902b7",
366371
}
367372

368373
parentCtx := tracing.ParentContext(ctx, cfg)
369-
sc := trace.SpanFromContext(parentCtx).SpanContext()
370-
371-
// ParentContext should inject a remote span context, not a local span.
372-
// The remote span context is accessible via trace.SpanContextFromContext after
373-
// trace.ContextWithRemoteSpanContext is used — check via propagation round-trip.
374-
// Instead of checking SpanFromContext (which returns noop span in the absence of a started span),
375-
// extract the remote span context directly.
376-
remoteCtx := trace.SpanContextFromContext(parentCtx)
377-
if !remoteCtx.IsValid() {
378-
// Check the context for the remote parent
379-
// The remote span context might not be on the regular span context
380-
t.Logf("SpanFromContext returned: %v", sc)
381-
t.Logf("SpanContextFromContext returned: %v", remoteCtx)
382-
}
383-
// Verify the context is enriched (not the zero-value background context)
374+
375+
// The context must be enriched (different from background context)
384376
assert.NotEqual(t, ctx, parentCtx, "ParentContext must return an enriched context")
377+
378+
// Verify the remote span context contains the correct traceId and spanId
379+
// by extracting it from the context and checking via propagation round-trip.
380+
prop := otel.GetTextMapPropagator()
381+
carrier := propagation.MapCarrier{}
382+
prop.Inject(parentCtx, carrier)
383+
traceparent := carrier["traceparent"]
384+
require.NotEmpty(t, traceparent, "W3C traceparent must be present after injection")
385+
386+
// traceparent format: 00-{traceId}-{spanId}-{flags}
387+
assert.Contains(t, traceparent, "4bf92f3577b34da6a3ce929d0e0e4736",
388+
"traceparent must contain the configured traceId")
389+
assert.Contains(t, traceparent, "00f067aa0ba902b7",
390+
"traceparent must contain the configured spanId")
385391
}
386392

387393
// TestParentContext_WithTraceIDOnly verifies that ParentContext works when only traceId is provided.

0 commit comments

Comments
 (0)