Update OTLP headers to accept string format (spec v1.13.0)#3338
Merged
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/6d9ca53a-f6f1-4339-a254-cbe6544466ee Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Update gateway to accept OTLP headers as string
Update OTLP headers to accept string format (spec v1.13.0)
Apr 7, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the gateway’s OpenTelemetry OTLP header configuration to match spec v1.13.0, changing opentelemetry.headers from a key/value object to a comma-separated key=value string and parsing it into a map at export time.
Changes:
- Change
TracingConfig.Headers/StdinOpenTelemetryConfig.Headersfrommap[string]stringtostring. - Update config variable expansion to expand the headers string as a whole.
- Add
parseOTLPHeadersand routeresolveHeadersthrough it; update schema to reflectheaders: string.
Show a summary per file
| File | Description |
|---|---|
| internal/tracing/provider.go | Adds parsing for comma-separated OTLP headers and uses it when constructing exporter options. |
| internal/tracing/provider_test.go | Updates OTLP header forwarding test to use the new string format. |
| internal/config/validation.go | Updates ${VAR} expansion for headers from per-map-value expansion to whole-string expansion. |
| internal/config/schema/mcp-gateway-config.schema.json | Switches gateway.opentelemetry.headers schema type from object to string and updates description. |
| internal/config/config_tracing.go | Updates TracingConfig.Headers type and example docs to the new string format. |
| internal/config/config_tracing_test.go | Updates tracing config tests to match the new headers type/expansion behavior. |
| internal/config/config_stdin.go | Updates stdin JSON OpenTelemetry config headers type to string. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 4
Comment on lines
+100
to
+113
| func parseOTLPHeaders(raw string) map[string]string { | ||
| headers := make(map[string]string) | ||
| for _, pair := range strings.Split(raw, ",") { | ||
| trimmed := strings.TrimSpace(pair) | ||
| if trimmed == "" { | ||
| continue | ||
| } | ||
| k, v, ok := strings.Cut(trimmed, "=") | ||
| if !ok { | ||
| logTracing.Printf("Warning: skipping malformed OTLP header pair (missing '='): %q", trimmed) | ||
| continue | ||
| } | ||
| headers[strings.TrimSpace(k)] = strings.TrimSpace(v) | ||
| } |
There was a problem hiding this comment.
parseOTLPHeaders can produce an entry with an empty header name when the input contains pairs like "=value" or " = value". Passing a map with an empty key to the OTLP HTTP exporter can cause request creation to fail due to an invalid HTTP header field name. After trimming, validate that the header key is non-empty (and ideally a valid HTTP token) before adding it; otherwise log a warning and skip the pair.
internal/tracing/provider.go
Outdated
| } | ||
| k, v, ok := strings.Cut(trimmed, "=") | ||
| if !ok { | ||
| logTracing.Printf("Warning: skipping malformed OTLP header pair (missing '='): %q", trimmed) |
There was a problem hiding this comment.
The warning log for malformed OTLP header pairs includes the raw (trimmed) pair content. If a user accidentally misformats an Authorization header (e.g., missing '='), this could leak secrets into logs. Consider logging without echoing the full pair (or redact/truncate values) to avoid accidental credential exposure.
Suggested change
| logTracing.Printf("Warning: skipping malformed OTLP header pair (missing '='): %q", trimmed) | |
| logTracing.Printf("Warning: skipping malformed OTLP header pair (missing '=')") |
internal/tracing/provider_test.go
Outdated
Comment on lines
333
to
336
| cfg := &config.TracingConfig{ | ||
| Endpoint: ts.URL, | ||
| Headers: map[string]string{"Authorization": "Bearer test-token"}, | ||
| Headers: "Authorization=Bearer test-token", | ||
| } |
There was a problem hiding this comment.
There’s new parsing behavior for comma-separated OTLP headers (trimming, skipping malformed pairs). provider_test only verifies a single well-formed header is forwarded; adding focused test coverage for multiple headers + whitespace trimming + malformed/empty-key cases would help prevent regressions in parseOTLPHeaders/resolveHeaders.
See below for a potential fix:
tests := []struct {
name string
headers string
expected map[string]string
notExpectedSet []string
}{
{
name: "single well formed header",
headers: "Authorization=Bearer test-token",
expected: map[string]string{
"Authorization": "Bearer test-token",
},
},
{
name: "multiple headers with whitespace are trimmed",
headers: " Authorization = Bearer test-token , X-Request-ID = req-123 ",
expected: map[string]string{
"Authorization": "Bearer test-token",
"X-Request-Id": "req-123",
},
},
{
name: "malformed and empty key headers are skipped",
headers: "Authorization=Bearer test-token, malformed, =empty-key, X-Trace-ID=trace-123",
expected: map[string]string{
"Authorization": "Bearer test-token",
"X-Trace-Id": "trace-123",
},
notExpectedSet: []string{"", "malformed"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Channel signals when the test server receives an export request.
received := make(chan http.Header, 1)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
headers := r.Header.Clone()
select {
case received <- headers:
default:
}
w.WriteHeader(http.StatusOK)
}))
defer ts.Close()
cfg := &config.TracingConfig{
Endpoint: ts.URL,
Headers: tt.headers,
}
provider, err := tracing.InitProvider(ctx, cfg)
require.NoError(t, err)
require.NotNil(t, provider)
// Create and end a span to trigger an export attempt.
tr := provider.Tracer()
_, span := tr.Start(ctx, "header-test-span")
span.End()
// Shutdown flushes the batch processor, ensuring the export is sent.
shutdownCtx, cancel := context.WithTimeout(ctx, 2*time.Second)
defer cancel()
_ = provider.Shutdown(shutdownCtx)
// Wait for the export request with a timeout.
select {
case headers := <-received:
for key, expectedValue := range tt.expected {
assert.Equal(t, expectedValue, headers.Get(key),
fmt.Sprintf("%s header must be forwarded to the OTLP collector", key))
}
for _, key := range tt.notExpectedSet {
assert.Empty(t, headers.Get(key),
fmt.Sprintf("%s header must be skipped when OTLP headers are malformed", key))
}
case <-time.After(3 * time.Second):
t.Fatal("timed out waiting for OTLP export request — headers test is non-deterministic")
}
})
| }, | ||
| "headers": { | ||
| "type": "string", | ||
| "description": "Comma-separated key=value HTTP headers sent with every OTLP export request (e.g. \"Authorization=Bearer ${OTEL_TOKEN}\"). Supports ${VAR} expansion." |
There was a problem hiding this comment.
The schema now documents opentelemetry.headers as a comma-separated key=value string, but it accepts any string. If this schema is used for validation/autocomplete, consider adding a (reasonably permissive) pattern to catch obviously malformed values (e.g., missing '=' between key/value) early, before runtime warnings.
Suggested change
| "description": "Comma-separated key=value HTTP headers sent with every OTLP export request (e.g. \"Authorization=Bearer ${OTEL_TOKEN}\"). Supports ${VAR} expansion." | |
| "description": "Comma-separated key=value HTTP headers sent with every OTLP export request (e.g. \"Authorization=Bearer ${OTEL_TOKEN}\"). Supports ${VAR} expansion.", | |
| "pattern": "^(\\$\\{[A-Za-z_][A-Za-z0-9_]*\\}|\\s*[^=\\s,][^=,]*=.*?(\\s*,\\s*[^=\\s,][^=,]*=.*?)*)\\s*$" |
Collaborator
|
@copilot apply changes based on the comments in this thread |
…ecret log, better tests, schema pattern Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/67af45f2-b80e-4dce-b095-042952b04590 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Author
Applied all four review suggestions in commit
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:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
Spec v1.13.0 changes
opentelemetry.headersfrom an object to a comma-separatedkey=valuestring. The compiler now emits"headers": "Authorization=******"instead of a JSON object.Changes
TracingConfig.Headers/StdinOpenTelemetryConfig.Headers:map[string]string→stringexpandTracingVariables: expands the headers string directly instead of iterating map valuesparseOTLPHeaders(new): parses"key=value,key=value"intomap[string]stringat export time; logs a warning for malformed pairsresolveHeaders: delegates toparseOTLPHeadersopentelemetry.headerschanged fromtype: object+additionalProperties: stringtotype: stringExample
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-build113898129/b514/launcher.test /tmp/go-build113898129/b514/launcher.test -test.testlogfile=/tmp/go-build113898129/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build113898129/b216/vet.cfg 7845551/b412/_pkg_.a 64/src/go/scanner/errors.go ache/go/1.25.8/x64/pkg/tool/linu-nilfunc -p l -lang=go1.25 ache/go/1.25.8/x64/pkg/tool/linu-tests -w 738659/b232/_pkg_.a ylov/l0IgnnYtDUbmqPCsylov x_amd64/vet -c -I /tmp/go-build171-bool x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build861738659/b222/config.test /tmp/go-build861738659/b222/config.test -test.testlogfile=/tmp/go-build861738659/b222/testlog.txt -test.paniconexit0 -test.timeout=10m0s -c go t.go x_amd64/compile(dns block)nonexistent.local/tmp/go-build113898129/b514/launcher.test /tmp/go-build113898129/b514/launcher.test -test.testlogfile=/tmp/go-build113898129/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build113898129/b216/vet.cfg 7845551/b412/_pkg_.a 64/src/go/scanner/errors.go ache/go/1.25.8/x64/pkg/tool/linu-nilfunc -p l -lang=go1.25 ache/go/1.25.8/x64/pkg/tool/linu-tests -w 738659/b232/_pkg_.a ylov/l0IgnnYtDUbmqPCsylov x_amd64/vet -c -I /tmp/go-build171-bool x_amd64/vet(dns block)slow.example.com/tmp/go-build113898129/b514/launcher.test /tmp/go-build113898129/b514/launcher.test -test.testlogfile=/tmp/go-build113898129/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build113898129/b216/vet.cfg 7845551/b412/_pkg_.a 64/src/go/scanner/errors.go ache/go/1.25.8/x64/pkg/tool/linu-nilfunc -p l -lang=go1.25 ache/go/1.25.8/x64/pkg/tool/linu-tests -w 738659/b232/_pkg_.a ylov/l0IgnnYtDUbmqPCsylov x_amd64/vet -c -I /tmp/go-build171-bool x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build113898129/b523/mcp.test /tmp/go-build113898129/b523/mcp.test -test.testlogfile=/tmp/go-build113898129/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build113898129/b257/vet.cfg 7845551/b362/_pkg_.a -trimpath(dns block)If you need me to access, download, or install something from one of these locations, you can either: