Skip to content

Conversation

@vwxyzh
Copy link
Contributor

@vwxyzh vwxyzh commented Apr 7, 2025

This pull request introduces activity tracing from app server to SignalR clients. The changes include creating and managing Activity instances to trace method calls and their outcomes, enhancing the observability of the service.

Tracing Enhancements:

Method Signature Changes:

  • CreateMessage method in ServiceLifetimeManager.cs: Added an Activity parameter to include tracing information in the message creation process. [1] [2]

Dependency Additions:

  • Added System.Diagnostics and Microsoft.Azure.SignalR.Internals namespaces to support activity tracing. [1] [2]

These changes aim to enhance the diagnostic capabilities of the Azure SignalR service by incorporating distributed tracing, making it easier to monitor and troubleshoot the service's behavior.

@BrennanConroy
Copy link
Collaborator

Can you show what this looks like from something like the aspire dashboard? e.g. https://learn.microsoft.com/en-us/aspnet/core/signalr/diagnostics?view=aspnetcore-9.0#net-signalr-client-activitysource

This pull request introduces activity tracing from app server to SignalR clients.

This sounds like it's trying to do the last bullet in dotnet/aspnetcore#51557 (comment)
Is that correct? If so, wouldn't it be better to wait for the core SignalR library to add this support? Or will this be needed regardless?

@vwxyzh
Copy link
Contributor Author

vwxyzh commented Apr 8, 2025

@BrennanConroy here is the task list for SignalR service. #2182
This pr is the implementation for server to client in server SDK.
And client to server in server SDK should be supported by net 9 SignalR by default, let me validate it later.
Since we rewrite HubLifetimeManager<THub>, we need implement tracing part by ourselves.
Of cause, we can wait for SignalR server to client part ready.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants