Skip to content

feat(telemetry): generated metrics schema and CI compatibility gate - #922

Open
sirahd wants to merge 2 commits into
sira/client-transfer-telemetryfrom
sira/telemetry-schema-contract
Open

feat(telemetry): generated metrics schema and CI compatibility gate#922
sirahd wants to merge 2 commits into
sira/client-transfer-telemetryfrom
sira/telemetry-schema-contract

Conversation

@sirahd

@sirahd sirahd commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #919 — base is sira/client-transfer-telemetry, so the diff here is just this change. Merge #919 first; the base will retarget to main automatically.

Publishes the telemetry metric vocabulary as a generated, committed JSON Schema, and enforces the compatibility rules consumers depend on. This is PR 2 of 2.

telemetry/metrics.schema.json is the source of truth for what we collect

All 65 properties carry a description of what they measure and their unit, so a consumer needs nothing from this repo's source to interpret a document:

"defrag_prevented_dedup_bytes": {
  "description": "Bytes that could have been deduplicated but were re-uploaded anyway,
                  because doing otherwise would have fragmented the xorb past the defrag
                  threshold. The cost of the fragmentation-vs-dedup tradeoff.",
  "type": "integer", "format": "uint64", "minimum": 0
}

A test fails the build if any property reaches the schema without a description. A metric documented only by its name is a metric nobody outside this repo can read.

xet_data/src/telemetry/payload.rs remains the source of truth for the code; the schema is generated from it and must never be hand-edited:

UPDATE_TELEMETRY_SCHEMA=1 cargo test -p xet-data --lib telemetry::schema

Without that variable, the same test asserts the committed file is current — so a payload change that forgets to regenerate fails cargo test.

schemars is a dev-dependency and the derives are cfg(test); cargo tree -p xet-data -e normal shows zero occurrences of it or its transitive deps, so nothing ships.

Scope: what this repo publishes

What the client emits — property names, JSON types, meanings.

It deliberately does not describe how any consumer stores, indexes, or aggregates these documents. That's the consumer's concern, this repo is public, and it has no way to keep a description of someone else's storage layer correct. A consumer types its own storage from each property's type:

JSON Schema Meaning
"integer" fits an unsigned 64-bit integer
"number" finite double; never NaN or infinity
"boolean"
"string" short, low-cardinality except transfer_id

Every value is a scalar: never null, never nested, never an array.

Two things the api_changes note flags for whoever builds the ingestion side: the numeric properties must be stored as numbers, or throughput and duration can't be averaged or range-queried — which is the entire point of collecting them. And the mapping should tolerate an unknown property, since consumers pin a tag and a client can ship ahead of a consumer rebuild.

CI gate

scripts/check_telemetry_schema_compat.py diffs the branch schema against the merge target in a new telemetry-schema-compat job.

Change Verdict
Property added pass
Property removed fail
Property type changed fail

A missing baseline is treated as the introducing commit. Checkout needs fetch-depth: 0 — the shallow default can't read the base ref.

Testing

6 schema tests: drift, every property documented, scalars only, both directions present, cross-direction type agreement, alerting metrics are numeric. I verified the drift test actually fires by tampering with the committed artifact, and exercised the CI script against all six cases (identical / missing baseline / added / removed / retyped / malformed).

  • cargo test --features "strict simulation internal-tools" — 38 test binaries, all green
  • cargo clippy -r --features "strict simulation internal-tools" — clean
  • cargo +nightly fmt — clean

Changed since first review

Dropped telemetry/es-index-template.json. It described the receiving service's document shape and storage layout, which doesn't belong in a public repo. The generated JSON Schema now carries the type information a consumer needs to derive its own mapping, and the storage-specific guidance moved to prose in the api_changes note. Related wording elsewhere in these files was made storage-agnostic for the same reason.

Note: api_changes/update_260728_client_transfer_telemetry.md from #919 still names the storage technology in several places. I left it alone rather than editing a file from an open PR — say the word and I'll clean it up in either PR.


Note

Low Risk
Mostly contract, tests, and CI; runtime telemetry behavior is unchanged and schemars stays dev-only.

Overview
Introduces a published, generated contract for client transfer telemetry: committed telemetry/metrics.schema.json is built from xet_data payload types (via test-only schemars + schema.rs), not hand-edited. Payload fields gain /// doc comments that become schema descriptions; tests require every property to be documented and keep the committed file in sync (UPDATE_TELEMETRY_SCHEMA=1 to regenerate).

Adds scripts/check_telemetry_schema_compat.py and a telemetry-schema-compat CI job that diffs the branch schema against the merge target: new metrics allowed, removals or JSON type changes fail. .gitattributes pins the schema to LF for stable byte comparisons on Windows.

Documents the contract in api_changes/update_260729_telemetry_schema_contract.md (pin-by-tag URL, regeneration, consumer typing rules). Cargo.lock updates reflect schemars as a dev-only dependency path.

Reviewed by Cursor Bugbot for commit 7819742. Bugbot is set up for automated code reviews on this repo. Configure here.

@sirahd
sirahd force-pushed the sira/telemetry-schema-contract branch from 6eaefb6 to d309473 Compare July 30, 2026 00:02
Comment thread xet_data/src/telemetry/schema.rs Outdated
@sirahd
sirahd force-pushed the sira/telemetry-schema-contract branch from d309473 to 8e75241 Compare July 30, 2026 19:21
@sirahd sirahd changed the title feat(telemetry): generated schema contract and CI compatibility gate feat(telemetry): generated metrics schema and CI compatibility gate Jul 30, 2026
Comment thread scripts/check_telemetry_schema_compat.py
@sirahd
sirahd force-pushed the sira/telemetry-schema-contract branch 4 times, most recently from ebbb146 to 45a0729 Compare July 31, 2026 00:00

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 45a0729. Configure here.

Comment thread xet_data/src/telemetry/schema.rs
sirahd and others added 2 commits July 31, 2026 14:22
Publishes the telemetry metric vocabulary as a generated, committed JSON
Schema and enforces the compatibility rules consumers depend on.

telemetry/metrics.schema.json is the source of truth for what the client
collects: every one of the 65 properties carries a description of what it
measures and its unit, so a consumer needs nothing from this repo's source
to interpret a document. A test fails the build if any property reaches the
schema without one - a metric documented only by its name is a metric nobody
outside this repo can read.

xet_data/src/telemetry/payload.rs remains the source of truth for the code;
the schema is generated from it and must never be hand-edited:

    UPDATE_TELEMETRY_SCHEMA=1 cargo test -p xet-data --lib telemetry::schema

Without that variable the same test asserts the committed file is current,
so a payload change that forgets to regenerate fails the build.

schemars is a dev-dependency and the JsonSchema derives are cfg(test):
`cargo tree -e normal` shows zero occurrences, so nothing ships.

Scope: this publishes what the client *emits* - property names, JSON types,
and meanings. It deliberately does not describe how any consumer stores or
indexes those documents. That is the consumer's concern, this repo is
public, and it has no way to keep a description of someone else's storage
layer correct. A consumer types its own storage from each property's `type`;
the schema states the compatibility rules that make that safe, and the
api_changes note spells out that the numeric properties must be stored as
numbers or the alerting cannot work.

CI gate (scripts/check_telemetry_schema_compat.py, telemetry-schema-compat
job) diffs the branch schema against the merge target: added property
passes, removed or retyped fails. A missing baseline is treated as the
introducing commit. Checkout uses fetch-depth 0, since the shallow default
cannot read the base ref. Verified against all six cases including
malformed input.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_committed_schema_is_current` compares the committed
`telemetry/metrics.schema.json` byte-for-byte against a freshly generated
string. Git checks that file out with CRLF on Windows under the default
`core.autocrlf=true`, while the generated string always uses `\n`, so the
comparison failed there and only there - reporting the schema as out of date
when it was identical.

This was latent: `build_and_test-win` was already failing to compile for an
unrelated reason, so the test never got far enough to run. Fixing that compile
error surfaced this.

Normalizes line endings before comparing, and adds a `.gitattributes` entry
pinning the file to LF. The test does not rely on the latter, since
`.gitattributes` only governs fresh checkouts.

Verified by rewriting the committed schema with CRLF locally: the test fails
without the normalization and passes with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sirahd
sirahd force-pushed the sira/telemetry-schema-contract branch from 82a233c to 7819742 Compare July 31, 2026 21:23

@seanses seanses left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand this PR, what's the purpose of generating a JSON schema? We don't expect users to consume or use the telemetry details right?

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.

2 participants