Record SessionSource and latency for DoH attempts per provider [chromium/src : main]

[Kenichi Ishibashi (Gerrit)]

Attention needed from Andrew Williams and Nidhi Jaju

Kenichi Ishibashi voted and added 1 comment

Votes added by Kenichi Ishibashi

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Kenichi Ishibashi . resolved

awillia@: PTAL overall
nidhijaju@: PTAL histograms.xml

Open in Gerrit

Related details

Attention is currently required from:

• Andrew Williams
• Nidhi Jaju

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I989902ddae06e49aa6c4e411e8dea2be737ea6ca

Gerrit-Change-Number: 7580339

Gerrit-PatchSet: 3

Gerrit-Owner: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Andrew Williams <awi...@chromium.org>

Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>

Gerrit-Attention: Andrew Williams <awi...@chromium.org>

Gerrit-Comment-Date: Thu, 19 Feb 2026 04:10:12 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Kenichi Ishibashi (Gerrit)]

Attention needed from Andrew Williams and Nidhi Jaju

Kenichi Ishibashi added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Kenichi Ishibashi . resolved

stefanoduo@: FYI, as you've been touching DnsTransaction recently.

Open in Gerrit

Related details

Attention is currently required from:

• Andrew Williams
• Nidhi Jaju

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I989902ddae06e49aa6c4e411e8dea2be737ea6ca

Gerrit-Change-Number: 7580339

Gerrit-PatchSet: 3

Gerrit-Owner: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Andrew Williams <awi...@chromium.org>

Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Stefano Duo <stefa...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>

Gerrit-Attention: Andrew Williams <awi...@chromium.org>

Gerrit-Comment-Date: Thu, 19 Feb 2026 04:10:55 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Nidhi Jaju (Gerrit)]

Attention needed from Andrew Williams and Kenichi Ishibashi

Nidhi Jaju voted and added 2 comments

Votes added by Nidhi Jaju

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Nidhi Jaju . resolved

lgtm

File net/dns/resolve_context.cc

Line 319, Patchset 3 (Latest): HttpConnectionInfoCoarseToString(info.connection_info);

Nidhi Jaju . unresolved

Maybe we should include "net/http/http_connection_info.h" for IWYU.

Open in Gerrit

Related details

Attention is currently required from:

• Andrew Williams
• Kenichi Ishibashi

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I989902ddae06e49aa6c4e411e8dea2be737ea6ca

Gerrit-Change-Number: 7580339

Gerrit-PatchSet: 3

Gerrit-Owner: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Andrew Williams <awi...@chromium.org>

Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Stefano Duo <stefa...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Attention: Andrew Williams <awi...@chromium.org>

Gerrit-Comment-Date: Thu, 19 Feb 2026 09:41:56 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Stefano Duo (Gerrit)]

Attention needed from Andrew Williams and Kenichi Ishibashi

Stefano Duo added 1 comment

File net/dns/dns_attempt.h

Line 80, Patchset 3 (Latest): // Returns the HTTP information for this attempt, if it was an HTTP attempt.
virtual std::optional<DnsHttpAttemptInfo> GetHttpAttemptInfo() const;

Stefano Duo . unresolved

What if we instead passed ResolveContext (or some higher-level abstraction) to DnsHttpAttempt? This would let it call RecordDohSessionStatus internally. Without having to leak, within DnsTransaction (other than at DnsAttempt creation time), whether a DnsAttempt is a DnsHttpAttempt or something else.

I'm worried that over time DnsAttempt would become a set of partially implemented methods, making it hard to figure out whether it's safe to call something.

Gerrit-Comment-Date: Thu, 19 Feb 2026 10:12:23 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Kenichi Ishibashi (Gerrit)]

Kenichi Ishibashi added 1 comment

File net/dns/dns_attempt.h

Line 80, Patchset 3 (Latest): // Returns the HTTP information for this attempt, if it was an HTTP attempt.
virtual std::optional<DnsHttpAttemptInfo> GetHttpAttemptInfo() const;

Stefano Duo . unresolved

What if we instead passed ResolveContext (or some higher-level abstraction) to DnsHttpAttempt? This would let it call RecordDohSessionStatus internally. Without having to leak, within DnsTransaction (other than at DnsAttempt creation time), whether a DnsAttempt is a DnsHttpAttempt or something else.

I'm worried that over time DnsAttempt would become a set of partially implemented methods, making it hard to figure out whether it's safe to call something.

Kenichi Ishibashi

Yeah I considered passing ResolveContext DnsAttempt at first, but I had a feeling that the original implementer explicitly intended to make DnsAttempt independent (unaware) of ResolveContext, so took this approach.

awillia@ gave me initial feedback and I plan to rework this CL so I'll consider this approach as well.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I989902ddae06e49aa6c4e411e8dea2be737ea6ca

Gerrit-Change-Number: 7580339

Gerrit-PatchSet: 3

Gerrit-Owner: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Andrew Williams <awi...@chromium.org>

Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Stefano Duo <stefa...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Comment-Date: Thu, 19 Feb 2026 10:33:49 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Stefano Duo <stefa...@google.com>

[Kenichi Ishibashi (Gerrit)]

Attention needed from Andrew Williams and Stefano Duo

Kenichi Ishibashi added 3 comments

File net/dns/dns_attempt.h

Line 80, Patchset 3: // Returns the HTTP information for this attempt, if it was an HTTP attempt.


virtual std::optional<DnsHttpAttemptInfo> GetHttpAttemptInfo() const;

Stefano Duo . unresolved

What if we instead passed ResolveContext (or some higher-level abstraction) to DnsHttpAttempt? This would let it call RecordDohSessionStatus internally. Without having to leak, within DnsTransaction (other than at DnsAttempt creation time), whether a DnsAttempt is a DnsHttpAttempt or something else.

I'm worried that over time DnsAttempt would become a set of partially implemented methods, making it hard to figure out whether it's safe to call something.

Kenichi Ishibashi

Yeah I considered passing ResolveContext DnsAttempt at first, but I had a feeling that the original implementer explicitly intended to make DnsAttempt independent (unaware) of ResolveContext, so took this approach.

awillia@ gave me initial feedback and I plan to rework this CL so I'll consider this approach as well.

Kenichi Ishibashi

Changed to pass ResolveContext and DnsSession. Does this look better?

File net/dns/dns_http_attempt.cc

Line 335, Patchset 9 (Latest): if (!is_probe_ && resolve_context_ && session_ &&
session_->config().secure_dns_mode == SecureDnsMode::kAutomatic) {

Kenichi Ishibashi . unresolved

awillia@: Does this look what you suggested?

File net/dns/resolve_context.cc

Line 319, Patchset 3: HttpConnectionInfoCoarseToString(info.connection_info);

Nidhi Jaju . resolved

Maybe we should include "net/http/http_connection_info.h" for IWYU.

Kenichi Ishibashi

Done.

Open in Gerrit

Related details

Attention is currently required from:

• Andrew Williams
• Stefano Duo

Submit Requirements:

• Code-Coverage
• Code-Owners

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I989902ddae06e49aa6c4e411e8dea2be737ea6ca

Gerrit-Change-Number: 7580339

Gerrit-PatchSet: 9

Gerrit-Owner: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Andrew Williams <awi...@chromium.org>

Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>

Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Stefano Duo <stefa...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Stefano Duo <stefa...@google.com>

Gerrit-Attention: Andrew Williams <awi...@chromium.org>

Gerrit-Comment-Date: Sat, 21 Feb 2026 01:00:03 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Kenichi Ishibashi <ba...@chromium.org>

Comment-In-Reply-To: Stefano Duo <stefa...@google.com>

Comment-In-Reply-To: Nidhi Jaju <nidh...@chromium.org>