From cb6fa70f66221c202188ca19423bacb280c38a94 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 16:12:27 +0000 Subject: [PATCH 1/3] Initial plan From 33ca8104ffc90ad4332fbe25a0c38ba77a8e22f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 16:35:41 +0000 Subject: [PATCH 2/3] Apply remaining changes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../otel_observability_formal_test.go | 245 ++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 pkg/workflow/otel_observability_formal_test.go diff --git a/pkg/workflow/otel_observability_formal_test.go b/pkg/workflow/otel_observability_formal_test.go new file mode 100644 index 00000000000..cf828a62f64 --- /dev/null +++ b/pkg/workflow/otel_observability_formal_test.go @@ -0,0 +1,245 @@ +package workflow + +import ( + "testing" + + "github.com/github/gh-aw/pkg/constants" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const determinismTestIterations = 10 +const authPlaceholder = "******" + +func TestFormal_EndpointFormNormalization(t *testing.T) { + t.Run("string object and array normalize to ordered entries", func(t *testing.T) { + stringForm := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": "https://string.example.com:4317", + }, + }, + } + assert.Equal(t, + []otlpEndpointEntry{{URL: "https://string.example.com:4317"}}, + collectAllOTLPEndpoints(stringForm), + ) + + objectForm := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": map[string]any{"url": "https://object.example.com:4317"}, + }, + }, + } + assert.Equal(t, + []otlpEndpointEntry{{URL: "https://object.example.com:4317"}}, + collectAllOTLPEndpoints(objectForm), + ) + + arrayForm := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": []any{ + map[string]any{"url": "https://first.example.com:4317"}, + map[string]any{"url": "https://second.example.com:4317"}, + }, + }, + }, + } + assert.Equal(t, + []otlpEndpointEntry{ + {URL: "https://first.example.com:4317"}, + {URL: "https://second.example.com:4317"}, + }, + collectAllOTLPEndpoints(arrayForm), + ) + }) + + t.Run("empty and absent normalize to empty", func(t *testing.T) { + assert.Empty(t, collectAllOTLPEndpoints(nil)) + assert.Empty(t, collectAllOTLPEndpoints(map[string]any{})) + assert.Empty(t, collectAllOTLPEndpoints(map[string]any{"observability": map[string]any{}})) + }) +} + +func TestFormal_HeaderMapDeterminism(t *testing.T) { + headers := map[string]any{"z": "3", "a": "1", "m": "2"} + want := "a=1,m=2,z=3" + + for range determinismTestIterations { + assert.Equal(t, want, normalizeOTLPHeadersForEndpoint(headers, "https://example.com:4317")) + } +} + +func TestFormal_SentryAuthHeaderRewrite(t *testing.T) { + normalizedSentryHeaders := normalizeOTLPHeadersForEndpoint("Authorization="+authPlaceholder, "https://o0.ingest.sentry.io/api/0/envelope/") + normalizedNonSentryHeaders := normalizeOTLPHeadersForEndpoint("Authorization="+authPlaceholder, "https://otlp.example.com:4317") + normalizedSentryMixedHeaders := normalizeOTLPHeadersForEndpoint( + map[string]any{"Authorization": authPlaceholder, "X-Tenant": "acme"}, + "https://o0.ingest.sentry.io/api/0/envelope/", + ) + + assert.Equal(t, "x-sentry-auth="+authPlaceholder, normalizedSentryHeaders) + assert.Equal(t, "Authorization="+authPlaceholder, normalizedNonSentryHeaders) + assert.Equal(t, "x-sentry-auth="+authPlaceholder+",X-Tenant=acme", normalizedSentryMixedHeaders) +} + +func TestFormal_IfMissingPolicyValidation(t *testing.T) { + assert.Equal(t, "error", normalizeOTLPIfMissingMode("error")) + assert.Equal(t, "warn", normalizeOTLPIfMissingMode("WARN")) + assert.Equal(t, "ignore", normalizeOTLPIfMissingMode(" Ignore ")) +} + +func TestFormal_ServiceNameFormation(t *testing.T) { + assert.Equal(t, "gh-aw", otelServiceName(nil)) + assert.Equal(t, "gh-aw.repo-triage-weekly", otelServiceName(&WorkflowData{WorkflowID: "repo-triage-weekly", Name: "Sample Name"})) + assert.Equal(t, "gh-aw.repo-triage-weekly", otelServiceName(&WorkflowData{WorkflowID: "Repo Triage/Weekly", Name: "Sample Name"})) + assert.Equal(t, "gh-aw.workflow-name", otelServiceName(&WorkflowData{Name: "Workflow Name"})) +} + +func TestFormal_StaticDomainExtraction(t *testing.T) { + assert.Equal(t, "traces.example.com", extractOTLPEndpointDomain("https://traces.example.com:4317")) + assert.Empty(t, extractOTLPEndpointDomain("")) + assert.Empty(t, extractOTLPEndpointDomain("${{ secrets.OTLP_ENDPOINT }}")) +} + +func TestFormal_ExpressionProducesNoAllowlistEntry(t *testing.T) { + assert.Empty(t, extractOTLPEndpointDomain("${{ secrets.OTLP_ENDPOINT }}")) +} + +func TestFormal_TopLevelHeadersApplyToStringFormOnly(t *testing.T) { + stringForm := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": "https://string.example.com:4317", + "headers": "Authorization=" + authPlaceholder, + }, + }, + } + entries := collectAllOTLPEndpoints(stringForm) + require.Len(t, entries, 1) + assert.Equal(t, "Authorization="+authPlaceholder, entries[0].Headers) + + objectForm := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": map[string]any{ + "url": "https://object.example.com:4317", + "headers": "X-Per-Entry=v", + }, + "headers": "Authorization=" + authPlaceholder, + }, + }, + } + objectEntries := collectAllOTLPEndpoints(objectForm) + require.Len(t, objectEntries, 1) + assert.Equal(t, "X-Per-Entry=v", objectEntries[0].Headers) +} + +func TestFormal_FanOutPreservesDeclarationOrder(t *testing.T) { + frontmatter := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": []any{ + map[string]any{"url": "https://one.example.com:4317"}, + map[string]any{"url": "https://two.example.com:4317"}, + map[string]any{"url": "https://three.example.com:4317"}, + }, + }, + }, + } + + entries := collectAllOTLPEndpoints(frontmatter) + require.Len(t, entries, 3) + assert.Equal(t, "https://one.example.com:4317", entries[0].URL) + assert.Equal(t, "https://two.example.com:4317", entries[1].URL) + assert.Equal(t, "https://three.example.com:4317", entries[2].URL) +} + +func TestFormal_MirrorPathConstant(t *testing.T) { + assert.Equal(t, "/tmp/gh-aw/otel.jsonl", constants.TmpGhAwDirSlash+constants.OtelJsonlFilename) +} + +func TestFormal_EmptyURLEntriesDiscarded(t *testing.T) { + frontmatter := map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": []any{ + map[string]any{"url": ""}, + map[string]any{"url": "https://valid.example.com:4317"}, + }, + }, + }, + } + + assert.Equal(t, []otlpEndpointEntry{{URL: "https://valid.example.com:4317"}}, collectAllOTLPEndpoints(frontmatter)) +} + +func TestFormal_StringHeaderFormPreservedForNonSentry(t *testing.T) { + assert.Equal(t, + "Authorization="+authPlaceholder, + normalizeOTLPHeadersForEndpoint("Authorization="+authPlaceholder, "https://otlp.example.com:4317"), + ) +} + +func TestFormal_NilAndEmptyHeadersYieldEmptyString(t *testing.T) { + assert.Empty(t, normalizeOTLPHeadersForEndpoint(nil, "https://example.com:4317")) + assert.Empty(t, normalizeOTLPHeadersForEndpoint("", "https://example.com:4317")) + assert.Empty(t, normalizeOTLPHeadersForEndpoint(map[string]any{}, "https://example.com:4317")) +} + +func TestFormal_InvalidIfMissingFallsBackToDefault(t *testing.T) { + for _, mode := range []string{"fail", "silent", "skip", "abort"} { + assert.Empty(t, normalizeOTLPIfMissingMode(mode)) + } + + workflowData := &WorkflowData{ + RawFrontmatter: map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": "https://traces.example.com:4317", + "if-missing": "fail", + }, + }, + }, + ParsedFrontmatter: &FrontmatterConfig{ + Observability: &ObservabilityConfig{ + OTLP: &OTLPConfig{ + Endpoint: "https://traces.example.com:4317", + IfMissing: "fail", + }, + }, + }, + } + (&Compiler{}).injectOTLPConfig(workflowData) + assert.NotContains(t, workflowData.Env, "GH_AW_OTLP_IF_MISSING") + + validWorkflowData := &WorkflowData{ + RawFrontmatter: map[string]any{ + "observability": map[string]any{ + "otlp": map[string]any{ + "endpoint": "https://traces.example.com:4317", + "if-missing": "warn", + }, + }, + }, + ParsedFrontmatter: &FrontmatterConfig{ + Observability: &ObservabilityConfig{ + OTLP: &OTLPConfig{ + Endpoint: "https://traces.example.com:4317", + IfMissing: "warn", + }, + }, + }, + } + (&Compiler{}).injectOTLPConfig(validWorkflowData) + assert.Contains(t, validWorkflowData.Env, "GH_AW_OTLP_IF_MISSING") + assert.Contains(t, validWorkflowData.Env, "warn") +} + +func TestFormal_AbsentObservabilityProducesNoEndpoints(t *testing.T) { + assert.Empty(t, collectAllOTLPEndpoints(nil)) + assert.Empty(t, collectAllOTLPEndpoints(map[string]any{})) + assert.Empty(t, collectAllOTLPEndpoints(map[string]any{"observability": nil})) +} From 32fff3a2a4438c2ef384029f423f4eec2bbb64b2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:25:38 +0000 Subject: [PATCH 3/3] docs(adr): add draft ADR-41906 for formal OTEL observability compatibility test suite Co-Authored-By: Claude Sonnet 4.6 --- ...-observability-compatibility-test-suite.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/adr/41906-formal-otel-observability-compatibility-test-suite.md diff --git a/docs/adr/41906-formal-otel-observability-compatibility-test-suite.md b/docs/adr/41906-formal-otel-observability-compatibility-test-suite.md new file mode 100644 index 00000000000..a5a5c945f89 --- /dev/null +++ b/docs/adr/41906-formal-otel-observability-compatibility-test-suite.md @@ -0,0 +1,44 @@ +# ADR-41906: Formal OTEL Observability Compatibility Test Suite (v0.4.0) + +**Date**: 2026-06-27 +**Status**: Draft +**Deciders**: Unknown (automated draft from PR #41906 — review and finalize) + +--- + +### Context + +The repository has a formal specification `specs/otel-observability-spec.md` (v0.4.0) that defines 15 predicates for Level 1/2 `observability.otlp` behavior: endpoint form normalization, deterministic header serialization, Sentry `Authorization → x-sentry-auth` rewrite, `if-missing` policy validation, OTEL service-name construction, static domain extraction, expression exclusion from allowlisting, top-level header scoping, fan-out declaration-order preservation, mirror path constants, empty URL entry discard, and nil/empty header normalization. Without explicit test coverage mapped to these predicates, the implementation can silently diverge from the spec as either evolves. The existing `observability_otlp.go` helpers implement these behaviors, but predicate-level regression protection was absent. + +### Decision + +We will add a dedicated formal test file `pkg/workflow/otel_observability_formal_test.go` that maps each of the 15 predicates in `specs/otel-observability-spec.md` v0.4.0 to an explicit `TestFormal_*` test. Tests assert against the public compiler/runtime behavior of `observability_otlp.go` helpers (e.g., `collectAllOTLPEndpoints`, `normalizeOTLPHeadersForEndpoint`, `otelServiceName`) rather than implementation internals, making the spec a continuously-enforced compatibility contract verified on every CI run. + +### Alternatives Considered + +#### Alternative 1: Integration/End-to-End Tests + +Test the full OTEL pipeline end-to-end (workflow compilation → OTLP emission → collector receipt) rather than testing each predicate in isolation. This would be more realistic but far slower to run and harder to isolate failures to specific predicates. It would also require a running OTEL collector in CI, adding infrastructure complexity. Unit-level predicate tests provide faster feedback and pinpoint exactly which invariant broke. + +#### Alternative 2: Property-Based / Fuzz Testing + +Use Go's `testing/quick` or fuzz infrastructure to cover the input space for endpoint normalization and header serialization rather than explicit hand-written cases. This would catch edge cases the spec authors didn't enumerate, but would not verify that the specific named invariants from the formal spec are met. Explicit named tests serve as a living checklist against the spec document, which fuzz tests cannot replicate. + +### Consequences + +#### Positive +- All 15 v0.4.0 predicates from the formal spec are continuously enforced; any regression breaks CI immediately. +- `TestFormal_*` tests serve as executable documentation — future contributors can cross-reference the spec and know exactly which test covers which predicate. +- Tests run at unit speed with no external dependencies, giving fast CI feedback. + +#### Negative +- Tests directly call package-internal functions (`collectAllOTLPEndpoints`, `normalizeOTLPHeadersForEndpoint`, etc.), so internal API refactors require corresponding test updates even if observable behavior is unchanged. +- When `specs/otel-observability-spec.md` is updated (v0.5.0, etc.), the test file must be manually updated to reflect new or changed predicates — there is no automated linkage between the spec document and the test file. + +#### Neutral +- The formal test file lives in `package workflow` (not `package workflow_test`), giving it access to unexported symbols — this is intentional to allow contract-level assertions without adding a testing-only public API. +- The `determinismTestIterations = 10` constant for `TestFormal_HeaderMapDeterminism` is a pragmatic choice; it is not derived from the formal spec and may need tuning if map iteration order changes are suspected. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*