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

Update AspNetCore and Hosting Dependencies/Features to Match MEH #681

Open
TheXenocide opened this issue Jan 21, 2020 · 10 comments
Open

Update AspNetCore and Hosting Dependencies/Features to Match MEH #681

TheXenocide opened this issue Jan 21, 2020 · 10 comments
Milestone

Comments

@TheXenocide
Copy link

Bug(?)/Improvement(?)

NLog version: 4.6.8
NLog.Extensions.Logging version: 1.6.1
NLog.Web.AspNetCore version: 4.9.0

Platform: .NET Framework 4.8, .NET Core 3.1, .NET Standard 2.0

Currently there are features that could be universally supported for all applications referencing the Microsoft.Extensions.Hosting generic application host but that are only currently supported in ASP.NET Core (e.g. ${aspnet-environment}). Since ASP.NET Hosting now uses Microsoft.Extensions.Hosting and desktop applications that are likely to use Microsoft.Extensions.Logging will often do so through Microsoft.Extensions.Hosting going forward it seems like the feature support (and dependency hierarchy) should be organized in the same order as the Microsoft types (i.e. since Microsoft's Dependencies go ASP.NET -> Microsoft.Extensions.Hosting -> Microsoft.Extensions.Logging it would be ideal for NLog dependencies to mirror this, e.g. NLog.Web.AspNetCore -> NLog.Extensions.Hosting -> NLog.Extensions.Logging)

One reason this comes up for us is because we use Microsoft.Extensions.Hosting for a single product that contains both .NET 4.8 and .NET Core 3.1 applications. For re-use and establishing base patterns we have various hosting libraries with extension methods to set the hosts up the way we like and since our general NLog extensions (we have custom formatters, conditions, etc.) and common NLog.config file are universal they are defined in a library that also exposes the extension methods designed to be used with Microsoft.Extensions.Hosting. Now that we're adding an ASP.NET Core hosting library, it references NLog.Web.AspNetCore as well as our NLog package (and therefore references NLog.Extensions.Hosting).

We would really like to use the environment name in our universal config file but they are currently only features of the ASP.NET Core. Additionally we would like our dependency hierarchies for the packages we're generating and consuming to match up to reduce risk of divergence in the future (this hasn't caused an critical issues yet so far as I can tell, but it seems like it would make sense).

@TheXenocide TheXenocide changed the title Update AspNetCore and Hosting Dependcies/Features to Match MEH Update AspNetCore and Hosting Dependencies/Features to Match MEH Jan 21, 2020
@304NotModified
Copy link
Member

NLog.Web.AspNetCore -> NLog.Extensions.Hosting -> NLog.Extensions.Logging)

I think this is a good idea!

But I don't fully understand why you can't use NLog.Web.AspNetCore+NLog.Extensions.Hosting now?

Is that because NLog.Extensions.Hosting is targeting less platforms (no netstandard 1.x, no net framework)?

@TheXenocide
Copy link
Author

We are using both, it's mostly just that there are features that were originally implemented in the ASP.NET package which can now be supported in the generic host package. We'd like to use those in the config file we share with both packages. The rest is more of a conceptual arrangement of dependencies since our AspNetCore hosting package references our MEH package which references our logging package (the same as the MS dependency chain) but the NLog refs we have don't follow the same hierarchy.

@TheXenocide
Copy link
Author

It would also be ideal to expose IHostEnvironment.ApplicationName alongside IHostEnvironment.Environment since we intend to use both; I didn't realize the application name wasn't already supported in the ASP.NET Core side until I tried documenting this request alongside our changes.

@snakefoot
Copy link
Contributor

@TheXenocide You are very welcome to create a pull-request with your ideas.

@304NotModified
Copy link
Member

it would be ideal for NLog dependencies to mirror this, e.g. NLog.Web.AspNetCore -> NLog.Extensions.Hosting -> NLog.Extensions.Logging)

We have already, NLog.Extensions.Hosting -> NLog.Extensions.Logging`. So we only need
NLog.Web.AspNetCore -> NLog.Extensions.Hosting.

So moving this to the NLog.Web repo

@304NotModified 304NotModified transferred this issue from NLog/NLog.Extensions.Logging Aug 27, 2021
@304NotModified 304NotModified added this to the 5.0 milestone Aug 27, 2021
@304NotModified
Copy link
Member

304NotModified commented Aug 27, 2021

Indirectly the dependencies won't change as:

  • NLog.Extensions.Hosting depends on Microsoft.Extensions.Hosting.Abstractions
  • NLog.Web.AspNetCore depends on Microsoft.Extensions.Hosting.Abstractions and
  • Microsoft.Extensions.Hosting.Abstractions also depends on Microsoft.Extensions.Hosting.Abstractions

@304NotModified
Copy link
Member

304NotModified commented Aug 27, 2021

ow the platform support is different

--> NLog/NLog.Extensions.Logging#526

@304NotModified
Copy link
Member

With the drop of ASP.NET Core 1 and NLog/NLog.Extensions.Logging#528, we could do this :)

@304NotModified
Copy link
Member

Unfortunate this isn't an option regarding .NET 4.6.1. See NLog/NLog.Extensions.Logging#527

The issue is then we could not support ASP.NET Core 2 with .NET 4.6.1. And for now that is still supported.

@304NotModified 304NotModified modified the milestones: 5.0, 6.0 Aug 28, 2021
@304NotModified
Copy link
Member

targeting 6.0

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

No branches or pull requests

3 participants