-
Notifications
You must be signed in to change notification settings - Fork 102
feat(telemetry): client transfer performance telemetry #919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
6f46dcb
984be35
e275ac0
a701369
28f2f6d
a95b389
0983538
ec6715b
06ac796
9e02529
ac3adae
d639ce6
4cd858c
aeec419
72681cb
d1a9f2c
6c824da
173d45a
218528b
a05ec97
cc9a9c4
8af0427
3d48e2d
9cf0fbf
94e478c
388238b
b7ddd15
92fbc1f
e33f4fe
cae4cef
3f1119a
6098134
f039f8b
fea0f48
edad220
cdbd48d
2c45835
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| # Client transfer telemetry | ||
|
|
||
| **Date**: 2026-07-28 | ||
| **Crates**: `xet-runtime` (new `telemetry` config group), `xet-client` | ||
| (`cas_client::telemetry`, `Client` trait), `xet-data` (`telemetry` module, session hooks) | ||
|
|
||
| ## What changed | ||
|
|
||
| The client now reports one performance summary per transfer to `POST /v1/telemetry` on the CAS | ||
| server (added server-side in `huggingface-internal/xetcas#1207`). Reporting is best-effort: it is | ||
| never retried, never surfaces an error, and never blocks data movement. | ||
|
|
||
| ### New config group: `telemetry` | ||
|
|
||
| | Field | Env var | Default | | ||
| |---|---|---| | ||
| | `enabled` | `HF_XET_TELEMETRY_ENABLED` | `true` | | ||
| | `heartbeat_after` | `HF_XET_TELEMETRY_HEARTBEAT_AFTER` | `300s` | | ||
| | `heartbeat_interval` | `HF_XET_TELEMETRY_HEARTBEAT_INTERVAL` | `300s` | | ||
| | `request_timeout` | `HF_XET_TELEMETRY_REQUEST_TIMEOUT` | `5s` | | ||
| | `final_flush_timeout` | `HF_XET_TELEMETRY_FINAL_FLUSH_TIMEOUT` | `2s` | | ||
| | `max_in_flight` | `HF_XET_TELEMETRY_MAX_IN_FLIGHT` | `4` | | ||
|
|
||
| `HF_HUB_DISABLE_TELEMETRY` and `HF_HUB_OFFLINE` also force it off, and win over | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove mentioning of |
||
| `HF_XET_TELEMETRY_ENABLED=1`. These are applied at the end of `XetConfig::with_env_overrides` | ||
| rather than through `ENVIRONMENT_NAME_ALIASES`, because that table maps names with identical | ||
| polarity and these are inverted. | ||
|
|
||
| ### `Client` trait gained a method — **with a default body** | ||
|
|
||
| ```rust | ||
| #[cfg(not(target_family = "wasm"))] | ||
| fn transfer_telemetry(&self) -> Option<Arc<TransferTelemetry>> { None } | ||
| ``` | ||
|
|
||
| **No existing implementor needs to change.** Only `RemoteClient` overrides it; the local, | ||
| in-memory, and simulation clients inherit `None` and report nothing. | ||
|
|
||
| Note the failure mode this creates: an override with a mistyped signature compiles cleanly and is | ||
| silently never called. `xet_data/tests/test_transfer_telemetry.rs` exists to catch that and should | ||
| not be deleted. | ||
|
|
||
| ### Session behavior | ||
|
|
||
| - `FileUploadSession::finalize_impl` now delegates to a new private `finalize_inner` and reports on | ||
| both the success and error paths. Public signatures are unchanged. | ||
| - `FileDownloadSession::finalize` likewise. | ||
| - **Both sessions gained a `Drop` impl**, emitting an `aborted`/`dropped` summary when the session | ||
| was never finalized. This is the only reporting path for `XetDownloadStreamGroup`, which holds a | ||
| download session and has no explicit `finish()`. Anything constructing these sessions in a | ||
| non-tokio context is unaffected: `Drop` returns early when there is no runtime handle. | ||
| - New public accessors: `FileDownloadSession::client()`, and | ||
| `TestEnvironment::telemetry_docs()` under the `simulation` feature. | ||
|
|
||
| ### Simulation server | ||
|
|
||
| `POST /v1/telemetry` is now routed by the local test server, and received documents are readable | ||
| via `LocalServer::telemetry_docs()` / `LocalTestServer::telemetry_docs()`. | ||
|
|
||
| ### New dependencies | ||
|
|
||
| `chrono` and `uuid` were added to `xet-client`, gated to non-wasm targets. | ||
|
|
||
| ## Why | ||
|
|
||
| We had no client-side view of upload/download performance, so a throughput regression shipped in | ||
| an `hf-xet` release was undetectable. Server-side metrics cover CAS request latency but not | ||
| end-to-end client throughput, dedup effectiveness, or where a transfer's wall time goes. | ||
|
|
||
| ## Notes for downstream agents | ||
|
|
||
| - **The metric key set is a contract.** `xet_data/src/telemetry/payload.rs` is the source of truth. | ||
| Adding a key is safe; changing an existing key's JSON type is not — Elasticsearch field mappings | ||
| are immutable once established, so a type change produces per-document indexing failures and | ||
| needs a reindex. `test_upload_key_set_is_exact` and `test_numeric_types_stable` enforce this. | ||
| - **All `f64` values must go through the finite guard** in that module. `serde_json` renders NaN | ||
| and infinity as `null`, and a single such document poisons the field's mapping. | ||
| - **No PII.** The payload carries no file names, paths, hashes, repository ids, or user ids; the | ||
| server derives identity from the request's JWT. | ||
| - A follow-up PR will add the generated schema artifacts (`telemetry/metrics.schema.json` and | ||
| `telemetry/es-index-template.json`) and a CI compatibility gate. Until then the const key lists | ||
| in the tests are the only thing pinning the contract. | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove mentioning of
HF_HUBenv vars as disabling telemetry is done programmatically.