Skip to content

fix: rename OTEL cache attrs to avoid Sentry PII scrubber#3504

Merged
lpcox merged 1 commit into
mainfrom
fix/otel-avoid-token-scrub
May 21, 2026
Merged

fix: rename OTEL cache attrs to avoid Sentry PII scrubber#3504
lpcox merged 1 commit into
mainfrom
fix/otel-avoid-token-scrub

Conversation

@lpcox

@lpcox lpcox commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Sentry's data scrubbing redacts values for any attribute with 'token' in the name (assumes it's a credential). Renamed:

  • awf.cache_read_tokensawf.cached_read
  • awf.cache_write_tokensawf.cached_write
  • awf.reasoning_tokensawf.reasoning

Values are still emitted as strings (Sentry drops unknown numeric custom attributes).

Sentry's data scrubbing redacts any attribute containing 'token' in the
name (assumes it's a secret). Rename to avoid the scrubber:

- awf.cache_read_tokens → awf.cached_read
- awf.cache_write_tokens → awf.cached_write
- awf.reasoning_tokens → awf.reasoning

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 21, 2026 01:08
@lpcox lpcox enabled auto-merge (squash) May 21, 2026 01:08
@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.72% 95.79% 📈 +0.07%
Statements 95.55% 95.62% 📈 +0.07%
Functions 96.70% 96.70% ➡️ +0.00%
Branches 89.20% 89.24% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

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 updates the api-proxy’s OpenTelemetry (OTEL) token-usage attribute names to avoid Sentry’s PII scrubber redacting values for any attribute key containing "token", while keeping the emitted values as strings for Sentry compatibility.

Changes:

  • Renamed OTEL span/event attributes from awf.*_tokens to awf.cached_* and awf.reasoning.
  • Updated the OTEL unit test expectations to use the new cache attribute keys.
Show a summary per file
File Description
containers/api-proxy/otel.js Renames emitted OTEL attribute keys for cache/reasoning usage to avoid Sentry redaction.
containers/api-proxy/otel.test.js Updates assertions to validate the renamed cache attribute keys.

Copilot's findings

Tip

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

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

Comment on lines +452 to +455
// Cache and reasoning as strings — avoid "token" in name (Sentry PII scrubber redacts it)
'awf.cached_read': String(normalizedUsage.cache_read_tokens),
'awf.cached_write': String(normalizedUsage.cache_write_tokens),
'awf.reasoning': String(normalizedUsage.reasoning_tokens || 0),
expect(s.attributes['awf.cache_read_tokens']).toBe('200');
expect(s.attributes['awf.cache_write_tokens']).toBe('50');
expect(s.attributes['awf.cached_read']).toBe('200');
expect(s.attributes['awf.cached_write']).toBe('50');
Comment on lines 242 to +247
const usageEvent = s.events.find(e => e.name === 'gen_ai.usage');
expect(usageEvent).toBeDefined();
expect(usageEvent.attributes['gen_ai.usage.input_tokens']).toBe(1000);
expect(usageEvent.attributes['gen_ai.usage.output_tokens']).toBe(500);
expect(usageEvent.attributes['awf.cache_read_tokens']).toBe('200');
expect(usageEvent.attributes['awf.cache_write_tokens']).toBe('50');
expect(usageEvent.attributes['awf.cached_read']).toBe('200');
expect(usageEvent.attributes['awf.cached_write']).toBe('50');
@github-actions

Copy link
Copy Markdown
Contributor

✅ GitHub API: Confirmed 2 PR entries in recent-prs.json
✅ Playwright: Navigated to github.com, title contains "GitHub"
✅ File verify: smoke-test-claude-26199332374.txt exists

Result: PASS ✅ All smoke tests passed.

💥 [THE END] — Illustrated by Smoke Claude

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity ❌ template vars unexpanded
File write/read ❌ template vars unexpanded

Overall: FAIL — Pre-step outputs (steps.smoke-data.outputs.*) were not interpolated into the prompt; connectivity and file tests could not be evaluated.

PR by @lpcox · reviewer: @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity ⚠️ pre-step data not substituted
File write/read ⚠️ pre-step data not substituted
BYOK inference (api-proxy → api.githubcopilot.com)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

PR: fix: rename OTEL cache attrs to avoid Sentry PII scrubber by @lpcox

Overall: PARTIAL (pre-step outputs not injected into prompt)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Status Notes
S1: Module Loading otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled
S2: Test Suite 32/32 tests passed (otel.test.js)
S3: Env Var Forwarding src/services/api-proxy-service.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, OTEL_SERVICE_NAME, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID
S4: Token Tracker Integration onUsage callback exists in token-tracker-http.js (line 57) as OTEL hook point
S5: OTEL Diagnostics No OTLP endpoint configured (expected); graceful degradation to local file fallback (/var/log/api-proxy/otel.jsonl)

All scenarios pass. OTEL integration is complete and working correctly.

📡 OTel tracing validated by Smoke OTel Tracing

@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 passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit 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 #3504 · ● 4M ·

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results

  • Redis PING: ❌ (connection timeout — host.docker.internal:6379 unreachable)
  • PostgreSQL pg_isready: ❌ (no response — host.docker.internal:5432 unreachable)
  • PostgreSQL SELECT 1: ❌ (skipped — pg_isready failed)

Overall: FAIL — Service containers are not reachable from this environment.

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test FAIL: connectivity 000, mcpscripts missing.

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

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.15.0 v20.20.2
Go go1.22.12 go1.22.12

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

Tested by Smoke Chroot

@lpcox lpcox merged commit 719b171 into main May 21, 2026
70 of 73 checks passed
@lpcox lpcox deleted the fix/otel-avoid-token-scrub branch May 21, 2026 01:24
@github-actions github-actions Bot mentioned this pull request May 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Smoke test Codex: FAIL
✅ PR: fix: rename OTEL cache attrs to avoid Sentry PII scrubber
✅ PR: fix: emit cache token OTEL attributes as strings for Sentry
❌ safeinputs-gh unavailable
✅ Playwright title contains GitHub
❌ Tavily search unavailable
✅ File/bash checks passed
❌ Discussion query unavailable
✅ npm ci && npm run build passed

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

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.

2 participants