refactor: use pickEnvVars for OTEL environment variable forwarding#3752
Conversation
Replace verbose spread-operator patterns for OTEL/trace env vars
with the existing pickEnvVars() helper in api-proxy-service.ts.
Before (12 lines):
...(process.env.OTEL_EXPORTER_OTLP_ENDPOINT && { ... }),
...(process.env.OTEL_EXPORTER_OTLP_HEADERS && { ... }),
...(process.env.GITHUB_AW_OTEL_TRACE_ID && { ... }),
...(process.env.GITHUB_AW_OTEL_PARENT_SPAN_ID && { ... }),
After (5 lines):
...pickEnvVars('OTEL_EXPORTER_OTLP_ENDPOINT', 'OTEL_EXPORTER_OTLP_HEADERS',
'GITHUB_AW_OTEL_TRACE_ID', 'GITHUB_AW_OTEL_PARENT_SPAN_ID'),
Closes #3612
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors OpenTelemetry (OTEL) / tracing environment variable forwarding in buildApiProxyService to use the existing pickEnvVars() helper, replacing repetitive conditional spread patterns while preserving the same “only forward non-empty env vars” semantics.
Changes:
- Replaces multiple
...(process.env.X && { X: process.env.X })spreads with a single...pickEnvVars(...)call for OTEL/trace-related variables. - Keeps
OTEL_SERVICE_NAMEbehavior unchanged (still defaults toawf-api-proxywhen unset).
Show a summary per file
| File | Description |
|---|---|
src/services/api-proxy-service.ts |
Uses pickEnvVars() to forward OTEL/trace env vars into the api-proxy container environment, reducing duplication without changing behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
✅ Smoke Test Results
Status: PASS — All smoke test checks passed.
|
Smoke Test: Copilot BYOK (Offline) Mode✅ GitHub MCP: Retrieved PR #3704 "Reduce Documentation Maintainer workflow prompt/tool overhead" Mode: BYOK offline (COPILOT_OFFLINE=true) via api-proxy sidecar Note: File and HTTP test data from pre-step appears to be missing from workflow context.
|
|
Smoke Test Results ✅ GitHub MCP: Fetched PR #3704 "Reduce Documentation Maintainer workflow prompt/tool overhead" Status: FAIL (1 test failed, 1 inconclusive) cc: @lpcox
|
|
Smoke test: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test Results: API Proxy OpenTelemetry Tracing✅ Scenario 1: Module LoadingStatus: PASS ✅ Scenario 2: Test SuiteStatus: PASS
✅ Scenario 3: Env Var ForwardingStatus: PASS
✅ Scenario 4: Token Tracker IntegrationStatus: PASS ✅ Scenario 5: OTEL DiagnosticsStatus: PASS (Expected Development State) SummaryResult: ✅ All scenarios pass The PR successfully refactors OTEL environment variable forwarding from verbose conditional spreads to the cleaner
The refactoring reduces code from 12 lines to 5 lines with identical semantics. Warning Firewall blocked 3 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "127.0.0.1"
- "api.example.com"
- "api.openai.com"See Network Configuration for more information.
|
Gemini Smoke Test Results
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Service Connectivity Results❌ Redis: TIMEOUT/FAIL Overall: FAIL - Services not reachable via host.docker.internal
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS All build and test operations completed successfully across all ecosystems.
|
Chroot Version Comparison ResultsThe chroot environment version tests have completed with some mismatches:
Summary
Overall Result: The
|
Summary
Replace verbose
...(process.env.X && { X: process.env.X })patterns for OTEL/trace environment variables with the existingpickEnvVars()helper.This reduces 12 lines of repetitive conditional spreads to a single 5-line
pickEnvVars()call while preserving identical semantics (only set vars are forwarded).Verification
tsc --noEmitpassesCloses #3612