Bug 2043832 - happy-eyeballs glean metrics for https rr#2
Closed
mxinden-bot wants to merge 1 commit into
Closed
Conversation
mxinden
reviewed
May 30, 2026
Comment on lines
+351
to
+361
| let summary = metrics::HttpsRecordSummary { | ||
| has_records: !infos.is_empty(), | ||
| advertises_h3: infos.iter().any(|i| { | ||
| i.alpn_http_versions | ||
| .contains(&happy_eyeballs::HttpVersion::H3) | ||
| }), | ||
| has_ech: infos.iter().any(|i| i.ech_config.is_some()), | ||
| has_ipv4hint: infos.iter().any(|i| !i.ipv4_hints.is_empty()), | ||
| has_ipv6hint: infos.iter().any(|i| !i.ipv6_hints.is_empty()), | ||
| }; | ||
| self.metrics.dns_response_https(id, summary); |
There was a problem hiding this comment.
this should live in metrics. pass the record there. then do all the accounting in metrics.rs
Owner
Author
There was a problem hiding this comment.
Done in 1f427f7. lib.rs now just passes the &[ServiceInfo] slice to Metrics::dns_response_https, and all the h3/ECH/hint accounting happens in metrics.rs.
Generated by Claude Code
Comment on lines
+243
to
+262
| happy_eyeballs_https_rr_features: | ||
| type: labeled_counter | ||
| description: > | ||
| Happy Eyeballs: which connection-bootstrapping features a received | ||
| HTTPS DNS record carried. Incremented once per feature present, per | ||
| connection that saw a non-empty HTTPS record. "ech" and the address | ||
| hints can only be delivered ahead of the first connection via the DNS | ||
| record, not via an Alt-Svc header. | ||
| labels: | ||
| - h3_alpn | ||
| - ech | ||
| - ipv4hint | ||
| - ipv6hint | ||
| bugs: | ||
| - https://bugzilla.mozilla.org/show_bug.cgi?id=2026606 | ||
| data_reviews: | ||
| - https://bugzilla.mozilla.org/show_bug.cgi?id=2026606 | ||
| notification_emails: | ||
| - necko@mozilla.com | ||
| expires: never |
There was a problem hiding this comment.
how would I know how many https records there were in total. can there be a total label?
Owner
Author
There was a problem hiding this comment.
Added a total label, incremented once per connection that saw a non-empty HTTPS record. It's the denominator, so e.g. ECH adoption among records = ech / total.
Generated by Claude Code
Comment on lines
+264
to
+281
| happy_eyeballs_winning_http_version: | ||
| type: labeled_counter | ||
| description: > | ||
| Happy Eyeballs: the HTTP version negotiated by the connection attempt | ||
| that ultimately succeeded. Combined with happy_eyeballs_h3_discovery, | ||
| shows how often a published HTTPS record let the first connection reach | ||
| h3. | ||
| labels: | ||
| - h3 | ||
| - h2 | ||
| - h1 | ||
| bugs: | ||
| - https://bugzilla.mozilla.org/show_bug.cgi?id=2026606 | ||
| data_reviews: | ||
| - https://bugzilla.mozilla.org/show_bug.cgi?id=2026606 | ||
| notification_emails: | ||
| - necko@mozilla.com | ||
| expires: never |
Owner
Author
There was a problem hiding this comment.
mxinden-bot
force-pushed
the
claude/jolly-ride-y9jF8
branch
from
8fb98ad to
581a4c2
Compare
mxinden-bot
changed the title
mxinden
reviewed
May 30, 2026
Comment on lines
+75
to
+77
| let alt_svc_h3 = alt_svc_vec | ||
| .iter() | ||
| .any(|a| matches!(a.http_version, happy_eyeballs::HttpVersion::H3)); |
Owner
Author
There was a problem hiding this comment.
Moved in fdaf105. lib.rs now passes &alt_svc_vec to Metrics::new, which derives the alt_svc_h3 flag itself — same as the HTTPS-record accounting.
Generated by Claude Code
Comment on lines
+230
to
+236
| Happy Eyeballs: which connection-bootstrapping features a received | ||
| HTTPS DNS record carried. The "total" label is incremented once per | ||
| connection that saw a non-empty HTTPS record and serves as the | ||
| denominator; each feature label is incremented once per connection | ||
| whose HTTPS record carried that feature. "ech" and the address hints | ||
| can only be delivered ahead of the first connection via the DNS record, | ||
| not via an Alt-Svc header. |
There was a problem hiding this comment.
Maybe stress that this is the combination of all https records for that connection
Owner
Author
There was a problem hiding this comment.
Reworded the description to stress the features are the union across all HTTPS records received for the connection, not per-record.
Generated by Claude Code
Extends the Happy Eyeballs glean instrumentation (bug 2026606) with two labeled counters so we have field visibility into how Firefox discovers HTTP/3 and what the HTTPS DNS records we receive contain. - happy_eyeballs_h3_discovery: per run, crosses whether Alt-Svc advertised h3 with whether the HTTPS record advertised h3 in its ALPN set (none / altsvc_only / https_rr_only / both). The 'altsvc_only' bucket measures how often an origin supports h3 but only advertises it via Alt-Svc, forcing the first connection to spend an extra round trip discovering h3 that a published HTTPS record would have avoided. - happy_eyeballs_https_rr_features: which bootstrapping features a received HTTPS record carried (h3 ALPN, ECH, ipv4hint, ipv6hint), against a 'total' label that counts connections seeing a non-empty record as the denominator. The glue captures the Alt-Svc h3 flag at creation and passes the received HTTPS records to metrics.rs, where all accounting lives. Drops the now redundant happy_eyeballs_https_record_available, whose available/unavailable split is derivable from the two new metrics.
mxinden-bot
force-pushed
the
claude/jolly-ride-y9jF8
branch
from
581a4c2 to
fdaf105
Compare
mxinden-bot
pushed a commit
that referenced
this pull request
Jun 2, 2026
Upstream commit: https://webrtc.googlesource.com/src/+/7bb61f8b76569daacb24a105286400b32ab40990 [M149] Guard RestoreTokenManager add/reads with Mutex Original change's description: > Guard RestoreTokenManager add/reads with Mutex > > The RestoreTokenManager is a singleton who's adds and reads could be > accessed by multiple threads simultaneously. To prevent any potential > issues/collisions, ensure that such accesses happen behind a lock. > > Bug: chromium:513049286 > Change-Id: I44c1c7977a6d02e1ac3fbe00e6ffc7bc22e41e46 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/472480 > Reviewed-by: Mark Foltz <mfoltz@chromium.org> > Auto-Submit: Alexander Cooper <alcooper@chromium.org> > Commit-Queue: Mark Foltz <mfoltz@chromium.org> > Cr-Commit-Position: refs/heads/main@{#47711} (cherry picked from commit b9f0b184cb5a487fcb4910efaf4f84483ddba284) Bug: chromium:514929759,chromium:513049286 Change-Id: I44c1c7977a6d02e1ac3fbe00e6ffc7bc22e41e46 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/474080 Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Chrome Cherry Picker <chrome-cherry-picker@chops-service-accounts.iam.gserviceaccount.com> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/branch-heads/7827@{#2} Cr-Branched-From: d606bc991592fbf4dcbe85e9d05db5e501a5ad42-refs/heads/main@{#47595}
mxinden-bot
pushed a commit
that referenced
this pull request
Jun 2, 2026
…ions and bracket finishers r=android-reviewers,moyin The pager previously sorted cards into three coarse buckets (LIVE > PAST > UPCOMING) with chronological order within each. This buried the most-relevant cards in several end-of-tournament scenarios -- the champion card landed last in a long followed-team pager; an upcoming Final the user's team is about to play landed at the very end; and the no-team view showed TPP before Final purely because Saturday precedes Sunday. Replace orderedForPager with a single rule applied to both views: 1. Live matches (chronological) 2. Champion card (Final decided, TournamentWinner outcome) 3. Third Place card (TPP decided, ThirdPlace outcome; independent of #2) 4. Upcoming bracket-finishing matches by date (TPP Sat before Final Sun) 5. Other past matches reverse-chronologically, so the team's most recent KO round sits next to the celebrations rather than buried under group-stage history 6. Other upcoming matches chronologically Adds seven scenario tests covering the followed-team and no-team paths across the pre-tournament, mid-tournament, between-TPP-and-Final, and post-Final phases. Differential Revision: https://phabricator.services.mozilla.com/D303821
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
2 participants
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.
Superseded by #3 (branch renamed
claude/jolly-ride-y9jF8→happy-https-rr-metrics). All review feedback here is incorporated in #3. Closing; the old branch can't be deleted automatically (protected), so it lingers but is unused.Summary
Extends the Happy Eyeballs glean instrumentation (bug 2026606) with two labeled counters for HTTP/3 discovery source and HTTPS-record contents. See #3 for the current version.
🤖 Generated with Claude Code