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 option to disable logging in sample Blazor app #27721

Closed
CoffeeFlux opened this issue Nov 11, 2020 · 8 comments
Closed

Add option to disable logging in sample Blazor app #27721

CoffeeFlux opened this issue Nov 11, 2020 · 8 comments
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-component-model Any feature that affects the component model for Blazor (Parameters, Rendering, Lifecycle, etc) feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly severity-nice-to-have This label is used by an internal tool

Comments

@CoffeeFlux
Copy link

When publishing the Blazor sample, System.Extensions.Logging and System.Extensions.Logging.Abstractions are present in the output and appear to be held alive by DependencyInjection. It would be nice to ensure they can be linked out by someone attempting to maximize size reduction, even if that requires an extra flag to be set in the build configuration.

@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Nov 11, 2020
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Nov 11, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Nov 11, 2020
@ghost
Copy link

ghost commented Nov 11, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@SteveSandersonMS
Copy link
Member

Logging is a feature of the default app template. Is this a proposal to remove that feature, or something more subtle?

@CoffeeFlux
Copy link
Author

CoffeeFlux commented Apr 2, 2021

Is that feature in the template or Blazor itself? Is there a reason we need to log in the final published app? Apologies if this is silly; it's been a while since I filed this and don't remember the details.

@eerhardt
Copy link
Member

eerhardt commented Apr 6, 2021

I believe the intention here is to introduce a feature switch which allows for logging to be conditionally removed. All that should be necessary is to add an if around this code:

Services.AddLogging(builder =>
{
builder.AddProvider(new WebAssemblyConsoleLoggerProvider(DefaultWebAssemblyJSRuntime.Instance));
});

That would allow for the Microsoft.Extensions.Logging*.dll assemblies to be removed from the app (~18 KB .br compressed).

By default, we can keep the same behavior as we have now. But if someone wants to get their app as small as possible, this would enable them to remove these logging assemblies by flipping the feature switch.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Apr 9, 2021

@eerhardt The availability of the logger DI services is an assumption we make throughout various core components, such as <Router>. 3rd-party components will also freely rely on its presence since it's always been part of the mandatory minimal set and within ASP.NET Core circles it's generally assumed that any application is going to support logging.

Do you think it would be reasonable for us to consider adding a "no logging" option only if we get some clear feedback from customers that they want it? If almost nobody would realistically turn on such an option, it seems like the benefit is artificial at best, and would also impose a some disproportionate costs like us trying to account for the cases where logging isn't available.

Is there a reason we need to log in the final published app?

Yes, logging is a core feature of the framework. Developers issue log messages through ILogger from their own code, plus they have the ability to configure in production how much log detail they get out of framework code. People use this to diagnose issues in production.

The line we tread between "minimalist library" and "full-featured framework" is a subjective one. ASP.NET Core - and Blazor by extension - tries not to be at either extreme on the spectrum, and tries to provide enough configurability to suit enough people, but without so much that it's impossible to build an ecosystem because nothing can be taken for granted. As it stands, logging happens to be one of the things we've learned to treat as core, but anything can change if needed!

@eerhardt
Copy link
Member

Do you think it would be reasonable for us to consider adding a "no logging" option only if we get some clear feedback from customers that they want it?

The idea is that if a customer wants their application as small as possible, they would turn this switch on. We would document it just like the others in https://docs.microsoft.com/en-us/aspnet/core/blazor/webassembly-performance-best-practices?view=aspnetcore-5.0#disable-unused-features.

and would also impose a some disproportionate costs like us trying to account for the cases where logging isn't available.

I think the ILogger would always be there for any code to use. Code that needs to log shouldn't need to be aware if logging was disabled or not. We would just strip builder.AddProvider(new WebAssemblyConsoleLoggerProvider(DefaultWebAssemblyJSRuntime.Instance)); and probably just add a NullLogger instead.

@javiercn javiercn added feature-blazor-component-model Any feature that affects the component model for Blazor (Parameters, Rendering, Lifecycle, etc) feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Apr 20, 2021
@ghost
Copy link

ghost commented Jul 20, 2021

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.

@mkArtakMSFT
Copy link
Member

Hi. Thanks for contacting us.
We're closing this issue as there was not much community interest in this ask for quite a while now.
You can learn more about our triage process and how we handle issues by reading our Triage Process writeup.

@mkArtakMSFT mkArtakMSFT closed this as not planned Won't fix, can't repro, duplicate, stale Nov 17, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-component-model Any feature that affects the component model for Blazor (Parameters, Rendering, Lifecycle, etc) feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

5 participants