feat(telemetry): outcome vocabulary and transfer metric payloads - #934
Merged
Merged
Conversation
Defines what a transfer report contains: the outcome and error-class vocabulary, the shared identity and common metrics, and the upload- and download-specific metric sets, with the OpenAPI description of the endpoint that receives them. These definitions live in xet_data rather than xet_client because this is the only layer that can see DeduplicationMetrics and GroupProgressReport. The outcome vocabulary stays ungated on wasm even though the payloads do not: it appears in FileDownloadSession's public signatures, and gating it would push a cfg onto every caller that merely names an outcome. Nothing emits these yet - the emit path and its session hooks are the next PR.
sirahd
marked this pull request as ready for review
August 10, 2026 23:05
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a1e7ab7. Configure here.
| ClientError::FileNotFound(_) | ClientError::XORBNotFound(_) => "not_found", | ||
| ClientError::InternalError(_) => "internal", | ||
| ClientError::ReqwestMiddlewareError(_) => "network", | ||
| ClientError::ReqwestError(e, _) => reqwest_error_class(e), |
There was a problem hiding this comment.
Middleware timeouts misclassified
Medium Severity
ClientError::ReqwestMiddlewareError always maps to network, so the timeout path in reqwest_error_class never runs for transport failures. RetryWrapper turns send-time timeouts into middleware errors, so real timeouts are bucketed as network and the closed timeout class stays empty for the main client path.
Reviewed by Cursor Bugbot for commit a1e7ab7. Configure here.
jgodlew
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Part 3 of 6 of the client transfer telemetry stack, split out of #919 for review. Each PR in the stack compiles and passes CI on its own.
Defines what a transfer report contains: the outcome and error-class
vocabulary, the shared identity and common metrics, and the upload- and
download-specific metric sets, with the OpenAPI description of the endpoint
that receives them.
These definitions live in xet_data rather than xet_client because this is the
only layer that can see DeduplicationMetrics and GroupProgressReport.
The outcome vocabulary stays ungated on wasm even though the payloads do not:
it appears in FileDownloadSession's public signatures, and gating it would push
a cfg onto every caller that merely names an outcome.
Nothing emits these yet - the emit path and its session hooks are the next PR.
Review note:
xet_data/src/telemetry/mod.rsdeclares onlyoutcomeandpayloadhere. Part 4 addsmod emit;and its re-exports — that file is additive across the two PRs, not replaced.Note
Low Risk
Additive API spec and library types with no runtime behavior change until a later PR wires emission; schema is guarded by tests but future key/type changes would affect analytics consumers.
Overview
Introduces the client transfer telemetry contract ahead of wiring: OpenAPI documents fire-and-forget
POST /v1/telemetry(read scope) with aTelemetryEnvelopewhosemetricsmap is intentionally open-ended and points to Rust as the source of truth.Adds
xet_data::telemetrywith a wasm-universal outcome layer (Outcome, stable wire strings,error_class/classify_erroroverDataError) and non-wasm payload builders that flattenCommonMetrics,UploadMetrics(dedup/shard/ingest timing), andDownloadMetricsfromGroupProgressReport,DeduplicationMetrics, andTransferTelemetrysnapshots. Payload construction enforces fixed key sets, scalar-only JSON, and finite ratios/rates; tests lock upload/download keys, types, and no-PII rules.No emission in this PR—only types, serializers, and API spec; the next stack piece hooks sessions to send these documents.
Reviewed by Cursor Bugbot for commit a1e7ab7. Bugbot is set up for automated code reviews on this repo. Configure here.