HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083

Merged
ManickaP merged 15 commits into
dotnet:mainfrom
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged

HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom
antonfirsov:h3-telemetry-0

Conversation

@antonfirsov

@antonfirsovantonfirsov commented Mar 31, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.

Fixes#104783.

@antonfirsovantonfirsov added this to the 10.0.0 milestone Mar 31, 2025
@antonfirsov
antonfirsov requested review from a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI reviewed Mar 31, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.

  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs Outdated
@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg mentioned this pull request Apr 3, 2025
Closed
This was referenced Apr 1, 2025
Open
Open
ManickaP
ManickaP reviewed Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs Outdated
This was referenced Apr 4, 2025
Closed
Closed
Closed
Closed
This was referenced Apr 12, 2025
Closed
Closed
ManickaP
ManickaP reviewed Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs Outdated
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs Outdated
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
This was referenced Apr 23, 2025
Open
Open
ManickaP
ManickaP approved these changes Apr 24, 2025
View reviewed changes

@ManickaPManickaP 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, thanks!
:shipit:
If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.

@ManickaP

ManickaP commented Apr 24, 2025

Copy link
Copy Markdown
Member

@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?

@pavelsavara

pavelsavara commented Apr 24, 2025

Copy link
Copy Markdown
Member

Would be able to help with resolving it?

Probably the new code keeps the metrics classes alive.

You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml

I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.

@pavelsavara

pavelsavara commented Apr 24, 2025

Copy link
Copy Markdown
Member

.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025
Closed
Closed
@antonfirsov

antonfirsov commented Apr 24, 2025

Copy link
Copy Markdown
ContributorAuthor

The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!

@antonfirsov

antonfirsov commented Apr 24, 2025

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Apr 24, 2025

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025
Closed
Closed
@ManickaP
ManickaP merged commit 87bf42d into dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry

4 participants

@antonfirsov@ManickaP@pavelsavara
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals - #114083
Merged
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Apr 25, 2025
Merged
HTTP/3: Eliminate near-zero "wait for connection" telemetry signals#114083
ManickaP merged 15 commits into
dotnet:mainfrom 
antonfirsov:h3-telemetry-0
Conversation
@antonfirsov
@antonfirsovantonfirsov
commented
Mar 31, 2025•

edited

Loading

Copy link


Copy Markdown

Contributor
We should only emit wait for connection activities, metrics and events when (1) there is no HTTP/3 connection (2) OpenOutboundStreamAsync() is expected to wait for available streams. When there is a usable connection in the pool, emitting near-zero spans is not consistent with telemetry for other HTTP versions and adds noise.
Fixes#104783.
@antonfirsovantonfirsov
added this to the 10.0.0 milestone
Mar 31, 2025
@antonfirsov
antonfirsov
requested review from
a team, ManickaP and CopilotMarch 31, 2025 21:30
CopilotAI
reviewed
Mar 31, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This pull request refactors the "wait for connection" telemetry logic for HTTP/3, ensuring such telemetry is only emitted for new connections where OpenOutboundStreamAsync() is expected to wait.
  • Updated test cases in MetricsTest.cs and DiagnosticsTests.cs to reflect the new telemetry behavior.
  • Refactored Http3Connection.SendAsync and associated telemetry tracking via a new WaitForHttp3ConnectionActivity structure.
  • Adjusted HttpConnectionPool.Http3.cs to integrate the new telemetry mechanism and removed obsolete timestamp parameters.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
FileDescription
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.csAdded a test to ensure queue time is measured only for the first connection.
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.csUpdated expected telemetry count in diagnostics tests.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.csRefactored SendAsync to use WaitForHttp3ConnectionActivity and updated telemetry logic.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.csAdded a comment to clarify custom telemetry behavior for HTTP/3 connections.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.csSwitched to the new telemetry mechanism and removed obsolete parameters.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.csExposed a new method to check for telemetry listeners.
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

@azure-pipelines

This comment was marked as outdated.

Sign in to view
@kgkg
mentioned this pull request
Apr 3, 2025

Closed


This was referenced Apr 1, 2025

Open


Open

ManickaP
ManickaP
reviewed
Apr 3, 2025
View reviewed changes
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated


This was referenced Apr 4, 2025

Closed


Closed


Closed


Closed


This was referenced Apr 12, 2025

Closed


Closed

ManickaP
ManickaP
reviewed
Apr 15, 2025
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Outdated

Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs
Outdated

Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
Comment thread...m.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs

This was referenced Apr 23, 2025

Open


Open

ManickaP
ManickaP
approved these changes
Apr 24, 2025
View reviewed changes
@ManickaPManickaP
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, thanks!
:shipit:

If you don't merge it byt the end of the day because you're already OOF, I'll merge it myself.
@ManickaP
ManickaP
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
@pavelsavara Seems like your new test in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs is failing on this PR. Would be able to help with resolving it?
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
Would be able to help with resolving it?
Probably the new code keeps the metrics classes alive.
You can run the test and see which dependencies are doing it by looking at artifacts\bin\trimmingTests\projects\System.Net.Http.TrimmingTests\MetricsHandlerTrimmedTest\win-x64\obj\Release\net10.0\win-x64\linked\linker-dependencies.xml
I suspect that the new code in WaitForHttp3ConnectionActivity is touching Settings._metrics without GlobalHttpSettings.MetricsHandler.IsGloballyEnabled guard. The trimmer is not very smart.
@pavelsavara
pavelsavara
commented
Apr 24, 2025

Copy link


Copy Markdown

Member
.\dotnet.cmd build src\libraries\System.Net.Http\tests\TrimmingTests\System.Net.Http.TrimmingTests.proj -c Release /p:TestTrimming=true

This was referenced Apr 24, 2025

Closed


Closed

@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
The trimmer issue should be fixed by cf00150. Good that #114326 included all the types in the trimming tests!
@antonfirsov
antonfirsov
commented
Apr 24, 2025

Copy link


Copy Markdown

ContributorAuthor
/azp run runtime-libraries-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
Apr 24, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 24, 2025

Closed


Closed

@ManickaP
ManickaP
merged commit 87bf42d
into
dotnet:mainApr 25, 2025
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
May 25, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ManickaPManickaPManickaP approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Http

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

[H/3] Distributed Tracing / Telemetry


4 participants

@antonfirsov@ManickaP@pavelsavara