Skip to content

GT-587: emit the wire format collectors already understand, and pin the vocabulary that can move - #308

Open
beyondnetPeru wants to merge 2 commits into
developfrom
feat/gt-587-semconv-genai
Open

GT-587: emit the wire format collectors already understand, and pin the vocabulary that can move#308
beyondnetPeru wants to merge 2 commits into
developfrom
feat/gt-587-semconv-genai

Conversation

@beyondnetPeru

Copy link
Copy Markdown
Contributor

Closes GT-587 (P2, Evolith Core). Starting state verified: grep -rn "gen_ai" src → 0 hits. The plumbing was already there; only the vocabulary was private.

What landed

  • core-domain/src/evaluation/telemetry/semconv.ts — the pinned vocabulary, SEMCONV_VERSION = '1.41.1', plus a manifest the drift check reads. Zero imports: rule HXA-05 flags @opentelemetry/api inside domain/application layers, and opentelemetry-tracker-adapter.ts already duck-types for the same reason. The Core owns the words; the adapters own the spans.
  • gen-ai-evaluation.ts — pure EvaluationResult → gen_ai.evaluation.result events, one per evaluated kind plus overall. Scores normalised to 0..1; kinds with no honest score omit the attribute rather than inventing one.
  • core-apievaluation-telemetry.service.ts wired into both branches of evaluation.controller.ts.
  • mcp-servermcp.method.name / mcp.tool.name / mcp.session.id, and trace context lifted from _meta instead of the traceparent previously smuggled through tool arguments. Only the three W3C keys are lifted: _meta is an open map, and copying it wholesale would let a caller inject arbitrary propagation keys. The argument path survives as a fallback. _meta and extra.sessionId were previously destructured away in mcp-server.service.ts; both now cross.

mcp.protocol.version is read from the HTTP header because the SDK does not expose the negotiated version to a handler — an attribute nobody can source is left absent, not fabricated.

The semconv attributes are emitted alongside evolith.*, not instead of them: the existing names are already being collected.

Criterion 3 is the one that would be easy to fake

.harness/scripts/ci/51-validate-semconv-pin.mjs checks three independently-failable rules and reads the committed lockfile, not node_modules, so local install state cannot make it green. All three were observed red:

• SEMCONV_VERSION drift: the module declares '1.42.0', the lockfile installs '1.41.1'.
• ATTR_GEN_AI_EVALUATION_NAME drift: pinned as 'gen_ai.eval.name', upstream now 'gen_ai.evaluation.name'.
• ATTR_GEN_AI_REQUEST_MODEL is exported but absent from PINNED_SEMCONV_ATTRIBUTES…

Independently falsified by the integrating session: bumping the pinned version → exit 1 naming the drift; restored → exit 0. Guard 43 (empty-corpus fixture) RED 1/1 · reported a PASS 0; guard 42 43/43; self-test 6/6 including the green case, so an unconditionally-failing guard could not pass. Wired into docs.ymlci-cd.yml and the two other files contested by an open PR were left untouched.

The pin matters because that registry is still in Development status and can change underneath us.

Suites

core-domain 1464 passed / 131 suites · mcp-server 444 / 54 · core-api 181 / 32 · root tsc -b clean.

An environment finding that matters beyond this PR

This worktree's node_modules symlink resolved @beyondnet/* back to the MAIN checkout. Until it was fixed, every cross-package test in the worktree was measuring the main checkout's built code rather than the branch's — a false-green generator for any worktree agent. node_modules was rebuilt as a real directory with per-entry symlinks, worktree-local @beyondnet links, and the six nested node_modules (@nestjs/cache-manager lives only in those). All numbers above are post-fix.

Relatedly, dist/ is load-bearing: core-api and mcp-server jest resolve core-domain through its exports map to dist/, so a new module is invisible until built. The guard deliberately reads source, not dist, for exactly that reason.

Also fixed en route: inserting the new controller dependency mid-constructor shifted positional construction in specs (9 red tests). Moved to last, with a comment saying why.

🤖 Generated with Claude Code

`grep -rn "gen_ai" src` returned 0: the tracing plumbing existed and GT-546
emitted `evolith_*` metrics, but every attribute name was private, so nothing
Evolith produced could join against what a customer already collects — and
telemetry is not backfillable.

- core-domain gains a PURE pinned vocabulary (`evaluation/telemetry/semconv.ts`)
  and a total `EvaluationResult -> gen_ai.evaluation.result` mapper. No OTel
  import: rule HXA-05 forbids an observability SDK in the domain/application
  layer, so the Core owns the words and the adapters own the spans.
- core-api attaches one event per evaluated kind to the request span, on BOTH
  the canonical and inline branches (they return the same result, so they must
  emit the same signal).
- mcp-server reads trace context from the request's `_meta` (SEP-414) instead of
  a `traceparent` smuggled through tool ARGUMENTS, and stamps `mcp.method.name`,
  `mcp.tool.name`, `mcp.session.id` and `mcp.protocol.version`. Only the three
  W3C keys are lifted out of `_meta` — it is an open map and must not become an
  injectable propagation carrier. The argument path stays as a fallback so
  existing callers do not silently lose trace linkage.

Every semconv name rides ALONGSIDE the `evolith.*` ones, never instead of them.

The pin is the load-bearing part: the GenAI registry is Development-status, so
`51-validate-semconv-pin.mjs` fails on three independent conditions — the
declared revision drifting from package-lock.json, a pinned literal no longer
matching upstream, and a constant exported without a manifest entry. All three
were observed red before this landed, plus the empty-corpus fixture via guard 43.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@beyondnetPeru
beyondnetPeru requested a review from a team as a code owner July 31, 2026 02:30
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

📊 Bilingual Coverage Impact

PR Changes

  • Paired EN/ES files modified: 4
  • New EN files needing ES translation: 2

Repository Coverage

Metric Value
Total EN files 525
Total ES files 499
Paired files 0
Coverage 0%

⚠️ Action required: 2 new EN file(s) added without ES counterparts.

To create skeletons:

node .harness/scripts/generate-es-skeleton.mjs <file.md>

Generated by GitHub Actions

The only conflict was two sessions appending a step at the same point in
docs.yml — the semconv pin guard (this branch) and the Scorecard regression
self-test (GT-597, already on develop). Both are kept; they are independent
checks. The filename collision that would have made this dangerous was already
avoided by renumbering the Scorecard guard 51 -> 52 before it merged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant