Skip to content

fix: emit cache token OTEL attributes as strings for Sentry#3501

Merged
lpcox merged 1 commit into
mainfrom
fix/otel-cache-as-strings
May 21, 2026
Merged

fix: emit cache token OTEL attributes as strings for Sentry#3501
lpcox merged 1 commit into
mainfrom
fix/otel-cache-as-strings

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented May 21, 2026

Sentry drops unknown numeric span attributes but keeps strings (confirmed: awf.request_id string shows up, numeric cache_read_tokens_count was dropped).

Emits cache/reasoning as strings under awf.* namespace:

  • awf.cache_read_tokens → e.g. "13288"
  • awf.cache_write_tokens → e.g. "0"
  • awf.reasoning_tokens → e.g. "0"

These will appear next to awf.request_id in Sentry's span attributes.

Sentry's OTLP ingestion drops unknown numeric span attributes but keeps
string attributes (confirmed: awf.request_id string shows up, but numeric
cache_read_tokens_count was dropped). Emit cache/reasoning values as
strings under the awf.* namespace:

- awf.cache_read_tokens (string)
- awf.cache_write_tokens (string)
- awf.reasoning_tokens (string)

These will appear alongside awf.request_id in Sentry's span attributes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 21, 2026 01:01
@lpcox lpcox merged commit ec6e0f0 into main May 21, 2026
35 of 36 checks passed
@lpcox lpcox deleted the fix/otel-cache-as-strings branch May 21, 2026 01:01
@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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 OpenTelemetry span attributes for cache/reasoning token counts so they appear in Sentry by emitting them as string attributes under the awf.* namespace (since Sentry drops unknown numeric span attributes).

Changes:

  • Emit awf.cache_read_tokens, awf.cache_write_tokens, and awf.reasoning_tokens as strings on the request span and gen_ai.usage event.
  • Remove the previous *_count numeric attribute keys for these fields.
  • Update OTEL unit tests to assert the new awf.cache_* attribute keys and string values.
Show a summary per file
File Description
containers/api-proxy/otel.js Switches cache/reasoning token OTEL attributes to awf.* string attributes for Sentry visibility.
containers/api-proxy/otel.test.js Updates expectations to match the new awf.cache_* string attributes.

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: 2

Comment on lines +448 to +452
// Standard GenAI semconv (Sentry recognizes these as numeric)
'gen_ai.usage.input_tokens': normalizedUsage.input_tokens,
'gen_ai.usage.output_tokens': normalizedUsage.output_tokens,
'gen_ai.request.stream': streaming,
// Cache and reasoning — use Sentry measurement naming (no dots, _count suffix)
'cache_read_tokens_count': normalizedUsage.cache_read_tokens,
'cache_write_tokens_count': normalizedUsage.cache_write_tokens,
'reasoning_tokens_count': normalizedUsage.reasoning_tokens || 0,
// Cache and reasoning as strings (Sentry drops unknown numeric attrs but keeps strings)
Comment on lines 235 to +247
expect(s.attributes['gen_ai.response.model']).toBe('gpt-4o');
expect(s.attributes['gen_ai.usage.input_tokens']).toBe(1000);
expect(s.attributes['gen_ai.usage.output_tokens']).toBe(500);
expect(s.attributes['cache_read_tokens_count']).toBe(200);
expect(s.attributes['cache_write_tokens_count']).toBe(50);
expect(s.attributes['awf.cache_read_tokens']).toBe('200');
expect(s.attributes['awf.cache_write_tokens']).toBe('50');
expect(s.attributes['gen_ai.request.stream']).toBe(false);

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['cache_read_tokens_count']).toBe(200);
expect(usageEvent.attributes['cache_write_tokens_count']).toBe(50);
expect(usageEvent.attributes['awf.cache_read_tokens']).toBe('200');
expect(usageEvent.attributes['awf.cache_write_tokens']).toBe('50');
@github-actions github-actions Bot mentioned this pull request May 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

✅ GitHub API: 2 PR entries confirmed
✅ Playwright: GitHub.com title contains "GitHub"
✅ File verify: Smoke test file exists and is valid

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity ✅ PR listed successfully
GitHub.com HTTP ⚠️ Template vars unresolved (pre-step data unavailable)
File write/read ⚠️ Template vars unresolved (pre-step data unavailable)
BYOK inference (this response) ✅ api-proxy → api.githubcopilot.com working

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

Overall: PASS (core BYOK path confirmed) — PR by @lpcox, reviewer @Copilot.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Copilot Smoke Test

Test Result
GitHub MCP connectivity ✅ (fetched PR: "fix: emit cache token OTEL attributes as strings for Sentry")
GitHub.com HTTP ❌ (pre-step template vars not expanded)
File write/read ❌ (pre-step template vars not expanded)

Overall: FAIL — Pre-computed step outputs (${{ steps.smoke-data.outputs.* }}) were not substituted before agent execution.

PR author: @lpcox

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: API Proxy OTEL Tracing Results

Scenario Result Notes
1. Module Loading otel.js loads cleanly; exports: startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled + internal helpers
2. Test Suite 32/32 tests passed across span creation, token attributes, parent context, OTLP export, file fallback, and graceful shutdown
3. Env Var Forwarding 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
4. Token Tracker Integration token-tracker-http.js exposes onUsage callback (line 57/232) as the OTEL hook point
5. OTEL Diagnostics No OTLP endpoint configured in this run → graceful degradation to file fallback (otel.jsonl); no errors

All scenarios pass. OTEL tracing integration is fully functional.

📡 OTel tracing validated by Smoke OTel Tracing

@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 ❌ NO
Node.js v24.15.0 v20.20.2 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

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

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test results:
PRs reviewed: fix: emit cache token OTEL attributes as strings for Sentry | fix: use Sentry measurement naming for cache token attributes
✅ GitHub PR review; ❌ safeinputs-gh tool; ✅ Playwright title
❌ Tavily search; ✅ file write/read; ✅ discussion comment
✅ npm ci && npm run build
Overall status: FAIL

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

🏗️ 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 All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All 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 #3501 · ● 4.6M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test passed

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

Smoke Test Results

Check Result
Redis PING ❌ timeout (no response)
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ skipped (pg_isready failed)

Overall: FAILhost.docker.internal services unreachable from this runner environment.

🔌 Service connectivity validated by Smoke Services

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