-
Notifications
You must be signed in to change notification settings - Fork 783
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.
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
[HttpClient] Add error.type
for traces and metrics
#5005
[HttpClient] Add error.type
for traces and metrics
#5005
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5005 +/- ##
==========================================
- Coverage 83.69% 83.39% -0.31%
==========================================
Files 296 296
Lines 12403 12452 +49
==========================================
+ Hits 10381 10384 +3
- Misses 2022 2068 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -271,6 +271,11 @@ public void OnStopActivity(Activity activity, object payload) | |||
|
|||
if (TryFetchResponse(payload, out HttpResponseMessage response)) | |||
{ | |||
if (currentStatusCode == ActivityStatusCode.Unset) | |||
{ | |||
activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change related to "error.type" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this was there already
Lines 284 to 287 in 5345863
if (currentStatusCode == ActivityStatusCode.Unset) | |
{ | |
activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode)); | |
} |
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
…lerDiagnosticListener.cs Co-authored-by: Utkarsh Umesan Pillai <[email protected]>
Co-authored-by: Utkarsh Umesan Pillai <[email protected]>
…lerMetricsDiagnosticListener.cs Co-authored-by: Timothy Mothra <[email protected]>
Co-authored-by: Utkarsh Umesan Pillai <[email protected]>
Co-authored-by: Utkarsh Umesan Pillai <[email protected]>
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
…lerMetricsDiagnosticListener.cs Co-authored-by: Utkarsh Umesan Pillai <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Tests should be simplified once #4928 is addressed.
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #
Design discussion issue #
Changes
Adds error.type attribute to activity and
http.client.request.duration
for HttpClient fornon-netframework
targets.error.type
attribute to activity fornetstandard
,net6.0
,net7.0
andnet8.0
targets. Fornet8.0
target the value will vary due to the addition of HttpRequestError Enum, which allows further drilldown ofHttpRequestException
\HttpIOExceptiontype
. This is in sync with how the tag is emitted onhttpclient.request.duration
metric on .NET8.0.error.type
attribute tohttp.client.request.duration
metric fornetstandard
,net6.0
andnet7.0
targets. Onnet8.0
, metric will be emitted via added meterSystem.Net.Http
within instrumentation library.TODO: Cover
HttpWebRequest
.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes