Skip to content

[Repo Assist] fix(tracing): isolate provider tests from ambient GH_AW_OTLP_ENDPOINTS#7478

Merged
lpcox merged 1 commit into
mainfrom
repo-assist/fix-tracing-test-isolation-20260613-de2180f8849ae895
Jun 13, 2026
Merged

[Repo Assist] fix(tracing): isolate provider tests from ambient GH_AW_OTLP_ENDPOINTS#7478
lpcox merged 1 commit into
mainfrom
repo-assist/fix-tracing-test-isolation-20260613-de2180f8849ae895

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Closes #7468 (Issues A and B)

Three tests in internal/tracing/provider_test.go lacked t.Setenv("GH_AW_OTLP_ENDPOINTS", "") guards, causing them to fail in CI environments where that variable is set — specifically the smoke-otel-tracing workflow which sets it to a JSON-array value.

Root Cause

resolveExtraEndpoints reads GH_AW_OTLP_ENDPOINTS directly from the environment. When it is non-empty, the extra endpoints take precedence over cfg.Endpoint, breaking tests that assume no active endpoints:

Test Failure mode
TestInitProvider_NoEndpoint_ReturnsNoopProvider Asserts global provider is noop.TracerProvider, but SDK provider is installed when env var is set
TestInitProvider_EmptyEndpoint_ReturnsNoopProvider Same path: empty cfg.Endpoint eclipsed by env-var endpoints
TestInitProvider_WithHeaders (×3 subtests) Per-subtest httptest.Server is the intended export target, but GH_AW_OTLP_ENDPOINTS routes spans to external garbage URLs; 3s timeout fires

Fix

Added t.Setenv("GH_AW_OTLP_ENDPOINTS", "") to each affected test. t.Setenv automatically restores the original value at test cleanup, so there are no side effects on other tests.

The other well-guarded tests (TestInitProvider_IsEnabled_Noop, TestInitProvider_IsEnabled_SDK, TestInitProvider_FanOut_*) already set this env var correctly.

What's Not Fixed Here (Issue C)

Issue C from #7468resolveExtraEndpoints not parsing the JSON-array format that the smoke workflow produces — is not addressed in this PR. That requires a design decision (support JSON format, or change the workflow to use plain comma-separated URLs per the documented format in AGENTS.md).

Test Status

⚠️ Infrastructure failure: Go module dependencies (go.opentelemetry.io/otel v1.44.0, github.com/stretchr/testify v1.11.1) are not cached in this environment and the network proxy is blocked, so go test cannot run. The changes are mechanically correct — they add only t.Setenv calls with the same pattern used in other already-passing tests in the same file.

The fix exactly mirrors the pattern already used in:

  • TestInitProvider_IsEnabled_Noop (line 29)
  • TestInitProvider_IsEnabled_SDK (line 39)
  • TestInitProvider_FanOut_EmptyEnvVar (line 156)

Warning

Firewall blocked 1 domain

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

  • proxy.golang.org

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

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · 1.2K AIC · ⊞ 50.2K ·
Comment /repo-assist to run again

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55

Three tests lacked t.Setenv("GH_AW_OTLP_ENDPOINTS", "") guards, causing
failures in CI environments where that variable is set (e.g. the
smoke-otel-tracing workflow that sets it to a JSON-array value):

• TestInitProvider_NoEndpoint_ReturnsNoopProvider — asserts the global
  provider is a noop.TracerProvider, but when GH_AW_OTLP_ENDPOINTS is
  non-empty resolveExtraEndpoints produces active endpoints, so an SDK
  provider is installed instead.

• TestInitProvider_EmptyEndpoint_ReturnsNoopProvider — same path; the
  empty cfg.Endpoint is eclipsed by the env-var endpoints.

• TestInitProvider_WithHeaders — the per-subtest httptest.Server is the
  intended export target, but GH_AW_OTLP_ENDPOINTS overrides cfg.Endpoint
  and routes spans to external URLs. The 3-second timeout fires because
  the local server never receives the export request.

Fix: add t.Setenv("GH_AW_OTLP_ENDPOINTS", "") to each affected test so
the env variable is reset to empty for the duration of the test and
automatically restored to its original value by testing.T cleanup.

Related: #7468 (Issues A and B of the reported root cause)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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 improves test isolation in internal/tracing/provider_test.go by ensuring certain provider tests are not affected by an ambient GH_AW_OTLP_ENDPOINTS environment variable set by CI workflows, which can change endpoint selection logic and make tests flaky/fail.

Changes:

  • Added t.Setenv("GH_AW_OTLP_ENDPOINTS", "") to TestInitProvider_NoEndpoint_ReturnsNoopProvider.
  • Added t.Setenv("GH_AW_OTLP_ENDPOINTS", "") to TestInitProvider_EmptyEndpoint_ReturnsNoopProvider.
  • Added t.Setenv("GH_AW_OTLP_ENDPOINTS", "") to TestInitProvider_WithHeaders to keep spans routed to the per-subtest httptest.Server.
Show a summary per file
File Description
internal/tracing/provider_test.go Clears GH_AW_OTLP_ENDPOINTS in tests that rely on the “no endpoint” and “local httptest endpoint” behaviors, preventing CI env leakage from changing endpoint resolution.

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

@lpcox lpcox merged commit a947b66 into main Jun 13, 2026
24 checks passed
@lpcox lpcox deleted the repo-assist/fix-tracing-test-isolation-20260613-de2180f8849ae895 branch June 13, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants