disable/trim MetricsHandler when System.Diagnostics.Metrics.Meter.IsSupported is false #114326

pavelsavara · 2025-04-07T10:23:31Z

use System.Diagnostics.Metrics.Meter.IsSupported to disable/trim MetricsHandler
use System.Net.Http.EnableActivityPropagation to disable/trim DiagnosticsHandler

- use System.Diagnostics.Metrics.Meter.IsSupported - call EventSource.EnableActivityTracker() when DiagnosticsHandler is enabled

dotnet-policy-service · 2025-04-07T10:24:15Z

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

ManickaP · 2025-04-07T12:09:48Z

cc @antonfirsov

Copilot

Copilot reviewed 11 out of 15 changed files in this pull request and generated 2 comments.

Files not reviewed (4)

src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml: Language not supported
src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj: Language not supported
src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.EventSource.xml: Language not supported
src/mono/sample/wasm/browser-advanced/Wasm.Advanced.Sample.csproj: Language not supported

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs

…andler.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

javiercn

LGTM!

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs

src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs

src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml

antonfirsov

Does the PR ensure that the SocketsHttpHandlerMetrics and the ConnectionMetrics types are also trimmed away when metrics are disabled?

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs

...ystem.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs

pavelsavara · 2025-04-11T08:23:04Z

~~I added tests. Trimming doesn't work. The constant is not being propagated. I don't know why.~~

Fixed. It works when there are no indirections between the trimmed property and methods that use them

# Conflicts: # src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs

src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs

src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs

antonfirsov

LGTM modulo small suggestion. Thanks!

Edit: Just noticed MetricsHandlerTrimmedTest.cs is failing, will check this again after the fix.

...ystem.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs

…andler/ConnectionPool/HttpConnectionWaiter.cs Co-authored-by: Anton Firszov <antonfir@gmail.com>

pavelsavara added 2 commits April 7, 2025 11:58

- rebase

4b531b1

- use System.Diagnostics.Metrics.Meter.IsSupported - call EventSource.EnableActivityTracker() when DiagnosticsHandler is enabled

more

979efe8

pavelsavara added the area-System.Net.Http label Apr 7, 2025

pavelsavara added this to the 10.0.0 milestone Apr 7, 2025

pavelsavara self-assigned this Apr 7, 2025

pavelsavara mentioned this pull request Apr 7, 2025

new System.Net.Http.EnableMetrics switch #113528

Closed

pavelsavara added 4 commits April 7, 2025 12:38

more trimming

3d5031c

more trimming

7e2fcd3

EnableActivityTracker only when used

35b07d4

fix

09b851c

pavelsavara added 2 commits April 7, 2025 14:23

more

3ac7ecc

fix

fe09d5f

pavelsavara mentioned this pull request Apr 7, 2025

[browser] event pipe - JavaScript part #110818

Merged

build-analysis bot mentioned this pull request Apr 7, 2025

Intermittent build failure in AfterSourceBuild: "Could not write state file" #76488

Open

pavelsavara requested a review from antonfirsov April 7, 2025 14:40

This was referenced Apr 7, 2025

[wasm][AOT] emcc : error - received SIGKILL (-9) #89402

Open

SIGKILL (OOM?) while running LibraryImportGenerator.Tests w/o actionable log messages or artifacts #114241

Closed

SIGKILL System.Runtime.Serialization.Formatters.Tests #114287

Open

pavelsavara marked this pull request as ready for review April 7, 2025 15:11

Copilot AI review requested due to automatic review settings April 7, 2025 15:11

pavelsavara requested a review from lewing as a code owner April 7, 2025 15:11

Copilot AI reviewed Apr 7, 2025

View reviewed changes

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs Outdated Show resolved Hide resolved

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs Outdated Show resolved Hide resolved

pavelsavara and others added 2 commits April 7, 2025 17:22

Update src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsH…

63b7334

…andler.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsH…

9cffe3f

…andler.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

build-analysis bot mentioned this pull request Apr 7, 2025

[Long Running Test] 'System.Composition.UnitTests.ConcurrencyTests.SharedInstancesAreNotVisibleUntilActivationCompletes' #113686

Open

javiercn approved these changes Apr 8, 2025

View reviewed changes

MihaZupan reviewed Apr 8, 2025

View reviewed changes

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs Outdated Show resolved Hide resolved

Merge branch 'main' into browser_MetricsHandler_optional

e8b93c9

antonfirsov reviewed Apr 8, 2025

View reviewed changes

src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs Outdated Show resolved Hide resolved

src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml Outdated Show resolved Hide resolved

use FeatureSwitchDefinition

b89b0e6

pavelsavara requested review from antonfirsov and MihaZupan April 8, 2025 15:55

build-analysis bot mentioned this pull request Apr 8, 2025

/root/helix/work/correlation/scripts/<hash>/execute.sh: Permission denied dotnet/dnceng#3412

Open

3 tasks

antonfirsov reviewed Apr 8, 2025

View reviewed changes

Merge branch 'main' into browser_MetricsHandler_optional

2423d63

This was referenced Apr 10, 2025

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

pavelsavara added 4 commits April 10, 2025 17:43

feedback, tests

b7eefae

typo

c962309

wip

6110ea3

wip

29339b8

pavelsavara added 2 commits April 11, 2025 10:38

Merge branch 'main' into browser_MetricsHandler_optional

89a764a

# Conflicts: # src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs

fix

6797af3

pavelsavara requested a review from antonfirsov April 11, 2025 09:32

antonfirsov reviewed Apr 11, 2025

View reviewed changes

src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs Outdated Show resolved Hide resolved

antonfirsov reviewed Apr 11, 2025

View reviewed changes

src/libraries/System.Net.Http/tests/TrimmingTests/MetricsHandlerTrimmedTest.cs Show resolved Hide resolved

pavelsavara added 2 commits April 14, 2025 11:25

feedback

a3abf3a

dump-dependencies

1c47d61

pavelsavara mentioned this pull request Apr 14, 2025

[draft] Trim ActivitySource via System.Diagnostics.ActivitySource.IsSupported feature #114636

Draft

antonfirsov approved these changes Apr 14, 2025

View reviewed changes

...ystem.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionWaiter.cs Outdated Show resolved Hide resolved

pavelsavara and others added 2 commits April 15, 2025 08:18

Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpH…

900f62f

…andler/ConnectionPool/HttpConnectionWaiter.cs Co-authored-by: Anton Firszov <antonfir@gmail.com>

fix

cbd6446

build-analysis bot mentioned this pull request Apr 15, 2025

System.Net.Quic.Tests.MsQuicTests.WriteTests failed with "System.Net.Quic.QuicException : The connection timed out from inactivity." #105177

Open

antonfirsov approved these changes Apr 15, 2025

View reviewed changes

pavelsavara merged commit d0e6ce8 into dotnet:main Apr 15, 2025
141 of 151 checks passed

pavelsavara deleted the browser_MetricsHandler_optional branch April 15, 2025 12:07

antonfirsov mentioned this pull request Apr 24, 2025

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

disable/trim MetricsHandler when System.Diagnostics.Metrics.Meter.IsSupported is false #114326

disable/trim MetricsHandler when System.Diagnostics.Metrics.Meter.IsSupported is false #114326

pavelsavara commented Apr 7, 2025 •

edited

Loading

dotnet-policy-service bot commented Apr 7, 2025

ManickaP commented Apr 7, 2025

Copilot AI left a comment

javiercn left a comment

antonfirsov left a comment •

edited

Loading

pavelsavara commented Apr 11, 2025 •

edited

Loading

antonfirsov left a comment •

edited

Loading

disable/trim MetricsHandler when System.Diagnostics.Metrics.Meter.IsSupported is false #114326

disable/trim MetricsHandler when System.Diagnostics.Metrics.Meter.IsSupported is false #114326

Conversation

pavelsavara commented Apr 7, 2025 • edited Loading

dotnet-policy-service bot commented Apr 7, 2025

ManickaP commented Apr 7, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

javiercn left a comment

Choose a reason for hiding this comment

antonfirsov left a comment • edited Loading

Choose a reason for hiding this comment

pavelsavara commented Apr 11, 2025 • edited Loading

antonfirsov left a comment • edited Loading

Choose a reason for hiding this comment

pavelsavara commented Apr 7, 2025 •

edited

Loading

antonfirsov left a comment •

edited

Loading

pavelsavara commented Apr 11, 2025 •

edited

Loading

antonfirsov left a comment •

edited

Loading