feat(s2n-quic-dc): implement metrics for s2n-quic-dc by boquan-fang · Pull Request #2884 · aws/s2n-quic

boquan-fang · 2025-11-05T20:10:14Z

Release Summary:

Implement s2n-quic-dc-metrics crate to emit metrics for s2n-quic-dc.

Resolved issues:

Description of changes:

We implement a highly optimized crate to emit metrics for s2n-quic-dc events.

Call-outs:

The implementation has some methods that are only stable after rust version 1.88.0. Hence, I made the decision to keep the MSRV for s2n-quic and s2n-quic-dc to be 1.84.0, and make the MSRV for the new s2n-quic-dc-metrics crate to be 1.88.0.
- Specifically for this:
```
  --> dc/s2n-quic-dc-metrics/src/counter.rs:29:37
   |
29 |         let (chunks, tail) = events.as_chunks::<8>();
```
- Because of that decision, I have to tweak our CI a little bit:
  - Update the nightly toolchain to be nightly-2025-05-29 which will include the as_chunks method.
  - Create a .clippy.toml file for the new crate specifically so that the rust version will be 1.88.0 for the new crate during clippy cheks.
  - Skip the new crate when the rust version is 1.84.0 (s2n-quic's MSRV).

Testing:

CI test should include all of s2n-quic-dc-metrics's tests, and they should pass.

     Running unittests src/lib.rs (target/debug/deps/s2n_quic_dc_metrics-ca2245c14d8becc3)
running 22 tests
test appender::test::flush_on_drop ... ok
test appender::test::test_reuse ... ok
test appender::test::test_rotation ... ok
test appender::test::test_configured_rotation ... ok
test appender::test::test_time_rollback ... ok
test rseq::tests::check_per_cpu ... ok
test rseq::tests::test_send_event_local ... ok
test rseq::tests::check_send_branches ... ok
test rseq::tests::check_send_slow_branches ... ok
test summary::bucket::tests::idx_to_lower_bound ... ok
test summary::bucket::tests::idx_to_upper_bound ... ok
test rseq::tests::test_thread_ctor_dtor ... ok
test summary::bucket::tests::sizes ... ok
test summary::bucket::tests::total_buckets ... ok
test summary::bucket::tests::value_to_idx ... ok
test summary::test::config ... ok
test summary::test::count_correct ... ok
test counter::basic ... ok
test summary::test::maximum ... ok
test rseq::tests::test_send_event_overflow ... ok
test test::counters_for_enum_counts ... ok
test summary::test::visits_all_buckets ... ok
test result: ok. 22 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

Also, I test it with deployment.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

version

Mark-Simulacrum · 2025-11-05T22:30:14Z

re:MSRV, we would need Rust 1.87 for asm goto (https://blog.rust-lang.org/2025/05/15/Rust-1.87.0/#asm-jumps-to-rust-code) regardless, I think, so keeping this at 1.88 seems fine to me.

WesleyRosenblum · 2025-11-08T05:31:53Z

.github/workflows/ci.yml

  # Pin the nightly toolchain to prevent breakage.
  # This should be occasionally updated.
-  RUST_NIGHTLY_TOOLCHAIN: nightly-2025-03-31
+  RUST_NIGHTLY_TOOLCHAIN: nightly-2025-06-29


I think this can just be updated to the very latest one

Seems like once I update the nightly toolchain to the latest, the miri test on Github for s2n-quic-core will break on the w_cubic test:

thread 'recovery::cubic::tests::w_cubic' (4012) panicked at quic/s2n-quic-core/src/recovery/cubic/tests.rs:53:5: assertion `left == right` failed left: 11.999997139s right: 12s

Seems like the result that we got is 11.99999 which is extremely close to the expected one. I ran it on my local machine and it also failed by a tiny deviation:

thread 'recovery::cubic::tests::w_cubic' (1052900) panicked at quic/s2n-quic-core/src/recovery/cubic/tests.rs:53:5: assertion `left == right` failed left: 12.000000954s right: 12s

I wonder if that would be the reason why we don't use the latest nightly tool. We should probably keep using nightly-2025-06-29 and wait to see if a even newer version will resolve the problem?

I tried different versions of rust nightly toolchain and found out that nightly-2025-08-01 would work for our test. I can update the version to that version. It's not the latest, but close to our goal.

dc/s2n-quic-dc-metrics/Cargo.toml

WesleyRosenblum · 2025-11-08T05:40:02Z

dc/s2n-quic-dc-metrics/src/task.rs

@@ -0,0 +1,257 @@
+// Copyright (c) 2022 Tokio Contributors


is our copyright checking script not covering this crate?

According to our copyright check, this would fulfill the requirements:

s2n-quic/scripts/copyright_check

Lines 14 to 16 in cf77e2b

for file in $S2N_QUIC_FILES; do

# The word "Copyright" should appear at least once in the first 3 lines of every file

COUNT=`head -3 $file | grep "Copyright" | wc -l`;

The word Copyright is indeed in the first three lines of this file.

We can make improvement to the check: maybe we should check for Copyright Amazon.com, Inc. or its affiliates.? I don't think that is in scope with this PR. I would recommend to manually fix the copyright for this PR, cut a issue to track this problem, and fix it in a separate PR.

I have cut a ticket to track this issue: #2889.

* update nightly toolchain * Cargo toml project comments * update copyright

WesleyRosenblum · 2025-11-10T18:18:10Z

.github/workflows/ci.yml

        env: [default]
        include:
+          - rust: ${{ needs.env.outputs.msrv }}
+            # s2n-quic-dc-metrics crate' MSRV is 1.88.0, which is higher than the rest


can you open an issue to clean this up when we are on 1.88.0 for the rest of s2n-quic?

Yes, I have created one: #2888.

* I checked the latest version has very slight calculation errors for recovery::cubic::tests::w_cubic test

Boquan Fang and others added 3 commits November 5, 2025 20:09

feat(s2n-quic-dc): implement metrics for s2n-quic-dc

f56d683

chore: fixing code to integrate s2n-quic-dc-metrics into CI

5470424

chore: address copyrights, clippy.toml format, and update nightly

6d0247e

version

boquan-fang marked this pull request as ready for review November 5, 2025 21:47

boquan-fang requested a review from a team as a code owner November 5, 2025 21:47

boquan-fang requested review from Mark-Simulacrum and camshaft November 5, 2025 21:48

boquan-fang requested review from WesleyRosenblum and maddeleine November 6, 2025 17:39

chore: add a mention of we modify the bucket.rs source code

d14936d

boquan-fang force-pushed the boquan-fang/impl-metrics branch from a5f216f to d14936d Compare November 6, 2025 23:28

chore: update nightly toolchain

096f981

boquan-fang force-pushed the boquan-fang/impl-metrics branch from ef401c4 to 096f981 Compare November 7, 2025 07:57

WesleyRosenblum reviewed Nov 8, 2025

View reviewed changes

boquan-fang force-pushed the boquan-fang/impl-metrics branch from 0b463f3 to a232e04 Compare November 10, 2025 18:07

boquan-fang requested a review from WesleyRosenblum November 10, 2025 18:07

chore: address PR comments:

3920668

* update nightly toolchain * Cargo toml project comments * update copyright

boquan-fang force-pushed the boquan-fang/impl-metrics branch from a232e04 to 3920668 Compare November 10, 2025 18:17

WesleyRosenblum reviewed Nov 10, 2025

View reviewed changes

This was referenced Nov 10, 2025

MSRV 1.88.0 Tracking CI tests update #2888

Closed

s2n-quic's Copyright checks improvement #2889

Closed

boquan-fang requested a review from WesleyRosenblum November 10, 2025 18:44

boquan-fang mentioned this pull request Nov 10, 2025

fix(ci): copyright check improvement #2890

Merged

chore(ci): update nightly toolchain version to 2025-08-01

6764bc0

* I checked the latest version has very slight calculation errors for recovery::cubic::tests::w_cubic test

boquan-fang force-pushed the boquan-fang/impl-metrics branch 2 times, most recently from 04318e1 to a0fc2ff Compare November 10, 2025 21:53

chore: use assert_delta for cubic tests and use the newest nightly rust

ec9f6a6

boquan-fang force-pushed the boquan-fang/impl-metrics branch from a0fc2ff to ec9f6a6 Compare November 10, 2025 21:54

WesleyRosenblum approved these changes Nov 10, 2025

View reviewed changes

boquan-fang merged commit 80f7de4 into main Nov 10, 2025
136 checks passed

boquan-fang deleted the boquan-fang/impl-metrics branch November 10, 2025 22:25

boquan-fang mentioned this pull request Nov 10, 2025

chore: s2n-quic 1.69.0 release #2891

Merged

dougch mentioned this pull request Nov 17, 2025

ci: exclude dc for timing #2896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(s2n-quic-dc): implement metrics for s2n-quic-dc#2884

feat(s2n-quic-dc): implement metrics for s2n-quic-dc#2884
boquan-fang merged 8 commits intomainfrom
boquan-fang/impl-metrics

boquan-fang commented Nov 5, 2025 •

edited

Loading

Uh oh!

Mark-Simulacrum commented Nov 5, 2025

Uh oh!

WesleyRosenblum Nov 8, 2025

Uh oh!

boquan-fang Nov 10, 2025

Uh oh!

boquan-fang Nov 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

WesleyRosenblum Nov 8, 2025

Uh oh!

boquan-fang Nov 10, 2025

Uh oh!

boquan-fang Nov 10, 2025

Uh oh!

WesleyRosenblum Nov 10, 2025

Uh oh!

boquan-fang Nov 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	for file in $S2N_QUIC_FILES; do
	# The word "Copyright" should appear at least once in the first 3 lines of every file
	COUNT=`head -3 $file \| grep "Copyright" \| wc -l`;

Conversation

boquan-fang commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Summary:

Resolved issues:

Description of changes:

Call-outs:

Testing:

Uh oh!

Mark-Simulacrum commented Nov 5, 2025

Uh oh!

WesleyRosenblum Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

boquan-fang Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

boquan-fang Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

WesleyRosenblum Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

boquan-fang Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

boquan-fang Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

WesleyRosenblum Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

boquan-fang Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

boquan-fang commented Nov 5, 2025 •

edited

Loading

boquan-fang Nov 10, 2025 •

edited

Loading