Conversation
- Add Headers, TraceID, SpanID fields to TracingConfig - Add opentelemetry TOML key (spec §4.1.3.6) alongside legacy tracing key - Add StdinOpenTelemetryConfig + wire into StdinGatewayConfig + stdin converter - Update JSON schema to include opentelemetry in gatewayConfig - Add validation: HTTPS endpoint, traceId (32-char hex), spanId (16-char hex) - Update InitProvider to pass headers to OTLP exporter - Add W3C parent context support from traceId/spanId - Update callBackendTool to use spec attribute names (mcp.server, mcp.method, mcp.tool, http.status_code) - Add compliance tests T-OTEL-001 through T-OTEL-010 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>
…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>
Copilot
AI
changed the title
[WIP] Support spec v1.11.0 OpenTelemetry OTLP tracing configuration
feat: Support spec v1.11.0 — OpenTelemetry OTLP tracing configuration (§4.1.3.6)
Apr 4, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the gateway’s tracing support to align with MCP Gateway Spec v1.11.0 §4.1.3.6 by introducing an opentelemetry config section, adding W3C parent context support (traceId/spanId), and updating tool-call span naming/attributes to the spec’s conventions.
Changes:
- Add OpenTelemetry OTLP options: export headers + W3C remote parent context construction from
traceId/spanId. - Add/validate
[gateway.opentelemetry]config (TOML + stdin JSON + schema) with precedence over legacy[gateway.tracing]. - Update
callBackendToolspan name/attributes to spec keys and addhttp.status_codeattribute.
Show a summary per file
| File | Description |
|---|---|
| internal/tracing/provider.go | Adds OTLP export headers and W3C remote parent context creation logic. |
| internal/tracing/provider_test.go | Adds tests for headers and ParentContext behavior. |
| internal/server/unified.go | Renames tool-call span and updates attributes + sets http.status_code. |
| internal/config/validation.go | Adds spec validation for opentelemetry endpoint/traceId/spanId. |
| internal/config/schema/mcp-gateway-config.schema.json | Adds gateway.opentelemetry to schema and reformats schema document. |
| internal/config/config_tracing.go | Extends TracingConfig with Headers/TraceID/SpanID; adds stdin converter for opentelemetry. |
| internal/config/config_tracing_test.go | Adds compliance tests for §4.1.3.6 validation/defaulting. |
| internal/config/config_stdin.go | Adds stdin JSON struct for gateway.opentelemetry. |
| internal/config/config_core.go | Adds GatewayConfig.Opentelemetry and merges it into legacy tracing with precedence + validation. |
| internal/cmd/root.go | Applies configured W3C parent context at startup. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 6
internal/config/validation.go
Outdated
Comment on lines
+517
to
+526
| // Validate traceId: must be 32-char lowercase hex (or a ${VAR} expression) | ||
| if cfg.TraceID != "" && !varExprPatternSimple.MatchString(cfg.TraceID) { | ||
| if !traceIDPattern.MatchString(cfg.TraceID) { | ||
| logValidation.Printf("Invalid traceId format: %s", cfg.TraceID) | ||
| return rules.InvalidValue("traceId", | ||
| fmt.Sprintf("traceId must be a 32-character lowercase hexadecimal string, got '%s'", cfg.TraceID), | ||
| "gateway.opentelemetry.traceId", | ||
| "Provide a valid W3C trace ID (32 lowercase hex chars, e.g., \"4bf92f3577b34da6a3ce929d0e0e4736\")") | ||
| } | ||
| } |
There was a problem hiding this comment.
W3C Trace Context forbids an all-zero traceId; the current regex check would accept it, and ParentContext will later drop it as an invalid SpanContext. Consider explicitly rejecting the all-zero traceId (and optionally all-zero spanId) during validation so misconfiguration is caught early and consistently.
internal/tracing/provider_test.go
Outdated
Comment on lines
+349
to
+354
| // The test server may or may not have received the export within the timeout. | ||
| // If it did, verify the Authorization header was forwarded correctly. | ||
| if capturedAuth != "" { | ||
| assert.Equal(t, "Bearer test-token", capturedAuth, | ||
| "Authorization header must be forwarded to the OTLP collector") | ||
| } |
There was a problem hiding this comment.
This test is effectively non-asserting: it only checks the forwarded header if capturedAuth is non-empty, so it can pass even when headers are not applied. Since Shutdown should flush the batcher, consider making the test deterministic by waiting for a request (e.g., via a channel) and failing if no export arrives within a short timeout, then asserting the Authorization header.
Comment on lines
+34
to
+46
| // Headers are HTTP headers sent with every OTLP export request (e.g. auth tokens). | ||
| // Values support ${VAR} variable expansion. | ||
| Headers map[string]string `toml:"headers" json:"headers,omitempty"` | ||
|
|
||
| // TraceID is an optional W3C trace ID (32-char lowercase hex) used to construct the | ||
| // parent traceparent header, linking gateway spans into a pre-existing trace. | ||
| // Supports ${VAR} variable expansion. Must be 32 lowercase hex characters. | ||
| TraceID string `toml:"trace_id" json:"traceId,omitempty"` | ||
|
|
||
| // SpanID is an optional W3C span ID (16-char lowercase hex) paired with TraceID | ||
| // to construct the parent traceparent header. Ignored when TraceID is absent. | ||
| // Supports ${VAR} variable expansion. Must be 16 lowercase hex characters. | ||
| SpanID string `toml:"span_id" json:"spanId,omitempty"` |
There was a problem hiding this comment.
Comments state tracing Headers/TraceID/SpanID “support ${VAR} variable expansion”, but TOML config loading doesn’t appear to perform ${VAR} expansion for these fields (stdin JSON does via ExpandRawJSONVariables). Consider clarifying the comment to match actual behavior, or implementing variable expansion for TOML-loaded tracing config so the documented behavior is true.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Add TOML variable expansion for tracing fields (endpoint, traceId, spanId, headers) via expandTracingVariables(), matching stdin JSON path behavior. Comments now accurately reflect expansion support. - Reject all-zero traceId/spanId per W3C Trace Context specification. - Make TestInitProvider_WithHeaders deterministic using a channel instead of conditional assertion on captured auth header. - Remove unused varExprPatternSimple regex. - Add tests: variable expansion, undefined var error, all-zero traceId/spanId rejection, unexpanded var expression rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the
opentelemetrygateway config section from MCP Gateway Spec v1.11.0 §4.1.3.6, bridging the gap between the existingTracingConfigskeleton and full spec compliance.Config changes
TracingConfig:Headers map[string]string,TraceID string,SpanID string[gateway.opentelemetry]alongside legacy[gateway.tracing](backward-compatible;opentelemetrytakes precedence)StdinOpenTelemetryConfigwired intoStdinGatewayConfig.opentelemetryopentelemetryadded togatewayConfigdefinition with correct types and patternsValidation (enforced only for the
opentelemetrysection)endpointrequired and must be HTTPStraceIdmust be 32-char lowercase hex (or${VAR})spanIdmust be 16-char lowercase hex (or${VAR})spanIdwithouttraceId→ warning, not errorOTLP provider
otlptracehttp.WithHeaders()traceId+spanIdat startup; missingspanIdgenerates a random one (T-OTEL-008)tracing.ParentContext(ctx, cfg)exported for use at startupInstrumentation (
callBackendTool)gateway.tool_call→mcp.tool_callmcp.server,mcp.method("tools/call"),mcp.toolhttp.status_codeattribute added: 200 (success) / 403 (DIFC denied) / 500 (error)Tests
Compliance tests T-OTEL-001 through T-OTEL-010 covering config validation, header propagation, W3C traceparent construction, random span ID generation, and service name defaulting.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build344646749/b510/launcher.test /tmp/go-build344646749/b510/launcher.test -test.testlogfile=/tmp/go-build344646749/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 0/clients.go 0/doc.go x_amd64/compile OUTPUT -d 168.63.129.16 x_amd64/compile o_.o�� ache/go/1.25.8/x-errorsas oZMU/gIMg3r8eCE3-ifaceassert de/node/bin/as -p sh -lang=go1.25 646749/b171/_x00-buildtags(dns block)/tmp/go-build952104743/b510/launcher.test /tmp/go-build952104743/b510/launcher.test -test.testlogfile=/tmp/go-build952104743/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true cmCl2WHYI 646749/b318/ 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linux_amd64/vet 6467�� olang.org/grpc@v1.80.0/internal/pretty/pretty.go-p pkg/mod/github.com/go-logr/logr@v1.4.3/context_smain cfg -b 646749/b318/_cgo/usr/bin/runc p/bin/git ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile(dns block)