diff --git a/.github/aw/github-agentic-workflows.md b/.github/aw/github-agentic-workflows.md index 0b6607fb98..8154b7e566 100644 --- a/.github/aw/github-agentic-workflows.md +++ b/.github/aw/github-agentic-workflows.md @@ -303,7 +303,6 @@ The YAML frontmatter supports these fields: - **`otlp:`** - Export OpenTelemetry spans to any OTLP-compatible backend (Honeycomb, Grafana Tempo, Sentry, etc.) (object) - `endpoint:` - OTLP collector endpoint URL. When a static URL is provided, its hostname is added to the AWF firewall allowlist automatically. Supports GitHub Actions expressions. - `headers:` - Comma-separated `key=value` HTTP headers included in every OTLP export request (e.g. `Authorization=Bearer `). Injected as `OTEL_EXPORTER_OTLP_HEADERS`. Supports GitHub Actions expressions. - - **`job-summary:`** - Control job summary output mode (`"on"` or `"off"`) - Example: ```yaml diff --git a/actions/setup/js/generate_observability_summary.cjs b/actions/setup/js/generate_observability_summary.cjs index b9ab440f92..dadacc872f 100644 --- a/actions/setup/js/generate_observability_summary.cjs +++ b/actions/setup/js/generate_observability_summary.cjs @@ -114,12 +114,6 @@ function buildObservabilitySummary(data) { } async function main(core) { - const mode = process.env.GH_AW_OBSERVABILITY_JOB_SUMMARY || ""; - if (mode !== "on") { - core.info(`Skipping observability summary: mode=${mode || "unset"}`); - return; - } - const data = collectObservabilityData(); const markdown = buildObservabilitySummary(data); await core.summary.addRaw(markdown).write(); diff --git a/actions/setup/js/generate_observability_summary.test.cjs b/actions/setup/js/generate_observability_summary.test.cjs index 5121761050..0ec81aa5a4 100644 --- a/actions/setup/js/generate_observability_summary.test.cjs +++ b/actions/setup/js/generate_observability_summary.test.cjs @@ -17,12 +17,10 @@ describe("generate_observability_summary.cjs", () => { beforeEach(async () => { vi.clearAllMocks(); fs.mkdirSync("/tmp/gh-aw/mcp-logs", { recursive: true }); - process.env.GH_AW_OBSERVABILITY_JOB_SUMMARY = "on"; module = await import("./generate_observability_summary.cjs"); }); afterEach(() => { - delete process.env.GH_AW_OBSERVABILITY_JOB_SUMMARY; for (const path of ["/tmp/gh-aw/aw_info.json", "/tmp/gh-aw/agent_output.json", "/tmp/gh-aw/mcp-logs/gateway.jsonl", "/tmp/gh-aw/mcp-logs/rpc-messages.jsonl"]) { if (fs.existsSync(path)) { fs.unlinkSync(path); @@ -87,13 +85,10 @@ describe("generate_observability_summary.cjs", () => { expect(summary).toContain("- **trace id**: 12345678901-1"); }); - it("skips summary generation when opt-in mode is disabled", async () => { - process.env.GH_AW_OBSERVABILITY_JOB_SUMMARY = "off"; - + it("always generates summary regardless of env vars", async () => { await module.main(mockCore); - expect(mockCore.summary.addRaw).not.toHaveBeenCalled(); - expect(mockCore.summary.write).not.toHaveBeenCalled(); - expect(mockCore.info).toHaveBeenCalledWith("Skipping observability summary: mode=off"); + expect(mockCore.summary.addRaw).toHaveBeenCalledTimes(1); + expect(mockCore.summary.write).toHaveBeenCalledTimes(1); }); }); diff --git a/docs/public/editor/autocomplete-data.json b/docs/public/editor/autocomplete-data.json index 59873dc88e..6d505b7944 100644 --- a/docs/public/editor/autocomplete-data.json +++ b/docs/public/editor/autocomplete-data.json @@ -1646,14 +1646,7 @@ "observability": { "type": "object", "desc": "Optional observability output settings for workflow runs.", - "children": { - "job-summary": { - "type": "string", - "desc": "If set to 'on', append a compact observability section to the GitHub Actions job summary.", - "enum": ["on", "off"], - "leaf": true - } - } + "children": {} }, "bots": { "type": "array", diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 7c5e30dff8..07613eb29f 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -5905,11 +5905,7 @@ secret-masking: # Optional observability output settings for workflow runs. # (optional) -observability: - # If set to 'on', append a compact observability section to the GitHub Actions job - # summary. Defaults to off when omitted. - # (optional) - job-summary: "on" +observability: {} # Allow list of bot identifiers that can trigger the workflow even if they don't # meet the required role permissions. When the actor is in this list, the bot must diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index 82c91b5d49..30ed258fd9 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -30,6 +30,7 @@ type importAccumulator struct { secretMaskingBuilder strings.Builder postStepsBuilder strings.Builder jobsBuilder strings.Builder // Jobs from imported YAML workflows + observabilityBuilder strings.Builder // observability config (first-wins for OTLP endpoint) engines []string safeOutputs []string mcpScripts []string @@ -350,6 +351,17 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } } + // Extract observability from imported file (first-wins: only set if not yet populated). + // This ensures an imported workflow's OTLP config is visible to injectOTLPConfig even + // when the main workflow's frontmatter does not contain an observability section. + if acc.observabilityBuilder.Len() == 0 { + obsContent, obsErr := extractFieldJSONFromMap(fm, "observability", "{}") + if obsErr == nil && obsContent != "" && obsContent != "{}" { + acc.observabilityBuilder.WriteString(obsContent) + log.Printf("Extracted observability from import: %s", item.fullPath) + } + } + return nil } @@ -381,6 +393,7 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import MergedCaches: acc.caches, MergedJobs: acc.jobsBuilder.String(), MergedFeatures: acc.features, + MergedObservability: acc.observabilityBuilder.String(), ImportedFiles: topologicalOrder, AgentFile: acc.agentFile, AgentImportSpec: acc.agentImportSpec, diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index edf9a1e45b..cc623d2225 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -39,6 +39,7 @@ type ImportsResult struct { MergedCaches []string // Merged cache configurations from all imports (appended in order) MergedJobs string // Merged jobs from imported YAML workflows (JSON format) MergedFeatures []map[string]any // Merged features configuration from all imports (parsed YAML structures) + MergedObservability string // Observability config (JSON) from first import that defines it (first-wins) ImportedFiles []string // List of imported file paths (for manifest) AgentFile string // Path to custom agent file (if imported) AgentImportSpec string // Original import specification for agent file (e.g., "owner/repo/path@ref") diff --git a/pkg/parser/schema_test.go b/pkg/parser/schema_test.go index 0fe8adf86e..515df3720a 100644 --- a/pkg/parser/schema_test.go +++ b/pkg/parser/schema_test.go @@ -194,25 +194,3 @@ func TestGetSafeOutputTypeKeys(t *testing.T) { } } } - -func TestValidateMainWorkflowFrontmatterWithSchema_AllowsObservabilityJobSummary(t *testing.T) { - frontmatter := map[string]any{ - "on": "push", - "observability": map[string]any{ - "job-summary": "on", - }, - } - - tempFile := "/tmp/gh-aw/test_observability_frontmatter.md" - if err := os.MkdirAll("/tmp/gh-aw", 0755); err != nil { - t.Fatalf("Failed to create temp directory: %v", err) - } - if err := os.WriteFile(tempFile, []byte("---\non: push\nobservability:\n job-summary: on\n---\n"), 0644); err != nil { - t.Fatalf("Failed to create temp file: %v", err) - } - defer os.Remove(tempFile) - - if err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, tempFile); err != nil { - t.Fatalf("Expected observability config to validate, got: %v", err) - } -} diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index ea62464d83..a58382f8ad 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -8324,11 +8324,6 @@ "type": "object", "description": "Optional observability output settings for workflow runs.", "properties": { - "job-summary": { - "type": "string", - "enum": ["on", "off"], - "description": "If set to 'on', append a compact observability section to the GitHub Actions job summary. Defaults to off when omitted." - }, "otlp": { "type": "object", "description": "OTLP (OpenTelemetry Protocol) trace export configuration.", diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 516b8478f6..c8fd0c570e 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -109,6 +109,18 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) // Extract YAML configuration sections from frontmatter c.extractYAMLSections(result.Frontmatter, workflowData) + // Merge observability config from imports into RawFrontmatter so that injectOTLPConfig + // can see an OTLP endpoint defined in an imported workflow (first-wins from imports). + if obs := engineSetup.importsResult.MergedObservability; obs != "" { + if _, hasObs := workflowData.RawFrontmatter["observability"]; !hasObs { + var obsMap map[string]any + if err := json.Unmarshal([]byte(obs), &obsMap); err == nil { + workflowData.RawFrontmatter["observability"] = obsMap + orchestratorWorkflowLog.Printf("Merged observability config from imports into RawFrontmatter") + } + } + } + // Inject OTLP configuration: add endpoint domain to firewall allowlist and // set OTEL env vars in the workflow env block (no-op when not configured). c.injectOTLPConfig(workflowData) diff --git a/pkg/workflow/compiler_yaml_ai_execution.go b/pkg/workflow/compiler_yaml_ai_execution.go index c640cd6555..1148bfc808 100644 --- a/pkg/workflow/compiler_yaml_ai_execution.go +++ b/pkg/workflow/compiler_yaml_ai_execution.go @@ -7,31 +7,6 @@ import ( "github.com/github/gh-aw/pkg/constants" ) -func getObservabilityJobSummaryMode(data *WorkflowData) string { - if data == nil { - return "" - } - - mode := "" - if data.ParsedFrontmatter != nil && data.ParsedFrontmatter.Observability != nil { - mode = data.ParsedFrontmatter.Observability.JobSummary - } - - if mode == "" && data.RawFrontmatter != nil { - if rawObservability, ok := data.RawFrontmatter["observability"].(map[string]any); ok { - if rawMode, ok := rawObservability["job-summary"].(string); ok { - mode = rawMode - } - } - } - - if mode == "off" { - return "" - } - - return mode -} - // generateEngineExecutionSteps generates the GitHub Actions steps for executing the AI engine func (c *Compiler) generateEngineExecutionSteps(yaml *strings.Builder, data *WorkflowData, engine CodingAgentEngine, logFile string) { @@ -119,21 +94,19 @@ func (c *Compiler) generateMCPGatewayLogParsing(yaml *strings.Builder) { yaml.WriteString(" await main();\n") } -// generateObservabilitySummary generates an opt-in step that synthesizes a compact +// generateObservabilitySummary generates a step that synthesizes a compact // observability section for the GitHub Actions step summary from existing runtime files. +// The step is only emitted when OTLP is configured in the workflow. func (c *Compiler) generateObservabilitySummary(yaml *strings.Builder, data *WorkflowData) { - mode := getObservabilityJobSummaryMode(data) - if mode == "" { + if !isOTLPEnabled(data) { return } - compilerYamlLog.Printf("Generating observability step summary: mode=%s", mode) + compilerYamlLog.Print("Generating observability step summary") yaml.WriteString(" - name: Generate observability summary\n") yaml.WriteString(" if: always()\n") fmt.Fprintf(yaml, " uses: %s\n", GetActionPin("actions/github-script")) - yaml.WriteString(" env:\n") - fmt.Fprintf(yaml, " GH_AW_OBSERVABILITY_JOB_SUMMARY: %q\n", mode) yaml.WriteString(" with:\n") yaml.WriteString(" script: |\n") yaml.WriteString(" const { setupGlobals } = require('" + SetupActionDestination + "/setup_globals.cjs');\n") @@ -142,6 +115,17 @@ func (c *Compiler) generateObservabilitySummary(yaml *strings.Builder, data *Wor yaml.WriteString(" await main(core);\n") } +// isOTLPEnabled returns true when OTLP has been configured in the workflow (including +// imported frontmatter). It checks whether injectOTLPConfig has already written the +// OTEL_EXPORTER_OTLP_ENDPOINT env var into workflowData.Env, which is the authoritative +// result of OTLP detection after all frontmatter (main + imports) has been processed. +func isOTLPEnabled(data *WorkflowData) bool { + if data == nil { + return false + } + return strings.Contains(data.Env, "OTEL_EXPORTER_OTLP_ENDPOINT") +} + // generateStopMCPGateway generates a step that stops the MCP gateway process using its PID from step output // It passes the gateway port and API key to enable graceful shutdown via /close endpoint func (c *Compiler) generateStopMCPGateway(yaml *strings.Builder, data *WorkflowData) { diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 1d466596ad..6f52687666 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -443,7 +443,7 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat artifactPaths = append(artifactPaths, "/tmp/gh-aw/"+constants.TokenUsageFilename) } - // Optionally synthesize a compact observability section from runtime artifacts. + // Synthesize a compact observability section from runtime artifacts when OTLP is enabled. c.generateObservabilitySummary(yaml, data) // Collect agent stdio logs path for unified upload diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index 2ae08be7a6..8fc0a0ea23 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -134,8 +134,7 @@ type OTLPConfig struct { // ObservabilityConfig represents workflow observability options. type ObservabilityConfig struct { - JobSummary string `json:"job-summary,omitempty"` - OTLP *OTLPConfig `json:"otlp,omitempty"` + OTLP *OTLPConfig `json:"otlp,omitempty"` } // FrontmatterConfig represents the structured configuration from workflow frontmatter diff --git a/pkg/workflow/frontmatter_types_test.go b/pkg/workflow/frontmatter_types_test.go index 4c0b0c4129..621d71560b 100644 --- a/pkg/workflow/frontmatter_types_test.go +++ b/pkg/workflow/frontmatter_types_test.go @@ -248,7 +248,9 @@ func TestParseFrontmatterConfig(t *testing.T) { t.Run("handles observability configuration", func(t *testing.T) { frontmatter := map[string]any{ "observability": map[string]any{ - "job-summary": "on", + "otlp": map[string]any{ + "endpoint": "https://traces.example.com", + }, }, } @@ -261,8 +263,8 @@ func TestParseFrontmatterConfig(t *testing.T) { t.Fatal("Observability should not be nil") } - if config.Observability.JobSummary != "on" { - t.Errorf("JobSummary = %q, want %q", config.Observability.JobSummary, "on") + if config.Observability.OTLP == nil { + t.Fatal("OTLP should not be nil") } }) diff --git a/pkg/workflow/observability_job_summary_test.go b/pkg/workflow/observability_job_summary_test.go index fd2765e5e8..10b67e64e4 100644 --- a/pkg/workflow/observability_job_summary_test.go +++ b/pkg/workflow/observability_job_summary_test.go @@ -9,7 +9,7 @@ import ( "testing" ) -func TestCompileWorkflow_IncludesObservabilitySummaryStepWhenOptedIn(t *testing.T) { +func TestCompileWorkflow_IncludesObservabilitySummaryStepWhenOTLPEnabled(t *testing.T) { tmpDir := t.TempDir() workflowPath := filepath.Join(tmpDir, "observability-summary.md") content := `--- @@ -17,11 +17,12 @@ on: push permissions: contents: read observability: - job-summary: on + otlp: + endpoint: https://traces.example.com:4317 engine: copilot --- -# Test Observability Summary +# Test Observability Summary with OTLP ` if err := os.WriteFile(workflowPath, []byte(content), 0o644); err != nil { @@ -41,17 +42,14 @@ engine: copilot compiled := string(lockContent) if !strings.Contains(compiled, "- name: Generate observability summary") { - t.Fatal("Expected observability summary step to be generated") - } - if !strings.Contains(compiled, "GH_AW_OBSERVABILITY_JOB_SUMMARY: \"on\"") { - t.Fatal("Expected observability summary mode env var to be set") + t.Fatal("Expected observability summary step to be generated when OTLP is enabled") } if !strings.Contains(compiled, "require('${{ runner.temp }}/gh-aw/actions/generate_observability_summary.cjs')") { t.Fatal("Expected generated workflow to load generate_observability_summary.cjs") } } -func TestCompileWorkflow_DoesNotIncludeObservabilitySummaryStepByDefault(t *testing.T) { +func TestCompileWorkflow_DoesNotIncludeObservabilitySummaryStepWithoutOTLP(t *testing.T) { tmpDir := t.TempDir() workflowPath := filepath.Join(tmpDir, "no-observability-summary.md") content := `--- @@ -79,7 +77,63 @@ engine: copilot t.Fatalf("Failed to read lock file: %v", err) } - if strings.Contains(string(lockContent), "- name: Generate observability summary") { - t.Fatal("Did not expect observability summary step when feature is not configured") + compiled := string(lockContent) + if strings.Contains(compiled, "- name: Generate observability summary") { + t.Fatal("Did not expect observability summary step when OTLP is not configured") + } + if strings.Contains(compiled, "GH_AW_OBSERVABILITY_JOB_SUMMARY") { + t.Fatal("Did not expect GH_AW_OBSERVABILITY_JOB_SUMMARY env var in compiled workflow") + } +} + +func TestCompileWorkflow_IncludesObservabilitySummaryStepWhenOTLPEnabledViaImport(t *testing.T) { + tmpDir := t.TempDir() + + // Create an imported workflow with OTLP configured + importedPath := filepath.Join(tmpDir, "shared-otlp.md") + importedContent := `--- +observability: + otlp: + endpoint: https://traces.example.com:4317 +--- +` + if err := os.WriteFile(importedPath, []byte(importedContent), 0o644); err != nil { + t.Fatalf("Failed to write imported workflow: %v", err) + } + + // Main workflow imports the shared OTLP config but has no observability section itself + workflowPath := filepath.Join(tmpDir, "main-import-otlp.md") + content := `--- +on: push +permissions: + contents: read +engine: copilot +imports: + - ./shared-otlp.md +--- + +# Test Observability Summary via Import +` + if err := os.WriteFile(workflowPath, []byte(content), 0o644); err != nil { + t.Fatalf("Failed to write main workflow: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Unexpected compile error: %v", err) + } + + lockPath := filepath.Join(tmpDir, "main-import-otlp.lock.yml") + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + compiled := string(lockContent) + if !strings.Contains(compiled, "- name: Generate observability summary") { + t.Fatal("Expected observability summary step when OTLP is enabled via import") + } + if !strings.Contains(compiled, "OTEL_EXPORTER_OTLP_ENDPOINT") { + t.Fatal("Expected OTEL_EXPORTER_OTLP_ENDPOINT env var to be injected when OTLP is configured via import") } } diff --git a/pkg/workflow/observability_otlp_test.go b/pkg/workflow/observability_otlp_test.go index 5de9ea9df0..f38b73d10d 100644 --- a/pkg/workflow/observability_otlp_test.go +++ b/pkg/workflow/observability_otlp_test.go @@ -310,12 +310,8 @@ func TestObservabilityConfigParsing(t *testing.T) { wantOTLPConfig: false, }, { - name: "observability without otlp", - frontmatter: map[string]any{ - "observability": map[string]any{ - "job-summary": "on", - }, - }, + name: "observability without otlp", + frontmatter: map[string]any{"observability": map[string]any{}}, wantOTLPConfig: false, }, { @@ -343,10 +339,9 @@ func TestObservabilityConfigParsing(t *testing.T) { expectedEndpoint: "${{ secrets.OTLP_ENDPOINT }}", }, { - name: "observability with both job-summary and otlp", + name: "observability with both otlp endpoint and config", frontmatter: map[string]any{ "observability": map[string]any{ - "job-summary": "on", "otlp": map[string]any{ "endpoint": "https://traces.example.com", }, @@ -427,10 +422,8 @@ func TestExtractOTLPConfigFromRaw(t *testing.T) { frontmatter: map[string]any{"name": "test"}, }, { - name: "observability without otlp", - frontmatter: map[string]any{ - "observability": map[string]any{"job-summary": "on"}, - }, + name: "observability without otlp", + frontmatter: map[string]any{"observability": map[string]any{}}, }, { name: "observability.otlp with endpoint",