Skip to content

Refactor OTEL test module-reload helper into shared utility#4941

Merged
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-code-in-otel-tests
Jun 14, 2026
Merged

Refactor OTEL test module-reload helper into shared utility#4941
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-code-in-otel-tests

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

The OTEL module reload/env-isolation helper logic was duplicated across otel.test.js and otel-fanout.test.js, and the copies had already drifted (GH_AW_OTLP_ENDPOINTS existed in only one). This change consolidates that behavior into one shared test utility to keep future env-key updates consistent.

  • Shared OTEL test utility

    • Added containers/api-proxy/test-helpers/otel-test-utils.js
    • Introduces:
      • OTEL_ENV_KEYS (single source of truth for OTEL-related env vars)
      • loadOtelModule(envOverrides) (save env → clear keys → jest.resetModules() → require otel → restore env)
  • otel.test.js cleanup

    • Replaced inline loadOtel implementation with a thin wrapper around loadOtelModule(...).
    • Preserves existing call sites and test structure.
  • otel-fanout.test.js cleanup

    • Replaced both duplicated loadOtelFresh implementations with wrappers that call loadOtelModule(...).
    • Keeps fan-out tests focused on endpoint parsing/initialization while removing repeated module-load plumbing.
// containers/api-proxy/test-helpers/otel-test-utils.js
const OTEL_ENV_KEYS = [
  'GH_AW_OTLP_ENDPOINTS',
  'OTEL_EXPORTER_OTLP_ENDPOINT',
  // ...
];

function loadOtelModule(envOverrides = {}) {
  const saved = {};
  for (const k of OTEL_ENV_KEYS) { saved[k] = process.env[k]; delete process.env[k]; }
  Object.assign(process.env, envOverrides);
  jest.resetModules();
  const mod = require('../otel');
  for (const k of OTEL_ENV_KEYS) {
    if (saved[k] !== undefined) process.env[k] = saved[k];
    else delete process.env[k];
  }
  return mod;
}

Copilot AI changed the title [WIP] Fix duplicate code in OTEL module-reload helper Refactor OTEL test module-reload helper into shared utility Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:33
Copilot AI requested a review from lpcox June 14, 2026 17:33
@lpcox
lpcox marked this pull request as ready for review June 14, 2026 17:35
Copilot AI review requested due to automatic review settings June 14, 2026 17:35

Copilot AI left a comment

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.

Pull request overview

This PR consolidates duplicated OTEL module reload + env isolation logic used by the api-proxy’s OTEL-related unit tests into a shared helper, ensuring consistent handling of OTEL env keys across test suites.

Changes:

  • Added a shared OTEL test utility (otel-test-utils.js) with a single source of truth for OTEL-related env keys and a helper to load otel.js with env isolation.
  • Updated otel.test.js to use the shared loader instead of an inline implementation.
  • Updated otel-fanout.test.js to use the shared loader in both places where fresh module loads were previously duplicated.
Show a summary per file
File Description
containers/api-proxy/test-helpers/otel-test-utils.js Introduces shared env-key list and otel.js fresh-load helper for tests.
containers/api-proxy/otel.test.js Replaces local OTEL module reload/env setup with the shared helper.
containers/api-proxy/otel-fanout.test.js Removes duplicated loadOtelFresh implementations in favor of the shared helper.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread containers/api-proxy/test-helpers/otel-test-utils.js Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx 1/1 passed ✅ PASS
Node.js execa 1/1 passed ✅ PASS
Node.js p-limit 1/1 passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #4941 ·

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test: Copilot PAT — PASS

Test Result
GitHub MCP connectivity
GitHub.com HTTP (200)
File write/read

Overall: PASS · Auth mode: PAT (COPILOT_GITHUB_TOKEN)

PR: "Refactor OTEL test module-reload helper into shared utility" · @lpcox @Copilot

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) — PASS ✅

Test Result
GitHub MCP connectivity
GitHub.com HTTP 200
File write/read
BYOK inference (agent → api-proxy → api.githubcopilot.com)

Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

Merged PRs:

  • Deduplicate Copilot bearer-prefix stripping in api-proxy
  • refactor(api-proxy): deduplicate guard enforcement between HTTP and WebSocket paths, fix 3 missing WebSocket guards

✅ GitHub PR query
✅ GH CLI query
✅ Playwright title check
✅ File write/read
✅ Discussion query/comment
✅ Build (npm ci && npm run build)

Overall: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor

🔭 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
1. Module Loading otel.js loads cleanly; exports startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled + internal helpers
2. Test Suite 59/59 tests passed across otel.test.js and otel-fanout.test.js
3. Env Var Forwarding src/services/api-proxy-service-config.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME; observability-environment.ts auto-forwards all OTEL_* vars
4. Token Tracker Integration onUsage callback present in token-tracker-http.js (line 256); invoked after usage normalization as the OTEL hook point
5. OTEL Diagnostics Graceful degradation confirmed — FileSpanExporter writes to /var/log/api-proxy/otel.jsonl when no OTLP endpoint is configured; no errors on unconfigured runs

All 5 scenarios passed.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

PR: Refactor OTEL test module-reload helper into shared utility
Author: @Copilot | Assignees: @lpcox @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP ❓ (template vars not substituted)
File write/read ❓ (template vars not substituted)

Overall: PARTIAL — MCP verified ✅; pre-step template variables were not substituted so HTTP and file tests could not be evaluated.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

Chroot Version Comparison

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.16.0 v22.22.3
Go go1.22.12 go1.22.12

Result: ❌ Not all versions match — Python and Node.js differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ timeout (no response on host.docker.internal:6379)
PostgreSQL pg_isready ❌ no response on host.docker.internal:5432
PostgreSQL SELECT 1 ❌ skipped (pg_isready failed)

host.docker.internal resolves to 172.17.0.1 but neither port is reachable.

Overall: FAIL

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results

  • GitHub MCP Testing: ❌ (Tools not found)
  • GitHub.com Connectivity: ❌ (Status 000/35)
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅

Overall Status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions

Copy link
Copy Markdown
Contributor

Deduplicate Copilot bearer-prefix stripping in api-proxy
MCP: ✅ github.com: ✅ file I/O: ✅ BYOK: ✅
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
Overall: PASS
cc @Copilot @lpcox

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@github-actions

Copy link
Copy Markdown
Contributor

@lpcox @Copilot

Refactor OTEL test module-reload helper into shared utility

MCP connectivity: ✅
GitHub.com connectivity: ✅
File write/read: ❌
BYOK inference: ✅

Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra

Overall: FAIL

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@lpcox
lpcox merged commit 57a0a28 into main Jun 14, 2026
120 of 141 checks passed
@lpcox
lpcox deleted the copilot/fix-duplicate-code-in-otel-tests branch June 14, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants