Skip to content

client_version survives Claude Code moving the version to the OTLP resource - #861

Merged
philcunliffe merged 2 commits into
masterfrom
fix/issue-854
Aug 19, 2026
Merged

client_version survives Claude Code moving the version to the OTLP resource#861
philcunliffe merged 2 commits into
masterfrom
fix/issue-854

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

What broke

Claude Code 2.1.233 stamped app.version on every telemetry event. 2.1.235
does not send it on any record and reports the version once per export, on the
OTLP resource, as service.version.

flattenClaudeTelemetryEvents touched the resource only to check the
self-telemetry marker and then discarded it, so mergeSessionFacts had nothing
to read:

facts.clientVersion ??= stringAttr(event, 'app.version')

Every ai_gateway_messages row produced by the OTEL capture path landed with a
null client_version, which is a silent column regression on the client
hyp attach claude now routes through, and a failure of
claude_otel_shape_check step 6 (with_version above zero).

The fix

  • flattenClaudeTelemetryEvents and flattenClaudeTelemetryMetrics decode
    each resource group once (the self-marker check already paid for that decode)
    and carry the decoded attributes on the events that group produced.
  • The projector reads app.version first and falls back to the resource's
    service.version. Event first keeps the 2.1.233 shape working instead of
    trading one version floor for the other.
  • resource is its own field on ClaudeTelemetryEvent, not a merge into
    attributes: claude_telemetry_events rows are per-event and per-export
    facts do not belong in that JSON, so the behavioral dataset is unchanged.

No Accepted LLP is edited. LLP 0262's field-parity table names app.version as
the source for client_version; that is the observation the spike made, and
this closes the gap the upstream shape change opened under the same
requirement (R1), rather than changing what the RFC settled.

Evidence

test/plugins/claude-telemetry-listener.test.js gains three tests. On master
the first one fails with undefined where 2.1.235 is expected:

not ok 8 - client_version falls back to the resource service.version when no event carries app.version
    + undefined
    - '2.1.235'

After the fix all three pass:

  • events stripped of app.version plus service.version on the resource:
    projection and every projected ai_gateway_messages row carry 2.1.235.
  • app.version present alongside a differing resource service.version: the
    event attribute still wins (2.1.233), so the older client is not regressed.
  • neither present: client_version stays unset rather than becoming an empty
    string.

HYP_HOME=$(mktemp -d) npm test is green apart from the known pre-existing
repo-scratch-hygiene failure (#852, PR #853 open). npm run typecheck is
clean. npm run smoke -- claude_telemetry_capture passes.

Docs

docs/ACCEPTANCE.md said the version comes off the events in two pass
conditions, which would read a healthy 2.1.235 capture as a failure. Both now
say where to look on each version.

Fixes #854

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review: approve, with two doc fixes pushed

Reviewed ae84c70e only; the merge commit 873457fa that brings master in for
the #852/#853 transcript cleanup is infrastructure and was excluded from scope.

The fix itself is correct. No correctness or behaviour defect found in the
code change:

  • hasSelfMarker is exactly equivalent to the old resourceHasSelfMarker
    (hypaware-core/plugins-workspace/claude/src/telemetry/events.js:246): same
    single decode per resource group, same true / 'true' acceptance, same drop
    semantics for the daemon's own telemetry. A group with no resource still
    decodes to {} and is kept.
  • The new resource field cannot widen the behavioral dataset. rowFromEvent
    (hypaware-core/plugins-workspace/claude/src/telemetry/events_dataset.js:104)
    reads only event.attributes, and nothing in source.js, policy.js, or
    bodies.js touches event.resource, so claude_telemetry_events rows and
    their content hashes are unchanged. The PR body's claim here holds.
  • The per-group resource object is aliased onto every event of the group, but
    no consumer mutates it and events are never serialized or spooled, so the
    aliasing is safe.
  • projection.js:363 orders stringAttr(event, 'app.version') ?? resourceAttr(event, 'service.version') behind ??=, which short-circuits, so
    the 2.1.233 event attribute still wins. The three new tests pin all three
    branches, including the neither-present case.
  • @ref LLP 0262#field-parity-r1 resolves: ## Field parity (R1) exists in
    llp/0262-otel-attach-replaces-proxy.rfc.md:190. No Accepted LLP text is
    edited, and reading a second location to satisfy R1 extends the parity
    requirement rather than changing what the RFC settled.

Verified on a worktree of 873457fa: npm test 4486 pass / 0 fail / 1 skipped,
npm run typecheck clean, hyp smoke claude_telemetry_capture and
claude_telemetry_session_ignore both ok. No em dashes, no semicolons in the
new JS.

Findings

1. docs/ACCEPTANCE.md:709 (low, docs correctness, introduced by this diff) - FIXED.
The new step 6 pass condition ended "a null here across a whole session is the
regression issue #854 records". Once this merges, #854 is closed and the
resource fallback is in place, so a null with_version means the version
reached neither place: a third upstream shape, or a broken fallback. The line
would send a release operator to a closed issue at the exact moment the
procedure catches a new regression. Reworded to say the version reached
neither place and to file it as new drift the way #854 was filed.

2. docs/ACCEPTANCE.md:764 (low, docs correctness, introduced by this diff) - FIXED.
The new step 8 text lists app.version and app.entrypoint together under one
explanation ("2.1.235 moved the version to the OTLP resource"), but only the
version moved. Issue #854's real 2.1.235 capture shows no app.entrypoint on
the events at all, and the resource carries no replacement, so
ai_gateway_messages.entrypoint goes null on that client for the same reason
client_version did. As written the step normalizes a second silent column
regression without naming it. Now says the entrypoint is simply gone with
nothing to fall back to, that the column goes null, and that this is a separate
gap from #854 which this step does not pass or fail on.

Both fixed in 5ceec1e2, pushed to this branch. Docs only: no behaviour change,
no test change, suite re-run green after the edit.

3. hypaware-core/plugins-workspace/claude/src/telemetry/events.js:122 (low, no action).
The metrics decoder threads resource onto every data-point event, but nothing
reads it: metrics events reach only claudeTelemetryEventRows -> rowFromEvent
(attributes only), and flattenClaudeTelemetryMetrics output never reaches
projectClaudeTelemetryEvents. Inert, symmetric with the logs decoder, and
cheap; left as is rather than made asymmetric. The PR body's "for logs and
metrics alike" overstates it slightly.

Remaining, out of scope for this PR

ai_gateway_messages.entrypoint is null on Claude Code 2.1.235 for the same
upstream reason, and no resource-side fallback exists for it. Nothing in this
PR can fix that; it wants its own issue. Step 6 has no with_entrypoint pass
condition today, so the acceptance procedure will not surface it either.

Also noted while reading step 6: its query filters on
message_created_at >= '$SINCE_SQL', which issue #854 reports silently returns
zero rows against a real cache (icebird filter pushdown), reading as "capture is
broken" rather than "column is null". Pre-existing and untouched by this PR.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Review: clean

Reviewed 5ceec1e2 (5 files, +127/-15) with the code-review skill at high, plus an
independent trace of every consumer of the new field. No blocking findings. The fix is
correct, minimal, and the claims in the PR body hold up under checking. Three non-blocking
observations are recorded at the bottom; I changed nothing and pushed nothing.

What I verified

  • The new resource field does not widen any dataset. event.resource is read in
    exactly one place in the whole repo
    (hypaware-core/plugins-workspace/claude/src/telemetry/projection.js:384).
    claudeTelemetryEventRows -> rowFromEvent
    (hypaware-core/plugins-workspace/claude/src/telemetry/events_dataset.js:101) iterates
    only event.attributes, and nothing anywhere spreads or JSON-serializes a whole event.
    So claude_telemetry_events rows are byte-identical to before, as the PR body claims,
    and the arbitrary resource attributes an operator can attach
    (OTEL_RESOURCE_ATTRIBUTES) are held in memory and never persisted or forwarded. That
    was the main privacy question a change like this raises and it comes out clean.

  • The resourceHasSelfMarker -> hasSelfMarker refactor is behavior-identical.
    decodeAttributes(asObject(group.resource)?.attributes) returns {} for a missing,
    null, or non-object resource, so the never-throw posture on a malformed envelope is
    preserved, and the decode count per group is unchanged (the self-marker check already
    paid for it). The PR body's cost claim is accurate.

  • The fallback order is the right one. Event first, resource second
    (projection.js:363) keeps 2.1.233 captures reading their own app.version instead of
    trading one version floor for the other. Covered by the second new test.

  • @ref LLP 0262#field-parity-r1 resolves. ## Field parity (R1) exists in
    llp/0262-otel-attach-replaces-proxy.rfc.md:190. No Accepted LLP is edited, which is
    the correct posture under the settled-docs rule.

  • Conventions. No semicolons added, no U+2014 anywhere in the diff, types stay in
    .d.ts + JSDoc, no inline import() types, no @typedef.

Checks run (worktree off origin/fix/issue-854, head 5ceec1e2)

  • HYP_HOME=$(mktemp -d) npm test: 4486 pass / 0 fail / 1 skipped. Note that the
    repo-scratch-hygiene failure the PR body records as pre-existing does not
    reproduce on this head; it is already resolved here, so the suite is fully green.
  • npm run typecheck: clean.
  • npm run smoke -- claude_telemetry_capture: ok.
  • npm run smoke -- hypignore_capture_drop: ok.

Non-blocking observations

  1. entrypoint is a second silent column regression, and it is documented but not
    tracked.
    docs/ACCEPTANCE.md:767-770 now states that 2.1.235 sends no
    app.entrypoint on the events and the resource offers no replacement, so
    ai_gateway_messages.entrypoint is null on the current client, and that step 8 does
    not pass or fail on it. That is the same failure class as client_version is null on the OTEL capture path: the projector reads app.version, but Claude Code 2.1.235 only sends service.version on the resource #854, and it is correct to
    keep it out of this PR's scope, but no issue tracks it (gh issue list --state all
    turns up nothing), and step 6's query has no with_entrypoint column, so no acceptance
    step will surface it either. docs/ACCEPTANCE.md's own step 6 rule says such drift is
    "filed as new drift the way client_version is null on the OTEL capture path: the projector reads app.version, but Claude Code 2.1.235 only sends service.version on the resource #854 was". Recommended follow-up: file the entrypoint
    gap as its own issue at merge time
    , otherwise this PR quietly converts a would-be
    gate failure into a permanently excused null. Step 10 still requires recording it in
    the release notes, which limits the damage. I did not file it myself: whether
    entrypoint is recoverable at all on the OTEL path is a design call for a maintainer,
    not something a review round should decide.

  2. The resource service.version is trusted unconditionally (projection.js:363).
    service.version is a generic OTEL resource attribute rather than a Claude-owned one,
    so an operator who sets it through OTEL_RESOURCE_ATTRIBUTES alongside the managed
    attach env block would have that value recorded as client_version on every row from
    2.1.235 onward, and step 6 only tests with_version > 0, so the gate would read green.
    I deliberately did not change this. The obvious hardenings both make things worse:
    gating on service.name === 'claude-code' reintroduces exactly the silent-null failure
    mode this PR fixes the moment upstream renames the service, and shape-validating the
    version string is a heuristic that can reject a legitimate release. Recording what the
    client reported is the defensible behavior. Noted so the tradeoff is on the record, not
    as a change request.

  3. The metrics half of the change is inert. flattenClaudeTelemetryMetrics now
    attaches resource to every metric event (events.js:154), but metric events take the
    short path in telemetry/source.js (flatten, record to claude_telemetry_events,
    done) and never reach projectClaudeTelemetryEvents, so nothing reads it. This is
    cheap (one shared object reference per group, not per event) and the symmetry between
    the two decoders is worth more than the saving. Mentioned only so a later reader does
    not go looking for the consumer.

Verdict

Ship it. The bug is real, the fix is the smallest one that addresses it, the tests pin
all three branches (event wins, resource fallback, neither present), and the acceptance
doc edits make the release gate readable against both client shapes instead of misreading
a healthy 2.1.235 capture as a failure.

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 19, 2026
test and others added 2 commits August 19, 2026 14:34
…source (#854)

Claude Code 2.1.233 stamped `app.version` on every telemetry event; 2.1.235
sends it nowhere on the records and reports the version once per export, on
the OTLP resource, as `service.version`. `flattenClaudeTelemetryEvents`
touched the resource only to check the self-telemetry marker and then threw it
away, so the projector had nothing to read and every `ai_gateway_messages` row
produced by the OTEL capture path landed with a null `client_version`.

The decoder now decodes each resource group once (which the self-marker check
already paid for) and carries the decoded attributes on the events that group
produced, for logs and metrics alike. The projector reads `app.version` first
and falls back to the resource's `service.version`, so the older shape keeps
working rather than being traded away for the newer one.

`resource` is a field of its own on `ClaudeTelemetryEvent`, not a merge into
`attributes`: `claude_telemetry_events` rows are per-event, and per-export
facts do not belong in that JSON.

The acceptance procedure's `claude_otel_shape_check` pass conditions said the
version comes off the events, which would now read a healthy 2.1.235 capture
as a failure; both spots say where to look on each version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two pass conditions this branch rewrote still read wrong once the fix is
in. Step 6 told the operator that a null `with_version` "is the regression
issue #854 records": after this lands, #854 is closed and the resource
fallback is in place, so a null there means the version reached neither
place, which is new drift to file rather than a closed issue to reopen.

Step 8 noted `app.entrypoint` alongside `app.version` under one
explanation, but only the version moved to the resource. 2.1.235 drops the
entrypoint attribute with nothing on the resource to fall back to, so
`ai_gateway_messages.entrypoint` goes null on that client. Saying so keeps
the step from normalizing a gap it does not actually cover.

Docs only: no behavior change, no test change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe
philcunliffe merged commit 019fc82 into master Aug 19, 2026
10 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-854 branch August 19, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

client_version is null on the OTEL capture path: the projector reads app.version, but Claude Code 2.1.235 only sends service.version on the resource

1 participant