NLog / NLog.Web

NLog integration for ASP.NET & ASP.NET Core 2-8
https://nlog-project.org
BSD 3-Clause "New" or "Revised" License
320 stars 166 forks source link

NLogBuilder not supported on .NET 4.6.1? #208

Closed porterchris closed 6 years ago

porterchris commented 7 years ago

Is it intent that the NLogBuilder would be supported only on .NET Standard 2.0? I'm trying to follow the setup instructions on the wiki for ASP.NET Core 2 and the NLogBuilder class is not available. Once I dug into the code I found this at the top of the NLogBuilder.cs file:

#if NETSTANDARD2_0

304NotModified commented 7 years ago

.NET 4.6.1 implements .NET standard 2?

Broad platform support. .NET Standard 2.0 is supported on the following platforms: .NET Framework 4.6.1 .NET Core 2.0 Mono 5.4 Xamarin.iOS 10.14 Xamarin.Mac 3.8 Xamarin.Android 7.5 Upcoming version of UWP (expected to ship later this year)

https://blogs.msdn.microsoft.com/dotnet/2017/08/14/announcing-net-standard-2-0/

porterchris commented 7 years ago

I understood it was a super-set and should be able to handle .NET standard 2 libraries. Perhaps it is an issue with the binary I'm pulling from nuget. This is a segment of my csproj:

 <ItemGroup>
...
    <PackageReference Include="NLog.Web.AspNetCore" Version="4.5.0-beta02" />
    <PackageReference Include="NLog" Version="4.5.0-beta07" />
  </ItemGroup>'

The binary I'm pulling does not have the NLogBuilder class included. I've confirmed I'm seeing the NLog.Web.AspNetCore.dll file dated 2017-10-13 @ 15:48 CDT, product version 4.5.0-beta02. That library in both VS2017 Object Browser and using dotPeek has no NLogBuilder class.

Looking at the AspNetExtensions.cs code in dotPeek, I only see AddNLogWeb, ConfigureNLog (public), and ConfigureNLog (private). The additional methods shown in this version of the file https://github.com/NLog/NLog.Web/blob/4a988ad49914b6d1277f980253a503ad9b202e2f/NLog.Web.AspNetCore/AspNetExtensions.cs, below the #if NETSTANDARD2_0 declaration: ConfigureNLog and UseNLog are not showing up in the library.

Is it possible that the CI build configuration for the NLog.Web.AspNetCore project is setup to something other than .NET Standard 2.0?

304NotModified commented 7 years ago

Is it possible that the CI build configuration for the NLog.Web.AspNetCore project is setup to something other than .NET Standard 2.0?

Well I think the store is using the net451 lib for net461. Dunno why. I could create a .net461 version, but it shouldn't be needed AFAIK

porterchris commented 7 years ago

I'm not seeing that on my side, but I'm not referencing that package. Should I be? image

This is where I'm seeing the issue (all the code in the #if NETSTANDARD2_0 declarations is missing): image

I'm curious if what happens with multiple target frameworks when conditional build declarations are used.

304NotModified commented 7 years ago

but I'm not referencing that package. Should I be?

Indeed.

Working on this atm, but will we probably tomorrow (almost night here)

304NotModified commented 7 years ago

LOL how do you create the project?

I get or ASP.NET (classic) on .NET 4.6.1 or ASP.NET Core on .NET Core 2. - so no ASP.NET Core on .NET 4.6.1

Is there a template for it in VS 2017?

porterchris commented 7 years ago

image

image

304NotModified commented 7 years ago

I think it's solution is here: https://github.com/dotnet/standard/issues/481

I Need to check it in depth. I will try to do that this week. If you see solution, please let me know - thx!

porterchris commented 7 years ago

I think that issue is related to runtime binding issues around the use of a .NET Standard library running in a .NET Framework configured ASP.NET Core site. I believe my issue is further up the chain when during the build. I've confirmed that the conditional compilation logic prevents NLogBuilder from being included in the .NET 4.5.1 build of the library. If the code isn't there, it isn't an issue in binding but a logic issue of why the intended classes aren't in the library where they would be expected. I removed the conditional flag but the build still broke. I haven't fully gone down that road yet but I think the issue is in the dependent NLog library reference.

I'm more curious though if the .NET Framework 4.5.1 version of the build even needs to exist. Please forgive my ignorance as I'm still very new to ASP.NET Core and .NET Standard libraries, but shouldn't the Standard 2.0 library work in place of the Framework 4.5.1 build? If nuget will pull down the Standard 2.0 library in the absence of a Framework 4.5.1 specific version, and assuming there is no binding issue happening after that as you suggested might be occurring, wouldn't the code run correctly?

I'm trying to mock this build up but I'm not a nuget power-user and not sure how to force my project to grab the .NET Standard version specifically. It might work if I use a locally hosted nuget server and remove the 4.5.1 build target from my copy of the code.

Either way, my current plan is to move up from Framework to Standard for the project. My reason for sticking with Framework was to help avoid issues with library binding as I build through this project and its proven to be more of an issue than what it was intended to prevent.

snakefoot commented 6 years ago

.NET 4.6.1 implements .NET standard 2?

Full .NET Framework will always choose .NET-Framework-assemblies. In this case the .NET 4.5.1 (NetCore v1.1)

Think two nuget-packages are needed:

NLog.Web.AspNetCore NLog.Web.AspNetCore v2

304NotModified commented 6 years ago

Think two nuget-packages are needed:

NLog.Web.AspNetCore NLog.Web.AspNetCore v2

yes, was also afraid for that.

Full .NET Framework will always choose .NET-Framework-assemblies. In this case the .NET 4.5.1 (NetCore v1.1)

Do you have a link for the complete set of rules for this? This was always unclear to me.

makravit commented 6 years ago

Hi guys, I am migrating an asp.net core 1.1 project to 2.0 that was targeting framework 4.6.1. I am not able to configure nlog properly using the 'new way' (extension method in the program.cs file). Is there any workaround to this until the new packages are available?

EricMKaufman commented 6 years ago

@makravit, this worked for me:

 private ILogger<Startup> _log;

 public Startup(IHostingEnvironment env, IConfiguration configuration)
 {
     _loggerFactory = new LoggerFactory();
 }

 public void ConfigureServices(IServiceCollection services)
 {

     services.AddSingleton<ILoggerFactory>(_loggerFactory);
     _loggerFactory.ConfigureNLog(_isDev ? "nlog.Development.config" : "nlog.config");
     _loggerFactory.AddNLog();

     _log = _loggerFactory.CreateLogger<Startup>();
     _log.LogInformation("Starting service");
 }
makravit commented 6 years ago

Thank you very much @EricMKaufman! I'll give it a try!

CamiloTerevinto commented 6 years ago

@304NotModified Just as an FYI: I have a project in ASP.NET Core 2 on 4.6.2 and I don't have the .NET Standard 2 stuff. I upgraded to .NET 4.7 to test and the same happens.

If I navigate to the metadata, I get:

// C:...nuget\packages\nlog.web.aspnetcore\4.5.0-beta04\lib\net451\NLog.Web.AspNetCore.dll

304NotModified commented 6 years ago

yes thanks,

as @snakefoot mentioned, ASP.NET 4.6 is using the .NET 4.5 dll and so we need a new package to prevent this, or make a breaking change. Both i'm not really happy with.

snakefoot commented 6 years ago

Maybe an alternative solution would be that NET46 includes two DLLs (AspNetCore1 and AspNetCore2)

So the solution builds two assemblies, but they are injected into a single nuget-package. Then Net46 and NetStandard2.0 can include both assemblies (will then also be available for Net47).

snakefoot commented 6 years ago

@304NotModified You should also add net461 to NLog.Web.AspNetCore-targetplatforms (To match net461 in NLog.Extension.Logging)

snakefoot commented 6 years ago

@304NotModified And add net461 to NLogBuilder.cs, and AspNetExtensions.cs (next to NETSTANDARD2_0), so it can also access NLogBuilder.

304NotModified commented 6 years ago

working on it!

304NotModified commented 6 years ago

Fixed in Nlog.Web.AspNetCore 4.5 rc1