Skip to content
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/open-telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ When `observability.otlp` is configured, gh-aw injects:
| Variable | Description |
| --- | --- |
| `OTEL_EXPORTER_OTLP_HEADERS` | Comma-separated `key=value` headers for the first endpoint (when headers are configured). |
| `OTEL_SERVICE_NAME` | Always `gh-aw`. |
| `OTEL_SERVICE_NAME` | `gh-aw.<sanitized-workflow-id>` when `WorkflowID` is available (for example, `Repo Triage/Weekly` → `gh-aw.repo-triage-weekly`); falls back to sanitized workflow name when only the name is available, otherwise `gh-aw`. |
| `GH_AW_OTLP_ENDPOINTS` | JSON array of all endpoint entries, used by gh-aw JavaScript span exporters for fan-out. |
| `GH_AW_OTLP_IF_MISSING` | Set to `warn` or `ignore` when `observability.otlp.if-missing` is configured. |
| `COPILOT_OTEL_FILE_EXPORTER_PATH` | Path for Copilot CLI span output (`/tmp/gh-aw/copilot-otel.jsonl`). |
Expand Down
33 changes: 30 additions & 3 deletions pkg/workflow/observability_otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,12 +399,14 @@ func (c *Compiler) injectOTLPConfig(workflowData *WorkflowData) {

firstEndpoint := entries[0].URL
firstHeaders := entries[0].Headers
serviceName := otelServiceName(workflowData)
ifMissingMode := getOTLPIfMissingMode(workflowData.ParsedFrontmatter, workflowData.RawFrontmatter)

// 2. Inject OTEL env vars into the workflow-level env: block.
// OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_SERVICE_NAME are set to the first
// endpoint for backward compatibility (MCP gateway, legacy scripts).
otlpEnvLines := fmt.Sprintf(" OTEL_EXPORTER_OTLP_ENDPOINT: %s\n OTEL_SERVICE_NAME: gh-aw", firstEndpoint)
// OTEL_EXPORTER_OTLP_ENDPOINT is set to the first endpoint for backward
// compatibility (MCP gateway, legacy scripts). OTEL_SERVICE_NAME is
// workflow-specific when WorkflowID is available.
otlpEnvLines := fmt.Sprintf(" OTEL_EXPORTER_OTLP_ENDPOINT: %s\n OTEL_SERVICE_NAME: %s", firstEndpoint, serviceName)

// 3. Inject per-endpoint headers env vars.
// OTEL_EXPORTER_OTLP_HEADERS = first endpoint headers (backward compat).
Expand Down Expand Up @@ -444,3 +446,28 @@ func (c *Compiler) injectOTLPConfig(workflowData *WorkflowData) {
workflowData.OTLPHeaders = firstHeaders
workflowData.OTLPEndpoints = encodeOTLPEndpoints(entries)
}

func otelServiceName(workflowData *WorkflowData) string {
const defaultServiceName = "gh-aw"
if workflowData == nil {
return defaultServiceName
}

// Prefer the file-based WorkflowID to avoid collisions across workflows that
// may share display names; fall back to workflow Name when WorkflowID is
// unavailable (for workflow_call-only contexts).
workflowIDOrName := strings.TrimSpace(workflowData.WorkflowID)
if workflowIDOrName == "" {
workflowIDOrName = workflowData.Name
}

// SanitizeWorkflowName lowercases the workflow identifier and converts
// separators/special characters (spaces, slashes, etc.) to hyphens so the
// service suffix is stable and backend-friendly.
sanitizedWorkflowName := SanitizeWorkflowName(workflowIDOrName)
if sanitizedWorkflowName == "" {
return defaultServiceName
}

return defaultServiceName + "." + sanitizedWorkflowName
}
35 changes: 33 additions & 2 deletions pkg/workflow/observability_otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,17 +358,18 @@ func TestInjectOTLPConfig(t *testing.T) {
assert.Equal(t, 1, strings.Count(wd.Env, "env:"), "should have exactly one env: key")
})

t.Run("OTEL_SERVICE_NAME is always gh-aw", func(t *testing.T) {
t.Run("OTEL_SERVICE_NAME includes sanitized workflow ID when available", func(t *testing.T) {
c := newCompiler()
wd := &WorkflowData{
WorkflowID: "Repo Triage/Weekly",
ParsedFrontmatter: &FrontmatterConfig{
Observability: &ObservabilityConfig{
OTLP: &OTLPConfig{Endpoint: "https://otel.corp.com"},
},
},
}
c.injectOTLPConfig(wd)
assert.Contains(t, wd.Env, "OTEL_SERVICE_NAME: gh-aw", "service name should always be gh-aw")
assert.Contains(t, wd.Env, "OTEL_SERVICE_NAME: gh-aw.repo-triage-weekly", "service name should include sanitized workflow ID")
})

t.Run("injects OTEL_EXPORTER_OTLP_HEADERS when headers are configured", func(t *testing.T) {
Expand Down Expand Up @@ -659,6 +660,36 @@ func TestInjectOTLPConfig_HeadersPresenceAfterInjection(t *testing.T) {
})
}

func TestOTELServiceName(t *testing.T) {
t.Run("uses workflow-specific service name when workflow ID is present", func(t *testing.T) {
got := otelServiceName(&WorkflowData{WorkflowID: "Repo Triage/Weekly"})
assert.Equal(t, "gh-aw.repo-triage-weekly", got, "should use WorkflowID as service name suffix when present")
})

t.Run("falls back to workflow name when workflow ID is empty", func(t *testing.T) {
got := otelServiceName(&WorkflowData{Name: "Repo Triage/Weekly"})
assert.Equal(t, "gh-aw.repo-triage-weekly", got, "should fall back to workflow name when WorkflowID is empty")
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test value WorkflowID: "Repo Triage/Weekly" doesn't reflect what WorkflowID actually looks like at runtime — GetWorkflowIDFromPath always returns a plain basename without extension (e.g. "repo-triage"), never a slash-separated display name.

Using a realistic value would make this test a better specification and avoid the false implication that WorkflowID can contain slashes or spaces:

t.Run("falls back to workflow ID when workflow name is empty", func(t *testing.T) {
    got := otelServiceName(&WorkflowData{WorkflowID: "repo-triage"})
    assert.Equal(t, "gh-aw.repo-triage", got)
})

The current value still exercises SanitizeWorkflowName, but it conflates the WorkflowID and Name concepts in a way that could mislead future readers.

t.Run("workflow ID takes precedence over workflow name", func(t *testing.T) {
got := otelServiceName(&WorkflowData{
WorkflowID: "Unique Workflow ID",
Name: "Shared Display Name",
})
assert.Equal(t, "gh-aw.unique-workflow-id", got, "should prefer WorkflowID over workflow name when both are present")
})

t.Run("falls back when workflow ID and name are empty", func(t *testing.T) {
got := otelServiceName(&WorkflowData{})
assert.Equal(t, "gh-aw", got, "should return default service name when WorkflowID and name are empty")
})

t.Run("falls back when workflow data is nil", func(t *testing.T) {
got := otelServiceName(nil)
assert.Equal(t, "gh-aw", got, "should return default service name when workflow data is nil")
})
}

// TestInjectOTLPConfig_OTLPEndpointField verifies that injectOTLPConfig sets workflowData.OTLPEndpoint
// so that downstream code (buildMCPGatewayConfig, mcp_setup_generator) can use it as the
// single source of truth for "is OTLP configured?" without re-reading raw frontmatter.
Expand Down
Loading