New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

.NET Framework HttpClient metrics #1

Closed

matt-hensley wants to merge 41 commits into main from net462httpclientmetricsv3

Owner

matt-hensley commented Sep 18, 2023 •

edited

Loading

Adds HttpClient metrics for .NET Framework 4.6.2 and newer. This implementation does not rely on tracing at all, and can be used without traces being sampled/recorded.

TODO

~~Metrics only tests~~
~~No metrics / no traces tests~~
~~Metrics + traces matching tests~~
~~Metrics generation when there is an exception~~
~~Duration from stopwatch when the activity is null~~

matt-hensley added 12 commits

September 14, 2023 20:35


          snapshot

17d67e0


          null actvitiy is tracing is disabled

800a71d


          setup metics without tracing

4a9239e


          only emit when there is a listener

d7b2472


          modify tests for netfx

46de1a9


          always capture response status code

4e445b5


          Merge branch 'main' into net462httpclientmetricsv3

f0ca432


          undo conditional extraction

bed7b2b


          activity duration or stopwatch

ba04ac0


          activity duration or stopwatch

48c354a


          Merge branch 'net462httpclientmetricsv3' of https://github.com/matt-h…

872ce63

…ensley/opentelemetry-dotnet into net462httpclientmetricsv3


          activity may be null here

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated Show resolved Hide resolved

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated Show resolved Hide resolved

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Show resolved Hide resolved

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated

+                          if (httpStatusCode.HasValue)
+                          {
+                              HttpClientDuration.Record(

pyohannes Sep 18, 2023

Worth asking upstream (on the channel or in the SIG meeting), but I think that metric generation for .NET and .NET framework should be consistent. Which means, we should support flags for emitting old or new attributes: https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs#L67C20-L67C20

Owner Author

matt-hensley Sep 18, 2023

Can ask in the SIG. For the OWIN metrics PR, they requested I use new attribute names. They did not ask to support both, so tbd.

pyohannes Sep 19, 2023

Yes, OWIN didn't send any metrics at all, so going just with the new ones there seems fine to me.

However, for HttpClient, there are already metrics emitted for .NET. I think it would be awkward if HttpClient would emit different metrics for .NET and .NET framework.

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Show resolved Hide resolved

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Show resolved Hide resolved

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated Show resolved Hide resolved

Kielek and others added 11 commits

September 18, 2023 11:05


          [OpenTracing.Shim] Remove obsolete constructor (open-telemetry#4862)

2c5c034


          Move @pellared to emeritus (open-telemetry#4856)

fc1100f


          Align console logging example with ASP.NET Core logging example (open…

7eb3e73

…-telemetry#4855)


          Use dotnet-coverage instead of CodeCoverage.exe (open-telemetry#4857)

224a4cc


          duration -> durationMs for now

42e9022


          StartTimestamp handling

527106f


          adopt taglist

e31f687


          using directive is unnecessary

01840c0


          using directive is unnecessary

9ef2915


          Merge branch 'net462httpclientmetricsv3' of https://github.com/matt-h…

47fc955

…ensley/opentelemetry-dotnet into net462httpclientmetricsv3

# Conflicts:
#	src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs


          [integration test] otlp log exporter (open-telemetry#4854)

7cb92d3

matt-hensley added 4 commits

September 18, 2023 15:45


          remove URL attribute

d9ca497


          status enum to int

308ca8f


          remove ternary

480940e


          early return if tracing and metrics are not enabled

27ad5ab

pyohannes reviewed

View reviewed changes

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated Show resolved Hide resolved

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated Show resolved Hide resolved

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Show resolved Hide resolved

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs Outdated Show resolved Hide resolved

matt-hensley and others added 14 commits

September 19, 2023 09:52


          set activity

025c02d


          Update src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebR…

dfaa8f7

…equestActivitySource.netfx.cs

Co-authored-by: Johannes Tax <johannes@johannes.tax>


          Merge branch 'net462httpclientmetricsv3' of https://github.com/matt-h…

ad10683

…ensley/opentelemetry-dotnet into net462httpclientmetricsv3


          Update src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebR…

8bd64ef

…equestActivitySource.netfx.cs

Co-authored-by: Johannes Tax <johannes@johannes.tax>


          Update src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebR…

6ca5c9d

…equestActivitySource.netfx.cs

Co-authored-by: Johannes Tax <johannes@johannes.tax>


          Merge branch 'net462httpclientmetricsv3' of https://github.com/matt-h…

61a01f7

…ensley/opentelemetry-dotnet into net462httpclientmetricsv3


          flag name change

b4dd250


          remove Moq from test

bca1361


          need to pick context off current activity for propagator to work

5200afb


          extra line

4d15ca6


          modify tests to handle metrics and/or tracing on/off

dd07ca0


          null handling

fc47558


          null check for safety, tests don't blow up here for some reason?

9ae0193


          Merge branch 'main' into net462httpclientmetricsv3

9c4c3b5

matt-hensley closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet