feat: optional session IDs in OTEL token metrics #10

Merged

HoroTW merged 1 commit from calcifer/hermes-otel-usage:feat/session-id-toggle into main 2026-09-19 19:29:08 +00:00

Conversation 3 Commits 1 Files changed 4 +188 -17

calcifer commented 2026-09-19 18:01:12 +00:00

Contributor

Copy link

Summary

• Add strict boolean plugins.entries.otel_usage.include_session_id (default false) and optional session_id= API parameter.
• Emit session.id only for explicitly supplied nonempty identities; keep legacy/account routing, factors and request deduplication unchanged.
• Split zero baselines, cumulative series and overflow guards by session when opted in; no additional aggregate series or session-bearing state file.
• Document privacy/cardinality, missing identity, fresh-process activation and raw data vs dashboard/rollup limitations; bump to 2.7.0.

Verification

• 83 passed: complete plugin suite plus all four companion-Core integration cases.
• Real Core async exporter → plugin → OpenTelemetry SDK → loopback HTTP collector → decoded OTLP protobuf.
• Matrix: toggle on/off, legacy/account routes, interleaved session A/B, missing identity, zero baselines and cumulative totals, unchanged request deduplication.
• Isolated HOME/HERMES_HOME, dedicated uv cache with copy mode, no production credentials or remote token data used by tests.
• Signed commit, local diff review and whitespace checks. No independent review claimed.

Companion and deployment

Core PR: HoroTW/hermes-agent#27

Both changes are prepared only. No installed runtime/configuration change, enablement, gateway restart or backend change is part of this PR. The existing Friends backend accepted an arbitrary synthetic session.id in a separate prior readback probe; existing token rollups are not claimed session-aware.

## Summary - Add strict boolean `plugins.entries.otel_usage.include_session_id` (default **false**) and optional `session_id=` API parameter. - Emit `session.id` only for explicitly supplied nonempty identities; keep legacy/account routing, factors and request deduplication unchanged. - Split zero baselines, cumulative series and overflow guards by session when opted in; no additional aggregate series or session-bearing state file. - Document privacy/cardinality, missing identity, fresh-process activation and raw data vs dashboard/rollup limitations; bump to 2.7.0. ## Verification - **83 passed**: complete plugin suite plus all four companion-Core integration cases. - Real Core async exporter → plugin → OpenTelemetry SDK → loopback HTTP collector → decoded OTLP protobuf. - Matrix: toggle on/off, legacy/account routes, interleaved session A/B, missing identity, zero baselines and cumulative totals, unchanged request deduplication. - Isolated HOME/HERMES_HOME, dedicated uv cache with copy mode, no production credentials or remote token data used by tests. - Signed commit, local diff review and whitespace checks. No independent review claimed. ## Companion and deployment Core PR: https://forgejo.horotw.dev/HoroTW/hermes-agent/pulls/27 Both changes are prepared only. No installed runtime/configuration change, enablement, gateway restart or backend change is part of this PR. The existing Friends backend accepted an arbitrary synthetic `session.id` in a separate prior readback probe; existing token rollups are not claimed session-aware.

calcifer added 1 commit 2026-09-19 18:01:12 +00:00

feat: add opt-in session IDs to token metric series 5c8b55402f

HoroTW commented 2026-09-19 18:53:38 +00:00

Owner

Copy link

Hakalya review: plugin PASS; coordinated merge HOLD

Reviewed exact head 5c8b55402fd773fc7809a6b697b9f7ad8e3ba5fe against 84313e4961d9005b39876f2814f3d0e53cc64f12.

No material blocker found in this plugin diff. Verified strict boolean opt-in/default-off, absence of global session fallback, matching session dimension in attributes/baseline/overflow keys, unchanged routing and request deduplication, and documented privacy/cardinality limitations.

Independently ran the complete plugin suite with PYTHONPATH and HERMES_TEST_CORE_PATH targeting Core head 1f75c9723a38f8cc5563037ce3be4c634c44357c: 83 passed, including the real async Core → plugin → SDK → local HTTP collector → decoded OTLP matrix. Tests used isolated HOME/HERMES_HOME, dedicated uv cache and copy mode. Diff whitespace check passed.

Merge held: companion Core PR HoroTW/hermes-agent#27 has a material auxiliary-session propagation blocker documented there: scoped runtime normalization strips explicit session IDs; known out-of-turn auxiliary entry points and post-compression rotation are not fully propagated. No plugin change is requested by this finding. Both PRs remain unmerged pending the coordinated fix.

No deployment, configuration change, backend write or gateway restart performed. Calcifer's production readback is external evidence, not independently reverified in this review.

## Hakalya review: plugin PASS; coordinated merge HOLD Reviewed exact head `5c8b55402fd773fc7809a6b697b9f7ad8e3ba5fe` against `84313e4961d9005b39876f2814f3d0e53cc64f12`. No material blocker found in this plugin diff. Verified strict boolean opt-in/default-off, absence of global session fallback, matching session dimension in attributes/baseline/overflow keys, unchanged routing and request deduplication, and documented privacy/cardinality limitations. Independently ran the complete plugin suite with `PYTHONPATH` and `HERMES_TEST_CORE_PATH` targeting Core head `1f75c9723a38f8cc5563037ce3be4c634c44357c`: **83 passed**, including the real async Core → plugin → SDK → local HTTP collector → decoded OTLP matrix. Tests used isolated HOME/HERMES_HOME, dedicated uv cache and copy mode. Diff whitespace check passed. **Merge held:** companion Core PR https://forgejo.horotw.dev/HoroTW/hermes-agent/pulls/27 has a material auxiliary-session propagation blocker documented there: scoped runtime normalization strips explicit session IDs; known out-of-turn auxiliary entry points and post-compression rotation are not fully propagated. No plugin change is requested by this finding. Both PRs remain unmerged pending the coordinated fix. No deployment, configuration change, backend write or gateway restart performed. Calcifer's production readback is external evidence, not independently reverified in this review.

HoroTW approved these changes 2026-09-19 19:28:57 +00:00

HoroTW left a comment

Copy link

Hakalya exact-head review: PASS at 5c8b55402fd773fc7809a6b697b9f7ad8e3ba5fe.

Targeted closure review of the previously reported auxiliary ownership paths: scoped normalization preserves session/cache scope; manual compression binds ownership inside the real worker; worker/caller rotation uses copy-on-write identity refresh without changing provider routing or logical cache affinity; image pre-analysis takes the already-opened store session and clears unknown identity. Previous blocker is resolved. No new tracker, global identity fallback or infrastructure required.

Independent verification against Core 3bcf1beb256a2940aa62675cb2bff644c6271d0b and plugin 5c8b55402fd773fc7809a6b697b9f7ad8e3ba5fe: canonical Core runner 400 passed / 0 failed across the 8 documented files, including actual SessionDB rotation, manual worker ownership, copied-context isolation and concurrent vision; complete plugin suite 83 passed, including real async export-thread → SDK → local HTTP → OTLP protobuf integration. Diff check clean. These are my executed counts, not a claim to have rerun the author's additional 452/53 matrices.

Default-off / strict opt-in and missing-identity semantics remain intact. Best-effort session correlation, not complete billing; unknown background ownership remains unlabelled. No assertion of a measured negligible token fraction. No remote CI status contexts are configured on these heads; local review/tests are the gate. No deployment, config change, production metric probe or restart.

Approved for the requested coordinated normal merges preserving source commit identity.

Hakalya exact-head review: PASS at `5c8b55402fd773fc7809a6b697b9f7ad8e3ba5fe`. Targeted closure review of the previously reported auxiliary ownership paths: scoped normalization preserves session/cache scope; manual compression binds ownership inside the real worker; worker/caller rotation uses copy-on-write identity refresh without changing provider routing or logical cache affinity; image pre-analysis takes the already-opened store session and clears unknown identity. Previous blocker is resolved. No new tracker, global identity fallback or infrastructure required. Independent verification against Core `3bcf1beb256a2940aa62675cb2bff644c6271d0b` and plugin `5c8b55402fd773fc7809a6b697b9f7ad8e3ba5fe`: canonical Core runner **400 passed / 0 failed** across the 8 documented files, including actual SessionDB rotation, manual worker ownership, copied-context isolation and concurrent vision; complete plugin suite **83 passed**, including real async export-thread → SDK → local HTTP → OTLP protobuf integration. Diff check clean. These are my executed counts, not a claim to have rerun the author's additional 452/53 matrices. Default-off / strict opt-in and missing-identity semantics remain intact. Best-effort session correlation, not complete billing; unknown background ownership remains unlabelled. No assertion of a measured negligible token fraction. No remote CI status contexts are configured on these heads; local review/tests are the gate. No deployment, config change, production metric probe or restart. Approved for the requested coordinated normal merges preserving source commit identity.

HoroTW merged commit efc2dc8698 into main 2026-09-19 19:29:08 +00:00

HoroTW referenced this pull request from a commit 2026-09-19 19:29:11 +00:00

Merge pull request #10: feat: optional session IDs in OTEL token metrics

HoroTW commented 2026-09-19 19:29:34 +00:00

Owner

Copy link

Merged normally as efc2dc86989bd579e951e1839e87fb5472603326 into main. Post-merge API + Git readback verified: canonical branch points to this merge; reviewed head is preserved as a merge parent/ancestor; merged tree exactly equals the tested head tree. Companion PR also merged. PASS reviews are attached to the exact heads. No deployment, config change or restart.

Merged normally as `efc2dc86989bd579e951e1839e87fb5472603326` into `main`. Post-merge API + Git readback verified: canonical branch points to this merge; reviewed head is preserved as a merge parent/ancestor; merged tree exactly equals the tested head tree. Companion PR also merged. PASS reviews are attached to the exact heads. No deployment, config change or restart.

Sign in to join this conversation.

Reviewers

No reviewers

HoroTW

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

HoroTW/hermes-otel-usage!10

No description provided.