Skip to content

Expand Blazor logging and diagnostics #11792

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

Closed
22 tasks done
SteveSandersonMS opened this issue Jul 2, 2019 · 6 comments
Closed
22 tasks done

Expand Blazor logging and diagnostics #11792

SteveSandersonMS opened this issue Jul 2, 2019 · 6 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jul 2, 2019

This is part of #10472

Ensure developers can trace all the major things that happen in a server-side Blazor application (e.g., circuits starting/ending) with a special focus on all incoming calls from the client (event notifications, JSInterop calls, renderbatch ACKs).

  • Outbound JS interop calls (.NET to JS)
    • Call sent
    • Reply received (with successful result or exception)
    • Cancelled due to timeout
    • To avoid noise, we should distinguish framework-originated calls from application-originated calls. This might be achieved by having a separate InternalInvokeAsync on RemoteJSRuntime that only the framework calls and is not on the IJSRuntime interface.
  • Inbound JS interop calls (JS to .NET)
    • Call received
    • Reply sent (with successful result or exception)
    • To avoid noise, we should distinguish framework-originated calls from application-originated calls.
  • Incoming UI event notifications
  • Rendering
    • Instantiating a component
    • Disposing a component
    • Rendering a component
    • Sending a render batch to the client
    • Receiving acknowledgement of a render batch from the client
  • Circuits
    • Circuit created
    • Circuit connected
    • Circuit disconnected
    • Circuit evicted
    • Unhandled exception
  • Routing
    • Navigation occurred, whether to a matching @page component or if no match was found

This assumes it's possible to do such fine-grained logging without any meaningful perf cost (at least, when the logging level is such that the messages are being skipped). If this does turn out to have a meaningful perf cost, we wouldn't want to do all the per-component parts under "rendering".

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Jul 2, 2019
@SteveSandersonMS SteveSandersonMS added this to the 3.0.0-preview8 milestone Jul 2, 2019
@SteveSandersonMS SteveSandersonMS self-assigned this Jul 2, 2019
@SteveSandersonMS
Copy link
Member Author

@javiercn and I have discussed this. Similar to #11791, he's going to deal with logging for JS interop. I'm going to deal with it for other areas.

@SteveSandersonMS
Copy link
Member Author

@rynowak Do you have a view on which parts of this should be handled using ILogger versus DiagnosticSource or System.Diagnostics.Trace? It's not something I have a lot of history with.

It would be convenient to use ILogger only, since it's unclear DiagnosticSource is a recommended pattern (looks like SignalR doesn't use it at all), and System.Diagnostics.DiagnosticSource can't immediately be referenced by a netstandard2.0 project due to not being in the shared framework.

Unless there are reasons, I expect to follow the same patterns for this as in the SignalR code.

@javiercn
Copy link
Member

javiercn commented Jul 4, 2019

diagnostics source is a lower level construct I believe, for way lower level tracing. ILogger afaik is good enough.

@rynowak
Copy link
Member

rynowak commented Jul 4, 2019

Let's just do ILogger for now. ILogger is always useful because it doesn't require any special tools to see the output. DiagnosticSource is only useful when someone is writing a tool to process the data.

@SteveSandersonMS
Copy link
Member Author

The non-JS-interop parts of this are now done, so I'm unassigning myself and leaving the JS interop bits with @javiercn.

@SteveSandersonMS SteveSandersonMS removed their assignment Jul 8, 2019
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Jul 8, 2019
@javiercn
Copy link
Member

Closing this as the changes have been included in #12250 which will be merged with the rest of the dependencies

@javiercn javiercn added Done This issue has been fixed and removed Working labels Jul 18, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

4 participants