Skip to content

Commit a7a7888

Browse files
Copilotpelikhan
andauthored
fix: emit OTLP headers deprecation warning once via WorkflowData.OTLPHeaders
- extractOTLPConfigFromRaw now returns a deprecated bool flag instead of printing the warning inline (pure helper, no side-effects) - injectOTLPConfig collects the deprecated flag from both extraction paths (raw frontmatter and ParsedFrontmatter fallback) and emits the warning exactly once - Add WorkflowData.OTLPHeaders field; injectOTLPConfig sets it alongside OTLPEndpoint so downstream code has a single source of truth - buildMCPGatewayConfig now reads workflowData.OTLPHeaders directly, eliminating the second call to extractOTLPConfigFromRaw and the duplicate deprecation warning it caused - Update tests: extend TestExtractOTLPConfigFromRaw to assert the deprecated return value; add TestInjectOTLPConfig_OTLPHeadersField Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9d7b90fb-97a7-4d37-81ea-71daf6c9e0e5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent e302f87 commit a7a7888

4 files changed

Lines changed: 108 additions & 50 deletions

File tree

pkg/workflow/compiler_types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ type WorkflowData struct {
491491
ParsedFrontmatter *FrontmatterConfig // cached parsed frontmatter configuration (for performance optimization)
492492
RawFrontmatter map[string]any // raw parsed frontmatter map (for passing to hash functions without re-parsing)
493493
OTLPEndpoint string // resolved OTLP endpoint (from observability.otlp.endpoint, including imports; set by injectOTLPConfig)
494+
OTLPHeaders string // normalized OTLP headers in key=value,key=value format (from observability.otlp.headers, including imports; set by injectOTLPConfig)
494495
ResolvedMCPServers map[string]any // fully merged mcp-servers from main workflow and all imports (for mcp inspect)
495496
ActionPinWarnings map[string]bool // cache of already-warned action pin failures (key: "repo@version")
496497
ActionMode ActionMode // action mode for workflow compilation (dev, release, script)

pkg/workflow/mcp_gateway_config.go

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -131,20 +131,9 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig
131131
// Use ${...} syntax for environment variable references that will be resolved by the gateway at runtime
132132
// Per MCP Gateway Specification v1.0.0 section 4.2, variable expressions use "${VARIABLE_NAME}" syntax
133133
//
134-
// OTLPEndpoint and OTLPHeaders are derived from workflowData.OTLPEndpoint and the raw
135-
// frontmatter headers string. These compile-time values (including GitHub Actions
136-
// expressions such as ${{ secrets.X }}) are written directly into the gateway config JSON.
137-
var otlpHeaders string
138-
if workflowData.OTLPEndpoint != "" {
139-
// Read headers from raw frontmatter (same source as injectOTLPConfig)
140-
_, otlpHeaders = extractOTLPConfigFromRaw(workflowData.RawFrontmatter)
141-
if otlpHeaders == "" && workflowData.ParsedFrontmatter != nil &&
142-
workflowData.ParsedFrontmatter.Observability != nil &&
143-
workflowData.ParsedFrontmatter.Observability.OTLP != nil {
144-
normalized, _ := normalizeOTLPHeaders(workflowData.ParsedFrontmatter.Observability.OTLP.Headers)
145-
otlpHeaders = normalized
146-
}
147-
}
134+
// OTLPEndpoint and OTLPHeaders are read from workflowData fields set by injectOTLPConfig.
135+
// These compile-time values (including GitHub Actions expressions such as ${{ secrets.X }})
136+
// are written directly into the gateway config JSON.
148137
return &MCPGatewayRuntimeConfig{
149138
Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer
150139
Domain: "${MCP_GATEWAY_DOMAIN}", // Gateway variable expression
@@ -154,12 +143,11 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig
154143
PayloadSizeThreshold: payloadSizeThreshold, // Size threshold in bytes
155144
TrustedBots: workflowData.SandboxConfig.MCP.TrustedBots, // Additional trusted bot identities from frontmatter
156145
KeepaliveInterval: workflowData.SandboxConfig.MCP.KeepaliveInterval, // Keepalive interval from frontmatter (0=default, -1=disabled, >0=custom)
157-
// OTLPEndpoint and OTLPHeaders are set from workflowData.OTLPEndpoint which is the
158-
// fully resolved OTLP endpoint (including imports) set by injectOTLPConfig. Using
159-
// these fields ensures gateway OTLP config honours observability defined in imported
160-
// shared workflows.
146+
// OTLPEndpoint and OTLPHeaders are set from workflowData by injectOTLPConfig, which is
147+
// the fully resolved OTLP config (including imports). Using these fields ensures gateway
148+
// OTLP config honours observability defined in imported shared workflows.
161149
OTLPEndpoint: workflowData.OTLPEndpoint,
162-
OTLPHeaders: otlpHeaders,
150+
OTLPHeaders: workflowData.OTLPHeaders,
163151
}
164152
}
165153

pkg/workflow/observability_otlp.go

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,10 @@ func generateOTLPHeadersMaskStep() string {
145145
// succeeding -- that function may fail for workflows with complex tool configurations
146146
// (e.g. engine objects, array-style bash configs), which would leave ParsedFrontmatter
147147
// nil and prevent OTLP injection.
148-
func extractOTLPConfigFromRaw(frontmatter map[string]any) (endpoint, headers string) {
148+
//
149+
// The third return value is true when the deprecated string form was used for headers,
150+
// so the caller can emit a deprecation warning.
151+
func extractOTLPConfigFromRaw(frontmatter map[string]any) (endpoint, headers string, deprecated bool) {
149152
obs, ok := frontmatter["observability"]
150153
if !ok {
151154
return
@@ -166,13 +169,7 @@ func extractOTLPConfigFromRaw(frontmatter map[string]any) (endpoint, headers str
166169
endpoint = ep
167170
}
168171
if raw, ok := otlpMap["headers"]; ok {
169-
normalized, deprecated := normalizeOTLPHeaders(raw)
170-
if deprecated {
171-
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(
172-
"observability.otlp.headers: string form is deprecated. Use the map form instead (e.g. headers: {Authorization: \"Bearer ${{ secrets.TOKEN }}\"})",
173-
))
174-
}
175-
headers = normalized
172+
headers, deprecated = normalizeOTLPHeaders(raw)
176173
}
177174
return
178175
}
@@ -191,7 +188,7 @@ func extractOTLPConfigFromRaw(frontmatter map[string]any) (endpoint, headers str
191188
func (c *Compiler) injectOTLPConfig(workflowData *WorkflowData) {
192189
// Read OTLP config from the raw frontmatter map so that injection works even
193190
// when ParseFrontmatterConfig failed (e.g. due to complex tool configs).
194-
endpoint, headers := extractOTLPConfigFromRaw(workflowData.RawFrontmatter)
191+
endpoint, headers, deprecated := extractOTLPConfigFromRaw(workflowData.RawFrontmatter)
195192

196193
// Fall back to ParsedFrontmatter when the raw map didn't yield an endpoint.
197194
if endpoint == "" {
@@ -221,14 +218,20 @@ func (c *Compiler) injectOTLPConfig(workflowData *WorkflowData) {
221218
if headers == "" && workflowData.ParsedFrontmatter != nil &&
222219
workflowData.ParsedFrontmatter.Observability != nil &&
223220
workflowData.ParsedFrontmatter.Observability.OTLP != nil {
224-
normalized, deprecated := normalizeOTLPHeaders(workflowData.ParsedFrontmatter.Observability.OTLP.Headers)
225-
if deprecated {
226-
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(
227-
"observability.otlp.headers: string form is deprecated. Use the map form instead (e.g. headers: {Authorization: \"Bearer ${{ secrets.TOKEN }}\"})",
228-
))
221+
var parsedDeprecated bool
222+
headers, parsedDeprecated = normalizeOTLPHeaders(workflowData.ParsedFrontmatter.Observability.OTLP.Headers)
223+
if parsedDeprecated {
224+
deprecated = true
229225
}
230-
headers = normalized
231226
}
227+
228+
// Emit the deprecation warning once after resolving headers from all sources.
229+
if deprecated {
230+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(
231+
"observability.otlp.headers: string form is deprecated. Use the map form instead (e.g. headers: {Authorization: \"Bearer ${{ secrets.TOKEN }}\"})",
232+
))
233+
}
234+
232235
if headers != "" {
233236
otlpEnvLines += "\n OTEL_EXPORTER_OTLP_HEADERS: " + headers
234237
otlpLog.Printf("Injected OTEL_EXPORTER_OTLP_HEADERS env var")
@@ -241,8 +244,9 @@ func (c *Compiler) injectOTLPConfig(workflowData *WorkflowData) {
241244
}
242245
otlpLog.Printf("Injected OTEL env vars into workflow env block")
243246

244-
// Store the resolved endpoint so downstream code (mcp_gateway_config, mcp_setup_generator)
245-
// can use workflowData.OTLPEndpoint as the single source of truth instead of
246-
// re-reading raw frontmatter independently.
247+
// Store the resolved endpoint and headers so downstream code (mcp_gateway_config,
248+
// mcp_setup_generator) can use workflowData.OTLPEndpoint / OTLPHeaders as the single
249+
// source of truth instead of re-reading raw frontmatter independently.
247250
workflowData.OTLPEndpoint = endpoint
251+
workflowData.OTLPHeaders = headers
248252
}

pkg/workflow/observability_otlp_test.go

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -406,10 +406,11 @@ func TestObservabilityConfigParsing(t *testing.T) {
406406
// TestExtractOTLPConfigFromRaw verifies direct raw-frontmatter OTLP extraction.
407407
func TestExtractOTLPConfigFromRaw(t *testing.T) {
408408
tests := []struct {
409-
name string
410-
frontmatter map[string]any
411-
wantEndpoint string
412-
wantHeaders string
409+
name string
410+
frontmatter map[string]any
411+
wantEndpoint string
412+
wantHeaders string
413+
wantDeprecated bool
413414
}{
414415
{
415416
name: "nil frontmatter",
@@ -446,7 +447,7 @@ func TestExtractOTLPConfigFromRaw(t *testing.T) {
446447
wantEndpoint: "${{ secrets.GH_AW_OTEL_ENDPOINT }}",
447448
},
448449
{
449-
name: "observability.otlp with endpoint and headers",
450+
name: "observability.otlp with endpoint and string headers (deprecated)",
450451
frontmatter: map[string]any{
451452
"observability": map[string]any{
452453
"otlp": map[string]any{
@@ -455,11 +456,12 @@ func TestExtractOTLPConfigFromRaw(t *testing.T) {
455456
},
456457
},
457458
},
458-
wantEndpoint: "https://traces.example.com",
459-
wantHeaders: "${{ secrets.GH_AW_OTEL_HEADERS }}",
459+
wantEndpoint: "https://traces.example.com",
460+
wantHeaders: "${{ secrets.GH_AW_OTEL_HEADERS }}",
461+
wantDeprecated: true,
460462
},
461463
{
462-
name: "Sentry-style header with space in value",
464+
name: "Sentry-style header with space in value (deprecated string form)",
463465
frontmatter: map[string]any{
464466
"observability": map[string]any{
465467
"otlp": map[string]any{
@@ -468,16 +470,32 @@ func TestExtractOTLPConfigFromRaw(t *testing.T) {
468470
},
469471
},
470472
},
471-
wantEndpoint: "https://sentry.io/api/123/envelope/",
472-
wantHeaders: "x-sentry-auth=Sentry sentry_key=abc123",
473+
wantEndpoint: "https://sentry.io/api/123/envelope/",
474+
wantHeaders: "x-sentry-auth=Sentry sentry_key=abc123",
475+
wantDeprecated: true,
476+
},
477+
{
478+
name: "observability.otlp with endpoint and map headers (not deprecated)",
479+
frontmatter: map[string]any{
480+
"observability": map[string]any{
481+
"otlp": map[string]any{
482+
"endpoint": "https://traces.example.com",
483+
"headers": map[string]any{"Authorization": "Bearer tok"},
484+
},
485+
},
486+
},
487+
wantEndpoint: "https://traces.example.com",
488+
wantHeaders: "Authorization=Bearer tok",
489+
wantDeprecated: false,
473490
},
474491
}
475492

476493
for _, tt := range tests {
477494
t.Run(tt.name, func(t *testing.T) {
478-
gotEndpoint, gotHeaders := extractOTLPConfigFromRaw(tt.frontmatter)
495+
gotEndpoint, gotHeaders, gotDeprecated := extractOTLPConfigFromRaw(tt.frontmatter)
479496
assert.Equal(t, tt.wantEndpoint, gotEndpoint, "endpoint")
480497
assert.Equal(t, tt.wantHeaders, gotHeaders, "headers")
498+
assert.Equal(t, tt.wantDeprecated, gotDeprecated, "deprecated")
481499
})
482500
}
483501
}
@@ -668,6 +686,54 @@ func TestInjectOTLPConfig_OTLPEndpointField(t *testing.T) {
668686
})
669687
}
670688

689+
// TestInjectOTLPConfig_OTLPHeadersField verifies that injectOTLPConfig sets workflowData.OTLPHeaders
690+
// so that buildMCPGatewayConfig can read it directly instead of re-reading raw frontmatter.
691+
func TestInjectOTLPConfig_OTLPHeadersField(t *testing.T) {
692+
c := &Compiler{}
693+
694+
t.Run("sets OTLPHeaders when headers are configured (map form)", func(t *testing.T) {
695+
wd := &WorkflowData{
696+
RawFrontmatter: map[string]any{
697+
"observability": map[string]any{
698+
"otlp": map[string]any{
699+
"endpoint": "https://traces.example.com",
700+
"headers": map[string]any{"Authorization": "Bearer tok", "X-Tenant": "acme"},
701+
},
702+
},
703+
},
704+
}
705+
c.injectOTLPConfig(wd)
706+
assert.Equal(t, "Authorization=Bearer tok,X-Tenant=acme", wd.OTLPHeaders, "OTLPHeaders should be set from map form")
707+
})
708+
709+
t.Run("sets OTLPHeaders when headers are configured (string form)", func(t *testing.T) {
710+
wd := &WorkflowData{
711+
RawFrontmatter: map[string]any{
712+
"observability": map[string]any{
713+
"otlp": map[string]any{
714+
"endpoint": "https://traces.example.com",
715+
"headers": "Authorization=Bearer tok",
716+
},
717+
},
718+
},
719+
}
720+
c.injectOTLPConfig(wd)
721+
assert.Equal(t, "Authorization=Bearer tok", wd.OTLPHeaders, "OTLPHeaders should be set from string form")
722+
})
723+
724+
t.Run("OTLPHeaders is empty when no headers are configured", func(t *testing.T) {
725+
wd := &WorkflowData{
726+
RawFrontmatter: map[string]any{
727+
"observability": map[string]any{
728+
"otlp": map[string]any{"endpoint": "https://traces.example.com"},
729+
},
730+
},
731+
}
732+
c.injectOTLPConfig(wd)
733+
assert.Empty(t, wd.OTLPHeaders, "OTLPHeaders should be empty when no headers are configured")
734+
})
735+
}
736+
671737
// TestNormalizeOTLPHeaders verifies the normalizeOTLPHeaders helper function.
672738
func TestNormalizeOTLPHeaders(t *testing.T) {
673739
tests := []struct {
@@ -869,14 +935,13 @@ func TestExtractOTLPConfigFromRaw_MapHeaders(t *testing.T) {
869935

870936
for _, tt := range tests {
871937
t.Run(tt.name, func(t *testing.T) {
872-
gotEndpoint, gotHeaders := extractOTLPConfigFromRaw(tt.frontmatter)
938+
gotEndpoint, gotHeaders, _ := extractOTLPConfigFromRaw(tt.frontmatter)
873939
assert.Equal(t, tt.wantEndpoint, gotEndpoint, "endpoint")
874940
assert.Equal(t, tt.wantHeaders, gotHeaders, "headers")
875941
})
876942
}
877943
}
878944

879-
// TestObservabilityConfigParsing_MapHeaders verifies that the map form for headers is
880945
// correctly parsed by ParseFrontmatterConfig.
881946
func TestObservabilityConfigParsing_MapHeaders(t *testing.T) {
882947
t.Run("map headers parsed as any type", func(t *testing.T) {

0 commit comments

Comments
 (0)