feat: merge all OTLP endpoints from shared agentic workflow imports#30209
Merged
feat: merge all OTLP endpoints from shared agentic workflow imports#30209
Conversation
- Change observabilityBuilder (first-wins) to observabilityConfigs []string that collects ALL imported observability JSON blobs - Add mergeObservabilityConfigs() in pkg/parser that deduplicates by URL and produces a single array-form JSON for all imports' endpoints - Add extractOTLPEndpointsFromObsMap() in pkg/parser that handles string/object/array endpoint forms, preserving raw headers format - Add extractRawOTLPEndpointMaps() in pkg/workflow/observability_otlp.go that extracts endpoint maps with original headers (prevents false deprecation warnings from normalized strings) - Update compiler_orchestrator_workflow.go to always merge endpoints from main workflow AND imports (not only when main has no observability), deduplicating by URL with main workflow taking precedence - Add comprehensive tests for all new helpers (21 new test cases) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bc1e4ef7-1c91-435e-8170-a4337b3c5993 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ssages - Document shallow copy intent in extractRawOTLPEndpointMaps - Add granular logging in compiler (main vs import endpoint counts) - Include import index in unmarshal error message - Include endpoint count in marshal error message Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bc1e4ef7-1c91-435e-8170-a4337b3c5993 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
May 4, 2026 19:10
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes OTLP observability endpoint handling when multiple shared workflows are imported by merging all imported observability.otlp.endpoint values (plus any main workflow endpoints) into a single deduplicated endpoint array for fan-out telemetry.
Changes:
- Collect observability configs from all imports, extract OTLP endpoints across string/object/array forms, and merge/deduplicate by URL.
- Preserve header representation (string vs map) through merging to avoid emitting false deprecation warnings.
- Always merge imported + main endpoints into
RawFrontmatterbefore OTLP injection so downstream compilation sees the full endpoint set.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/observability_otlp.go | Adds raw endpoint extraction helper to preserve header formats during merge. |
| pkg/workflow/observability_otlp_test.go | Adds unit tests for raw endpoint extraction across endpoint notations. |
| pkg/workflow/compiler_orchestrator_workflow.go | Updates compilation orchestration to merge imported + main OTLP endpoints (URL-deduped, main-first). |
| pkg/parser/import_processor.go | Updates MergedObservability documentation to reflect merged endpoint-array behavior. |
| pkg/parser/import_field_extractor.go | Collects observability from all imports and adds endpoint extraction + merge/dedup into a single JSON blob. |
| pkg/parser/import_field_extractor_test.go | Adds tests for endpoint extraction and merged observability JSON output/dedup behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
Comment on lines
+171
to
+210
| // Merge observability endpoints from imports with those from the main workflow. | ||
| // All OTLP endpoints from both sources are combined into an array, deduplicating | ||
| // by URL (main workflow endpoints take precedence). This allows multiple shared | ||
| // workflows each defining their own OTLP endpoint to fan out to all collectors. | ||
| 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") | ||
| var importedObs map[string]any | ||
| if err := json.Unmarshal([]byte(obs), &importedObs); err == nil { | ||
| seen := make(map[string]bool) | ||
| var mergedEndpoints []any | ||
|
|
||
| // Main workflow endpoints take precedence (first in, first wins dedup). | ||
| var mainObs map[string]any | ||
| if v, ok := workflowData.RawFrontmatter["observability"]; ok { | ||
| mainObs, _ = v.(map[string]any) | ||
| } | ||
| for _, ep := range extractRawOTLPEndpointMaps(mainObs) { | ||
| if url, _ := ep["url"].(string); url != "" && !seen[url] { | ||
| seen[url] = true | ||
| mergedEndpoints = append(mergedEndpoints, ep) | ||
| } | ||
| } | ||
|
|
||
| // Append import endpoints that aren't already present. | ||
| importAdded := 0 | ||
| for _, ep := range extractRawOTLPEndpointMaps(importedObs) { | ||
| if url, _ := ep["url"].(string); url != "" && !seen[url] { | ||
| seen[url] = true | ||
| mergedEndpoints = append(mergedEndpoints, ep) | ||
| importAdded++ | ||
| } | ||
| } | ||
|
|
||
| if len(mergedEndpoints) > 0 { | ||
| mainCount := len(mergedEndpoints) - importAdded | ||
| workflowData.RawFrontmatter["observability"] = map[string]any{ | ||
| "otlp": map[string]any{ | ||
| "endpoint": mergedEndpoints, | ||
| }, | ||
| } | ||
| orchestratorWorkflowLog.Printf("Merged OTLP endpoints into RawFrontmatter: %d from main workflow, %d from imports (%d total)", mainCount, importAdded, len(mergedEndpoints)) |
Collaborator
|
@copilot merge main and recompile |
…bility-endpoints # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…merging - TestCompileWorkflow_MultipleImportsOTLPEndpointsMerged: full scenario with 3 imports (string, object, array notation) + main workflow endpoint; verifies all distinct endpoints appear in GH_AW_OTLP_ENDPOINTS and main-workflow endpoint is the primary OTEL_EXPORTER_OTLP_ENDPOINT - TestCompileWorkflow_TwoImportsWithOTLPNoMainEndpoint: no main observability section; first import's endpoint becomes the primary - TestCompileWorkflow_DuplicateOTLPEndpointAcrossImportsDeduped: same URL in two imports produces only one entry in GH_AW_OTLP_ENDPOINTS Also includes merge of origin/main and workflow recompile. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bc26ac42-007e-4dae-94bc-93199026096c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done — merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot review all comments |
Contributor
Author
All comments have been addressed:
No open items remain. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When multiple shared workflows each define
observability.otlp, only the first import's endpoint survived (first-wins). All subsequent imports' endpoints were silently dropped. This change collects all endpoints from all imports, merges them with the main workflow's endpoints, deduplicates by URL, and injects the full array for fan-out telemetry.Changes
pkg/parser/import_field_extractor.goobservabilityBuilder strings.Builder→observabilityConfigs []string: collect all observability JSON blobs, not just the firstextractOTLPEndpointsFromObsMap(): handles string/object/array endpoint notation, preserving headers in original format (map vs string) to avoid false deprecation warningsmergeObservabilityConfigs(): deduplicates by URL (first-occurrence wins), produces a single{"otlp":{"endpoint":[...]}}array-form JSONpkg/workflow/observability_otlp.goextractRawOTLPEndpointMaps(): extracts endpointmap[string]anyentries with headers intact — unlikecollectAllOTLPEndpoints, does not normalize headers to string formpkg/workflow/compiler_orchestrator_workflow.goobservability)pkg/workflow/observability_job_summary_test.goTestCompileWorkflow_MultipleImportsOTLPEndpointsMerged: end-to-end regression test compiling a workflow with 3 imports (string, object, and array endpoint notation) plus a main-workflow endpoint; asserts all distinct endpoints appear inGH_AW_OTLP_ENDPOINTSand the main endpoint is the primaryOTEL_EXPORTER_OTLP_ENDPOINTTestCompileWorkflow_TwoImportsWithOTLPNoMainEndpoint: verifies first import's endpoint becomes primary when the main workflow has noobservabilitysectionTestCompileWorkflow_DuplicateOTLPEndpointAcrossImportsDeduped: verifies that the same URL defined in two imports produces exactly one entry inGH_AW_OTLP_ENDPOINTSExample — previously broken, now works
Supports mix of string, object, and array endpoint notation across imports. Deduplication is URL-based; headers from the first occurrence of a given URL win.