Skip to content
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

Exception event missing from activity in HttpClient native instrumentation #108050

Open
joegoldman2 opened this issue Sep 20, 2024 · 7 comments
Open

Comments

@joegoldman2
Copy link
Contributor

When using OpenTelemetry (OpenTelemetry.Instrumentation.Http) to instrument outgoing HTTP requests, exception is captured as event in the activity. However, when using the native instrumentation, the exception event is not present.

With OpenTelemetry.Instrumentation.Http:

Span #0
    Trace ID       : 3d70cb5cc8a0a8629b9a222b5ffa7613
    Parent ID      : 3a078ea57fd36204
    ID             : 4d1d804e54cff461
    Name           : POST
    Kind           : Client
    Start time     : 2024-09-20 06:10:55.7110111 +0000 UTC
    End time       : 2024-09-20 06:10:55.7140751 +0000 UTC
    Status code    : Error
    Status message :
Attributes:
     -> http.request.method: Str(POST)
     -> server.address: Str(localhost)
     -> server.port: Int(5000)
     -> url.full: Str(http://localhost:5000/orders)
     -> error.type: Str(connection_error)
Events:
SpanEvent #0
     -> Name: exception
     -> Timestamp: 2024-09-20 06:10:55.7140442 +0000 UTC
     -> DroppedAttributesCount: 0
     -> Attributes::
          -> exception.type: Str(System.Net.Http.HttpRequestException)
          -> exception.stacktrace: Str(System.Net.Http.HttpRequestException: Connection refused (localhost:5000)
 ---> System.Net.Sockets.SocketException (111): Connection refused
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource.GetResult(Int16 token)
   at System.Net.Sockets.Socket.<ConnectAsync>g__WaitForConnectWithCancellation|285_0(AwaitableSocketAsyncEventArgs saea, ValueTask connectTask, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String host, Int32 port, HttpRequestMessage initialRequest, Boolean async, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Net.Http.HttpConnectionPool.ConnectToTcpHostAsync(String host, Int32 port, HttpRequestMessage initialRequest, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.InjectNewHttp11ConnectionAsync(QueueItem queueItem)
   at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionWaiter`1.WaitForConnectionWithTelemetryAsync(HttpRequestMessage request, HttpConnectionPool pool, Boolean async, CancellationToken requestCancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.Metrics.MetricsHandler.SendAsyncWithMetrics(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken))
          -> exception.message: Str(Connection refused (localhost:5000))

With native instrumentation (.AddSource("System.Net.Http")):

Span #0
    Trace ID       : de8d90f61122ea78afbd80fe3766ce59
    Parent ID      : ac47c5ffff0c7b73
    ID             : 4850ee5ea617fe57
    Name           : POST
    Kind           : Client
    Start time     : 2024-09-20 06:08:05.1667865 +0000 UTC
    End time       : 2024-09-20 06:08:05.2191189 +0000 UTC
    Status code    : Error
    Status message :
Attributes:
     -> http.request.method: Str(POST)
     -> server.address: Str(localhost)
     -> server.port: Int(5000)
     -> url.full: Str(http://localhost:5000/orders)
     -> error.type: Str(connection_error)

When using native instrumentation, I think the exception should also be captured as an event in the activity.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Sep 20, 2024
Copy link
Contributor

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

@KalleOlaviNiemitalo
Copy link

System.Net.Http.DiagnosticsHandler can report the exception to DiagnosticListener, but apparently not to Activity:

if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ExceptionEventName))
{
// If request was initially instrumented, Activity.Current has all necessary context for logging
// Request is passed to provide some context if instrumentation was disabled and to avoid
// extensive Activity.Tags usage to tunnel request properties
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ExceptionEventName, new ExceptionData(ex, request));
}

The implementation could perhaps use the Activity.AddException API from #102905. AFAICT, this API is not called by anything in .NET Runtime yet.

Copy link
Contributor

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

@CarnaViire
Copy link
Member

cc @antonfirsov

@antonfirsov
Copy link
Member

The event created by AddException looks kinda heavy, but we can add it if users expect us to have it around.

@KalleOlaviNiemitalo I'm wonderering if the value of error.type is sufficient in your use case? It will contain the failure status code or the HttpRequestError or the exception type name.

Triage: if needed, this is trivial to implement. I'm tentatively putting this into .NET 10.

@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label Oct 3, 2024
@antonfirsov antonfirsov added this to the 10.0.0 milestone Oct 3, 2024
@KalleOlaviNiemitalo
Copy link

@antonfirsov I don't have a use case, I only commented on how the event requested by @joegoldman2 could be implemented.

@joegoldman2
Copy link
Contributor Author

@KalleOlaviNiemitalo I'm wonderering if the value of error.type is sufficient in your use case?

The exception gives more details in some cases (maybe not in my example above).

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

No branches or pull requests

4 participants