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

Add detailed signalR logs #1678

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Conversation

andchiind
Copy link
Contributor

Closes #1563

@andchiind andchiind added backend Backend related functionality improvement Improvement to existing functionality labels Jul 19, 2024
@andchiind andchiind self-assigned this Jul 19, 2024
Copy link
Contributor

@mrica-equinor mrica-equinor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

Comment on lines +27 to +28
"Microsoft.AspNetCore.SignalR": "Debug",
"Microsoft.AspNetCore.Http.Connections": "Debug"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want Debug to be the general level or is this something we set when we want to debug something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, the documentation is somewhat vague on what will be logged: https://learn.microsoft.com/en-us/aspnet/core/signalr/diagnostics?view=aspnetcore-8.0#server-side-logging
Here they only mention two options, debug and trace, and they state trace is more detailed than debug. As logn as we can see when messages are being sent then I think the log could be useful, but if this log is not detailed enough then another option would be to just use the normal logger in the SignalRService class, though this is theoretically not as reliable as we cannot guarantee that the messages are being sent properly when we call the function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets try with Debug then, and change it if it floods the logs with to much information

@andchiind andchiind merged commit 8338d41 into equinor:main Jul 24, 2024
10 checks passed
@andchiind andchiind deleted the signalr-logging branch July 24, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend related functionality improvement Improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set up server side SignalR log
3 participants