Skip to content

feat: add per-piece download duration metric split by traffic type - #2017

Merged
gaius-qi merged 2 commits into
dragonflyoss:mainfrom
sophiephoon-canva:client-metrics-download-duration-parent-local
Aug 28, 2026
Merged

feat: add per-piece download duration metric split by traffic type#2017
gaius-qi merged 2 commits into
dragonflyoss:mainfrom
sophiephoon-canva:client-metrics-download-duration-parent-local

Conversation

@sophiephoon-canva

Copy link
Copy Markdown
Contributor

Description

Adds a new download_piece_duration_milliseconds histogram, labeled by type (LOCAL_PEER, REMOTE_PEER, BACK_TO_SOURCE), that records how long each piece download takes broken out by traffic source.

Currently, download_task_duration_milliseconds only reports latency at the whole-task level, and its task_type label reflects the TaskType enum (STANDARD/PERSISTENT/PERSISTENT_CACHE/CACHE) rather than where the data came from. There is no existing metric that lets you compare p2p latency against back-to-source latency against local-cache latency — this PR adds one.

Changes:

  • dragonfly-client-metric/src/lib.rs: adds the DOWNLOAD_PIECE_DURATION histogram (registered and reset alongside the other custom metrics), a collect_download_piece_duration_metrics helper, and a unit test.
  • dragonfly-client/src/resource/piece.rs: records the new metric in every piece-download code path that does real I/O:
    • download_from_local_into_range_reader / download_persistent_from_local_into_async_read / download_persistent_cache_from_local_into_async_read → LOCAL_PEER
    • download_from_parent / download_persistent_from_parent / download_persistent_cache_from_parent → REMOTE_PEER
    • download_from_source / download_persistent_from_source → BACK_TO_SOURCE (reuses the start_time already captured for backend_request_duration_milliseconds, so both metrics measure the identical window)

The stub functions that only tally traffic bytes for an already-completed local-cache hit (download_from_local, download_persistent_from_local, download_persistent_cache_from_local) are intentionally left untouched — they do no real I/O, so there's nothing meaningful to time there.

In every case, the timer starts immediately before the network/storage call being measured and stops immediately after, excluding rate-limiter waits and unrelated bookkeeping (piece-started/failed storage calls), so all three traffic types are measured over a comparable window.

Related Issue

#2016

Motivation and Context

We wanted a Grafana panel showing download latency split by p2p vs. back-to-source vs. local-cache for our Dragonfly client fleet. The existing task_type label on download_task_duration_milliseconds looked like it should provide this, but it's actually the TaskType enum (standard/persistent/cache) and is constant across normal workloads, so it can't answer that question. The traffic-source distinction (TrafficType) already exists on the byte-counter metric (download_traffic) but had no corresponding duration metric. This PR closes that gap without touching any existing metric's behavior.

Screenshots (if appropriate)

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.40%. Comparing base (b78b2be) to head (57e1f7c).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
dragonfly-client-metric/src/lib.rs 84.61% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2017      +/-   ##
==========================================
+ Coverage   54.18%   54.40%   +0.21%     
==========================================
  Files         101      101              
  Lines       27237    27427     +190     
==========================================
+ Hits        14759    14921     +162     
- Misses      12478    12506      +28     
Files with missing lines Coverage Δ
dragonfly-client/src/resource/piece.rs 36.23% <ø> (ø)
dragonfly-client-metric/src/lib.rs 76.98% <84.61%> (+0.17%) ⬆️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… write

Previously, `collect_download_piece_duration_metrics` was called immediately after the network transfer completed, before the piece was written to storage. This caused duration metrics to exclude storage write time and to be recorded even on storage failures. Now the metric is collected only inside the success branch after `storage` confirms the piece is written.

Signed-off-by: Gaius <gaius.qi@gmail.com>
@gaius-qi gaius-qi added the enhancement New feature or request label Aug 28, 2026
@gaius-qi gaius-qi added this to the v2.6.0 milestone Aug 28, 2026
@gaius-qi
gaius-qi enabled auto-merge (squash) August 28, 2026 03:03

@gaius-qi gaius-qi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@chlins chlins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@gaius-qi
gaius-qi merged commit 5667a89 into dragonflyoss:main Aug 28, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants