feat(otel): opt-in OTel trace export, off by default (plan 39) by duyet · Pull Request #2243 · chmonitor/chmonitor · GitHub
Skip to content

feat(otel): opt-in OTel trace export, off by default (plan 39) - #2243

Merged
duyet merged 6 commits into
mainfrom
advisor/39-otel-trace-export
Jul 4, 2026
Merged

feat(otel): opt-in OTel trace export, off by default (plan 39)#2243
duyet merged 6 commits into
mainfrom
advisor/39-otel-trace-export

Conversation

@duyet

@duyet duyet commented Jul 3, 2026

Copy link
Copy Markdown
Member

What (plan 39 — OTel trace export)

Opt-in trace export gated on CHM_OTEL_EXPORTER_URL — a true no-op (zero OTel API touched) when unset. Root dashboard-request span (middleware after Sentry) → clickhouse-query child span at the real query-executor.ts seam (corrected from the plan's fetchDataWithHost, which has no production callers). Parses read_bytes from X-ClickHouse-Summary.

  • 27 + 230 tests pass (disabled no-op; enabled span shape/attributes/exception; async-parenting under a real ALS context manager; read_bytes mapping). bun run check / depcruise clean; fixed a real type bug in the middleware.

🔶 Held — hard merge blocker

Adds 6 @opentelemetry/* deps to apps/dashboard/package.json but bun.lock is not regenerated (agents can't bun install). Every --frozen-lockfile CI job will fail until someone runs bun install. Also based a few commits behind main (stale worktree base — verify start.ts/package.json merge cleanly). Do not merge until the lockfile is regenerated.

Co-Authored-By: duyetbot bot@duyet.net

@github-actions github-actions Bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file area: packages Shared packages/* app: dashboard Changes to the apps/dashboard TanStack Start app labels Jul 3, 2026

duyet commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

duyet and others added 6 commits July 4, 2026 12:09
Lays the foundation for plans/39-otel-trace-export.md: a memoized
BasicTracerProvider + BatchSpanProcessor + OTLPTraceExporter singleton
gated on CHM_OTEL_EXPORTER_URL, plus a withSpan(name, attrs, fn) helper
that is a true no-op (zero OTel API calls) when the URL is unset or
invalid. AsyncLocalStorageContextManager is registered only on the
enabled path so nested spans (wired in a follow-up commit) nest under
their parent without needing manual context threading.

withClickHouseQuerySpan wraps a FetchDataResult-shaped call and only
attaches query_id/read_bytes/host attributes when the result actually
populates them.

Co-Authored-By: duyetbot <bot@duyet.net>
fetchData/fetchJsonEachRowAsNormalizedJson now parse the read_bytes
field out of ClickHouse's X-ClickHouse-Summary response header into
metadata.readBytes, only when the header is present and parses
cleanly. Needed by the OTel clickhouse-query span attribute
(plans/39-otel-trace-export.md) but has no OTel dependency itself —
this package stays otel-agnostic.

Co-Authored-By: duyetbot <bot@duyet.net>
…quest path

Root span: otelMiddleware in start.ts wraps every request in a
dashboard-request span (positioned after Sentry so it covers
security-headers/auth/the route handler), warming the memoized OTel
singleton from the Worker env and flushing inline before the response
returns (no waitUntil available here, same constraint sentryMiddleware
already documents).

Child span: query-executor.ts's three ClickHouse execution call sites
(executeTableConfig, executeChartQuery, executeMultiChartQuery) each
wrap their fetchData/fetchJsonEachRowAsNormalizedJson call in a
clickhouse-query span via withClickHouseQuerySpan. This is the actual
load-bearing @chm/clickhouse-client seam (8 route handlers funnel
through it) — not clickhouse-helpers.ts's fetchDataWithHost, which has
no production callers.

AsyncLocalStorageContextManager (registered only when export is
enabled, see exporter.ts) propagates the parent/child relationship
across the async chain without manual context threading. There is no
distinguishable "system-table-read" seam beneath clickhouse-query
(fetchData makes one HTTP call per invocation), so the span tree is
two levels (dashboard-request -> clickhouse-query) rather than three,
per the plan's own documented fallback.

Co-Authored-By: duyetbot <bot@duyet.net>
Adds the env var to .env.example and the environment-variables
reference (self-host docs), with a Jaeger OTLP/HTTP example and the
workerd (HTTP-only, no gRPC) constraint. Declares
@opentelemetry/{api,context-async-hooks,exporter-trace-otlp-http,
resources,sdk-trace-base,semantic-conventions} in apps/dashboard's
package.json — app-side only, not packages/clickhouse-client.

NOTE: bun.lock is NOT updated by this commit (bun install could not be
run in this worktree — see task constraints). The packages are already
present in the shared node_modules and all otel tests pass against
them, but a real `bun install` is needed before a frozen-lockfile CI
job (cloudflare.yml, test.yml, a11y.yml, etc.) will pass. Flagged in
the handoff report.

Co-Authored-By: duyetbot <bot@duyet.net>
start.ts: wrap next() in an async arrow (not a bare () => next()) so
withSpan's fn parameter, typed Promise<T>, actually type-checks —
TanStack's next() return type isn't structurally a Promise. Caught by
bun run type-check (bun test alone never runs tsc).

Adds span-nesting.test.ts: registers a real
AsyncLocalStorageContextManager and proves a clickhouse-query span
started after an await inside dashboard-request's callback still comes
out as its child (parentSpanContext matches), and that two concurrent
requests' spans don't cross-contaminate. This verifies the
withSpan/ALS composition nests correctly; the residual (that TanStack's
own next() chain preserves the same ALS store end-to-end) is only
confirmed against a real collector, not by this unit test.

Co-Authored-By: duyetbot <bot@duyet.net>
apps/dashboard/bun.lock was missing the OTel deps added by this PR,
causing bun install --frozen-lockfile to fail in CI. Rebased onto
current main (clean, no conflicts) and regenerated the lockfile.

Co-Authored-By: duyetbot <bot@duyet.net>
@duyet
duyet force-pushed the advisor/39-otel-trace-export branch from 4c05ed2 to c4faf5f Compare July 4, 2026 05:13
@duyet
duyet enabled auto-merge (squash) July 4, 2026 05:13
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

☁️ Cloudflare Preview Deployment

App Preview URL
Dashboard https://preview.dash.chmonitor.dev
MCP https://preview.dash.chmonitor.dev/api/mcp
Landing https://preview.chmonitor.dev
Docs https://preview.docs.chmonitor.dev
Property Value
Commit c4faf5f
Deployed at 2026-07-04T05:16:28.286Z

Previews are automatically updated on every push to this PR.
Unchanged apps keep their previous preview (path-filtered deploys).

@duyet
duyet merged commit ad67d2e into main Jul 4, 2026
19 of 20 checks passed
@duyet
duyet deleted the advisor/39-otel-trace-export branch July 4, 2026 05:16
@duyet
duyet restored the advisor/39-otel-trace-export branch July 6, 2026 11:42
@duyet
duyet deleted the advisor/39-otel-trace-export branch July 16, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: dashboard Changes to the apps/dashboard TanStack Start app area: packages Shared packages/* dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant