dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.21k stars 9.95k forks source link

Add Meters for SignalR #2461

Closed aspnet-hello closed 6 days ago

aspnet-hello commented 6 years ago

We have meters for the ConnectionHandler layer used by SignalR, but we don't have any for the SignalR layer. See https://github.com/dotnet/aspnetcore/blob/2c111c7aa53c01294f284117eb1ee503940aa4b9/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs#L23 for existing meters.

Possible additions:

Original _From @anurse on Wednesday, November 1, 2017 3:28:02 PM_ Some events listed below. The goal is to add events that will help users solve problems so if these aren't going to do that job, feel free to suggest/discuss others! * `Microsoft-AspNetCore-WebSockets` EventSource * `ConnectionStarted` event - Triggered when a connection is established. * `ConnectionEnded` event - Triggered when a connection is terminated gracefully. * `ConnectionAborted` event - Triggered when a connection is terminated abnormally. * `FrameSent` event - Triggered when an frame is sent from the server. * `FrameReceived` event - Triggered when an frame is received by the server. * `PayloadBytesWritten` counter - Counts the number of payload bytes written. * `PayloadBytesRead` counter - Counts the number of payload bytes read. * `ConnectionDuration` counter - Measure the duration of each connection. * We could include some counters by frame type (Close, Ping, Pong, Text, Binary, Continuation, etc.) but many are hidden behind CoreFx. * `Microsoft-AspNetCore-Http-Connections` EventSource * ~~`ConnectionStart` event - Done~~ * ~~`ConnectionStop` event - Done~~ * ~~`ConnectionTimedOut` event - Done~~ * ~~`ConnectionsStarted` counter - Done~~ * ~~`ConnectionsStopped` counter - Done~~ * ~~`ConnectionsTimedOut` counter - Done~~ * ~~`ConnectionDuration` counter - Done~~ * `ConnectionsPerSecond` counter - Counts number of connections established per second? * `BytesReadPerSecond` counter - Counts number of bytes read per second? Perhaps hidden behind an EventSource keyword because it may have significant perf impact * `BytesWrittenPerSecond` counter - Counts number of bytes written per second? Perhaps hidden behind an EventSource keyword because it may have significant perf impact * `Microsoft-AspNetCore-SignalR` EventSource * `MessagesReceivedPerSecond` counter - Counts number of messages read per second? Perhaps hidden behind an EventSource keyword because it may have significant perf impact * `MessagesSentPerSecond` counter - Counts number of messages written per second? Perhaps hidden behind an EventSource keyword because it may have significant perf impact Some of these may not be feasible given the WebSocket APIs, so we may need to adjust or cut some events. Or perhaps just not do any of them if it's not possible :). See https://gist.github.com/anurse/af1859663ac91c6cf69c820cebe92303 for some guidance on adding EventSources and EventCounters to ASP.NET projects. _Copied from original issue: aspnet/WebSockets#209_
analogrelay commented 5 years ago

E V E N T C O U N T E R S !

analogrelay commented 5 years ago

I'd like to revive this given that we've got some patterns forming in https://github.com/aspnet/AspNetCore/pull/10884 .

davidfowl commented 5 years ago

@JamesNK you came up with a bunch of counters to expose yeah?

JamesNK commented 5 years ago

grpc client and server have the same set of counters. Messages sent, received, total calls, current calls, calls failed, calls exceeded deadline

js8080 commented 3 years ago

I stumbled on this looking for a way to instrument SignalR service calls. I'm using OpenTracing to instrument / trace ASP.NET Core web API calls and that is working fine for regular web requests but it is not picking up my SignalR service calls. The ASP.NET Core OpenTracing library utilizes the DiagnosticSource / DiagnosticListener logging mechanism to detect when traditional web requests are received, etc. so it seems that SignalR service calls do not emit those DiagnosticSource events that it relies on.

I'm having trouble finding useful documentation on whether there is any approach with SignalR to detect method calls without inserting code directly into my service methods on my Microsoft.AspNetCore.SignalR.Hub implementation. Is there something I can use to transparently capture that methods were called on my hub (like a middleware, DiagnosticSource, or EventSource)?

davidfowl commented 3 years ago

Signalr doesn't have a diagnostic source today. We can consider adding one to enable things like this

js8080 commented 3 years ago

Signalr doesn't have a diagnostic source today. We can consider adding one to enable things like this

Please do. Frameworks such as OpenTracing / Jaeger could utilize this to support SignalR tracing for ASP.NET Core applications.

@davidfowl would there be any alternative approaches you would recommend?

davidfowl commented 3 years ago

@BrennanConroy do we wan to do anything here for SignalR and WebSockets?

BrennanConroy commented 3 years ago

We should

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

david-acker commented 2 years ago

I started taking a look at this and setup a new WebSocketsEventSource with the events and counters listed for Microsoft-AspNetCore-WebSockets.

Is WebSocketsServerTransport the correct place to hook in the new event source? I tested this out while using the SignalRSamples app and the results from dotnet-counters monitor seem to line up.

BrennanConroy commented 2 years ago

I wonder if ManagedWebSocket should have the counters, that way everyone benefits not just SignalR.

adityamandaleeka commented 12 months ago

Is this superseded by https://github.com/dotnet/runtime/issues/75420 if that's the approach we want to take?

davidfowl commented 12 months ago

We can close this.

BrennanConroy commented 12 months ago

We don't want to keep it open to track adding Microsoft-AspNetCore-SignalR EventSource counters?

davidfowl commented 12 months ago

Do you mean meters? We're done with event counters

BrennanConroy commented 12 months ago

Sure, whatever new name we've come up with for counters. 😆

davidfowl commented 12 months ago

Can we update the title and description with what is still relevant? I thought we did all of the meters this release…

BrennanConroy commented 12 months ago

Updated

JamesNK commented 8 months ago

Note that the connection level metrics - https://learn.microsoft.com/en-us/dotnet/core/diagnostics/built-in-metrics-aspnetcore#microsoftaspnetcorehttpconnections - are server only. The client might want to add similar metrics.

adityamandaleeka commented 6 days ago

Closing in favor of #53429