From 1ee9d363c90bedf22d60926e33f451145f314919 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sat, 16 May 2026 15:40:35 -0700 Subject: [PATCH 1/2] feat: read OTEL_EXPORTER_OTLP_HEADERS env var as fallback for OTLP export headers When gateway.opentelemetry.headers (stdin JSON) or gateway.tracing.headers (TOML) is not set, resolveHeaders() now falls back to the standard OTEL_EXPORTER_OTLP_HEADERS environment variable. This follows the OTel OTLP Exporter specification (W3C Baggage format: key1=value1,key2=value2). Config headers still take precedence over the env var when both are set. This is the companion mcpg change for github/gh-aw#32277 and gh-aw PR #32280, which removed headers from the gateway JSON config and now passes them exclusively via container env var. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 1 + docs/ENVIRONMENT_VARIABLES.md | 1 + internal/tracing/parse_headers_test.go | 50 ++++++++++++++++++++++++++ internal/tracing/provider.go | 18 ++++++++-- 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 927dcbbda..e5f3e564e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -408,6 +408,7 @@ DEBUG_COLORS=0 DEBUG=* ./awmg --config config.toml - `MCP_GATEWAY_CA_CERT` - Path to CA certificate PEM file for client certificate verification; enables mutual TLS (mTLS) when set alongside `MCP_GATEWAY_TLS_CERT`/`MCP_GATEWAY_TLS_KEY` (sets default for `--tls-ca`) - `MCP_GATEWAY_HMAC_SECRET` - Shared HMAC-SHA256 secret for request signing and replay protection; when set, requests to MCP handlers must carry valid `X-MCP-Timestamp`, `X-MCP-Nonce`, and `X-MCP-Signature` headers (sets default for `--hmac-secret`) - `OTEL_EXPORTER_OTLP_ENDPOINT` - OTLP HTTP endpoint for trace export; sets default for `--otlp-endpoint` +- `OTEL_EXPORTER_OTLP_HEADERS` - Comma-separated `key=value` OTLP export headers (W3C Baggage format); used as fallback when `gateway.opentelemetry.headers` / `gateway.tracing.headers` is not set in config - `OTEL_SERVICE_NAME` - Service name in traces; sets default for `--otlp-service-name` - `AWMG_BINARY_PATH` - Override binary path for integration tests - `AWMG_WASM_GUARD_PATH` - Override WASM guard path for proxy integration tests diff --git a/docs/ENVIRONMENT_VARIABLES.md b/docs/ENVIRONMENT_VARIABLES.md index f63834765..8c8a30fc3 100644 --- a/docs/ENVIRONMENT_VARIABLES.md +++ b/docs/ENVIRONMENT_VARIABLES.md @@ -112,4 +112,5 @@ These standard OpenTelemetry environment variables set defaults for the correspo | Variable | Description | Default | |----------|-------------|---------| | `OTEL_EXPORTER_OTLP_ENDPOINT` | OTLP HTTP endpoint for trace export (e.g., `http://localhost:4318`). Tracing is disabled when empty. Sets default for `--otlp-endpoint`. | (disabled) | +| `OTEL_EXPORTER_OTLP_HEADERS` | Comma-separated `key=value` HTTP headers for OTLP export requests (W3C Baggage format, e.g., `Authorization=Bearer token,X-Custom=value`). Used as fallback when `gateway.opentelemetry.headers` / `gateway.tracing.headers` is not set in config. | (none) | | `OTEL_SERVICE_NAME` | Service name reported in traces. Sets default for `--otlp-service-name`. | `mcp-gateway` | diff --git a/internal/tracing/parse_headers_test.go b/internal/tracing/parse_headers_test.go index 644b1a3a3..2cb7e01cb 100644 --- a/internal/tracing/parse_headers_test.go +++ b/internal/tracing/parse_headers_test.go @@ -4,6 +4,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/github/gh-aw-mcpg/internal/config" ) // TestParseOTLPHeaders covers the parseOTLPHeaders helper with a range of inputs. @@ -87,3 +90,50 @@ func TestParseOTLPHeaders(t *testing.T) { }) } } + +// TestResolveHeaders_ConfigTakesPrecedence verifies that config headers +// take precedence over the OTEL_EXPORTER_OTLP_HEADERS environment variable. +func TestResolveHeaders_ConfigTakesPrecedence(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-token") + + cfg := &config.TracingConfig{ + Headers: "Authorization=Bearer config-token", + } + headers := resolveHeaders(cfg) + require.NotNil(t, headers) + assert.Equal(t, "Bearer config-token", headers["Authorization"]) +} + +// TestResolveHeaders_FallsBackToEnvVar verifies that when config headers +// are empty, the OTEL_EXPORTER_OTLP_HEADERS env var is used as a fallback. +func TestResolveHeaders_FallsBackToEnvVar(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-token,X-Custom=value") + + cfg := &config.TracingConfig{ + Headers: "", + } + headers := resolveHeaders(cfg) + require.NotNil(t, headers) + assert.Equal(t, "Bearer env-token", headers["Authorization"]) + assert.Equal(t, "value", headers["X-Custom"]) +} + +// TestResolveHeaders_NilConfig_FallsBackToEnvVar verifies env var fallback +// when the TracingConfig itself is nil. +func TestResolveHeaders_NilConfig_FallsBackToEnvVar(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-token") + + headers := resolveHeaders(nil) + require.NotNil(t, headers) + assert.Equal(t, "Bearer env-token", headers["Authorization"]) +} + +// TestResolveHeaders_NoConfigNoEnvVar returns nil when neither config +// nor env var provides headers. +func TestResolveHeaders_NoConfigNoEnvVar(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "") + + cfg := &config.TracingConfig{} + headers := resolveHeaders(cfg) + assert.Nil(t, headers) +} diff --git a/internal/tracing/provider.go b/internal/tracing/provider.go index 6179bb01c..61f55e621 100644 --- a/internal/tracing/provider.go +++ b/internal/tracing/provider.go @@ -22,6 +22,7 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "os" "strings" "time" @@ -122,11 +123,24 @@ func parseOTLPHeaders(raw string) map[string]string { } // resolveHeaders parses the configured OTLP export headers string (or returns nil). +// When no headers are configured via config, it falls back to the standard +// OTEL_EXPORTER_OTLP_HEADERS environment variable (W3C Baggage format: +// "key1=value1,key2=value2") per the OTel OTLP Exporter specification. func resolveHeaders(cfg *config.TracingConfig) map[string]string { - if cfg == nil || cfg.Headers == "" { + raw := "" + if cfg != nil { + raw = cfg.Headers + } + if raw == "" { + raw = os.Getenv("OTEL_EXPORTER_OTLP_HEADERS") + if raw != "" { + logTracing.Printf("Using OTEL_EXPORTER_OTLP_HEADERS env var for OTLP export headers") + } + } + if raw == "" { return nil } - return parseOTLPHeaders(cfg.Headers) + return parseOTLPHeaders(raw) } // resolveParentContext builds a context carrying the W3C remote parent span context From 22eac5c42a5c55078fd3f41cd9966027e6ef5b54 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 16:04:12 +0000 Subject: [PATCH 2/2] fix OTLP header env fallback decoding --- docs/ENVIRONMENT_VARIABLES.md | 4 ++-- internal/tracing/parse_headers_test.go | 16 ++++++++++++++-- internal/tracing/provider.go | 19 ++++++++++++++++++- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/docs/ENVIRONMENT_VARIABLES.md b/docs/ENVIRONMENT_VARIABLES.md index 8c8a30fc3..e77ed4665 100644 --- a/docs/ENVIRONMENT_VARIABLES.md +++ b/docs/ENVIRONMENT_VARIABLES.md @@ -107,10 +107,10 @@ These environment variables configure guard policies (e.g., AllowOnly policies f ## OpenTelemetry / Tracing Variables -These standard OpenTelemetry environment variables set defaults for the corresponding `--otlp-*` CLI flags: +These standard OpenTelemetry environment variables configure tracing. `OTEL_EXPORTER_OTLP_ENDPOINT` and `OTEL_SERVICE_NAME` set defaults for the corresponding `--otlp-*` CLI flags; `OTEL_EXPORTER_OTLP_HEADERS` is used as a fallback when config headers are unset. | Variable | Description | Default | |----------|-------------|---------| | `OTEL_EXPORTER_OTLP_ENDPOINT` | OTLP HTTP endpoint for trace export (e.g., `http://localhost:4318`). Tracing is disabled when empty. Sets default for `--otlp-endpoint`. | (disabled) | -| `OTEL_EXPORTER_OTLP_HEADERS` | Comma-separated `key=value` HTTP headers for OTLP export requests (W3C Baggage format, e.g., `Authorization=Bearer token,X-Custom=value`). Used as fallback when `gateway.opentelemetry.headers` / `gateway.tracing.headers` is not set in config. | (none) | +| `OTEL_EXPORTER_OTLP_HEADERS` | Comma-separated `key=value` HTTP headers for OTLP export requests (W3C Baggage format, e.g., `Authorization=Bearer%20token,X-Custom=value`). Used as fallback when `gateway.opentelemetry.headers` / `gateway.tracing.headers` is not set in config. | (none) | | `OTEL_SERVICE_NAME` | Service name reported in traces. Sets default for `--otlp-service-name`. | `mcp-gateway` | diff --git a/internal/tracing/parse_headers_test.go b/internal/tracing/parse_headers_test.go index 2cb7e01cb..af82fc6af 100644 --- a/internal/tracing/parse_headers_test.go +++ b/internal/tracing/parse_headers_test.go @@ -107,7 +107,7 @@ func TestResolveHeaders_ConfigTakesPrecedence(t *testing.T) { // TestResolveHeaders_FallsBackToEnvVar verifies that when config headers // are empty, the OTEL_EXPORTER_OTLP_HEADERS env var is used as a fallback. func TestResolveHeaders_FallsBackToEnvVar(t *testing.T) { - t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-token,X-Custom=value") + t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer%20env-token,X-Custom=value") cfg := &config.TracingConfig{ Headers: "", @@ -121,13 +121,25 @@ func TestResolveHeaders_FallsBackToEnvVar(t *testing.T) { // TestResolveHeaders_NilConfig_FallsBackToEnvVar verifies env var fallback // when the TracingConfig itself is nil. func TestResolveHeaders_NilConfig_FallsBackToEnvVar(t *testing.T) { - t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-token") + t.Setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer%20env-token") headers := resolveHeaders(nil) require.NotNil(t, headers) assert.Equal(t, "Bearer env-token", headers["Authorization"]) } +// TestResolveHeaders_ConfigPreservesLiteralValue verifies that config headers +// are parsed as literal header values rather than W3C-baggage-decoded values. +func TestResolveHeaders_ConfigPreservesLiteralValue(t *testing.T) { + cfg := &config.TracingConfig{ + Headers: "Authorization=Bearer%20config-token", + } + + headers := resolveHeaders(cfg) + require.NotNil(t, headers) + assert.Equal(t, "Bearer%20config-token", headers["Authorization"]) +} + // TestResolveHeaders_NoConfigNoEnvVar returns nil when neither config // nor env var provides headers. func TestResolveHeaders_NoConfigNoEnvVar(t *testing.T) { diff --git a/internal/tracing/provider.go b/internal/tracing/provider.go index 61f55e621..db847aa94 100644 --- a/internal/tracing/provider.go +++ b/internal/tracing/provider.go @@ -22,6 +22,7 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "net/url" "os" "strings" "time" @@ -101,6 +102,10 @@ func resolveSampleRate(cfg *config.TracingConfig) float64 { // warnings and skipped to avoid invalid HTTP header field names. // Leading/trailing whitespace around keys and values is trimmed. func parseOTLPHeaders(raw string) map[string]string { + return parseOTLPHeadersWithDecoder(raw, false) +} + +func parseOTLPHeadersWithDecoder(raw string, decodeValues bool) map[string]string { headers := make(map[string]string) for _, pair := range strings.Split(raw, ",") { trimmed := strings.TrimSpace(pair) @@ -117,7 +122,16 @@ func parseOTLPHeaders(raw string) map[string]string { logTracing.Printf("Warning: skipping OTLP header pair with empty key") continue } - headers[key] = strings.TrimSpace(v) + value := strings.TrimSpace(v) + if decodeValues { + decoded, err := url.PathUnescape(value) + if err != nil { + logTracing.Printf("Warning: invalid percent-encoding in OTLP header value for key %q; using raw value", key) + } else { + value = decoded + } + } + headers[key] = value } return headers } @@ -140,6 +154,9 @@ func resolveHeaders(cfg *config.TracingConfig) map[string]string { if raw == "" { return nil } + if cfg == nil || cfg.Headers == "" { + return parseOTLPHeadersWithDecoder(raw, true) + } return parseOTLPHeaders(raw) }