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

PROPOSAL: Introduce .NET Standard support in Common.Logging 4.0 and sunset Common.Logging 3.x after 3.4 #148

Open
sbohlen opened this issue Jul 15, 2017 · 28 comments

Comments

@sbohlen
Copy link
Member

sbohlen commented Jul 15, 2017

The project team would like to solicit comments/input from the community on the following proposal.

With all the churn in the .NET stack over the past few years, (e.g., PCLs, .NET Framework Profiles, WinRT, .NET Core, .NET Standard), its been challenging to keep projects like Common.Logging entirely aligned with what's needed for its adopters within all of the differing flavors of .NET.

Since the original concept of .NET Standard was first suggested (and along with it a completely different model/architecture for mutli-targeting class libraries), we've struggled for some time to arrive at a go-forward strategy for Common.Logging that would be aligned with the future direction/architecture of the .NET platform vs. being (mostly) backward-facing and constrained by legacy design decisions being carried forward from years prior. Part of the delay in our plans to support .NET Standard had been the gap between the arrival of .NET Standard and the availability of proper tooling/Visual Studio support for the concepts it represents.

Now that we've reached a point where the tooling is able to properly support the concepts, we think we've arrived at a go-forward strategy for this aspect of the project. After review and consideration we're now leaning strongly towards the following approach to support .NET Standard in Common.Logging:

  1. Common.Logging 3.4 will be the last release of the 3.x version branch (e.g., there will be no 3.5 release planned and we will only do 3.4.x bug-fix releases as needed for some limited time).
  2. Common.Logging 3.4 will be the last release of Common.Logging to support the 'legacy' .NET Framework versions which fall below the floor established by .NET Standard 1.0 (e.g., < .NET 4.5).
  3. Common.Logging 4.0 will support only .NET Standard, leveraging the new multi-targeting capabilities introduced in Visual Studio 2017 and abandoning PCL support entirely. NOTE: given the differing availability of .NET APIs in each version of .NET Standard, we have not yet determined for certain the min. version of .NET Standard that will be supported by Common.Logging 4.0. But it will at least target >= .NET Standard 1.0 and will not support any .NET Framework version that falls outside the scope of .NET Standard as defined here.
  4. Common.Logging 4.0 will support only Visual Studio 2017 as an IDE for building and contributing to the project. Support for building the project in legacy versions of Visual Studio will be dropped as they cannot support .NET Standard and the Community Version of Visual Studio 2017 is 100% free of charge and should be able to be used by adopters, contributors, etc. who desire to interact with the source code for the project.

Given that most versions of the .NET Framework < .NET Standard 1.0 have already reached (or are about to) their effective end-of-life re: support from Microsoft, this above approach seems a reasonable decision to make at this time. That said, since this decision will effectively 'orphan' any users of Common.Logging that are still targeting < .NET 4.5 from having access to any new features and/or new loggers for which support were to be introduced, we wanted to offer this opportunity for users to offer comment/feedback before this approach is officially pursued.

Thanks in advance for any and all input!

-The Common.Logging project team

@LHCGreg
Copy link

LHCGreg commented Jul 20, 2017

That all sounds reasonable to me, but would it be feasible to multitarget both netstandard and .NET Framework for those poor souls still on a version lower than 4.5? Would it require any extra work? I don't have any personal stake in it, just wondering.

@sbohlen
Copy link
Member Author

sbohlen commented Jul 26, 2017

I think I'd say that we can explore that and try to determine the LOE required to support that. If it can be achieved with a reasonable effort, I'd say we should try to accommodate as best we can.

I think in general the objectives go like this:

  1. Consolidate support for multiple versions of Visual Studio into only Visual Studio 2017.
  2. Remove support for versions of .NET not supported by reasonably recent versions of logging frameworks supported by Common.Logging (if e.g., Log4Net hasn't supported .NET 2.0 for 5+ years, why are we still supporting .NET 2.0 with Common.Logging?)

If we can achieve the above with adding support for e.g., .NET 3.5 then I think we're interested in going there; and if not, then not :)

@earloc
Copy link

earloc commented Jul 27, 2017

Hi! Even though I am not using this lib atm, going in this direction definitely seems the way to go. I am for my self in the process of updating/restructuring some of my packages.
However, supporting legacy (full)-Frameworks with netstandard-tooling seems to be less painful as it ought to be with PCLs.
Just wanted to provide one or two maybe helpful links ;)

And for the matter of the lost souls still sticking with eg. .net2.0:
If the community keeps alive those frameworks by still supporting them in libraries like this one, even though MS as already EOLed those frameworks, many people do not see any reason to make the framework shift in the first place (or maybe having a hard time to convince other "stakeholders" or teammembers to perform an update). That said, dropping support for this kind of legacy may help others argumenting to do an update, as more and more packages will drop legacy-support, rendering these older frameworks more and more useless in regards of available (up-to-date) 3rd party solutions.

@sbohlen
Copy link
Member Author

sbohlen commented Jul 27, 2017

@earloc Thanks for the input (and the links). I'd not seen the second of those, but was aware of the process outlined there (agree w/ the author that its scattered all over the place and its helpful to have it all in one place).

As mentioned, we're not surrendering on supporting older/legacy versions of .NET Framework where the effort is considered somewhat minimal. But we are shifting our approach with an eventual 4.0 release to place less of an emphasis on continuing to support older versions of the framework to ensure continued compatibility and more of an emphasis on simplifying the complexity that supporting so many parallel versions of the framework has created for this project.

Your point that multi-targeting in VS 2017/net-standard is already much simplified from the PCL approach is well-taken, however. It may well be that after a grand refactoring of the solution to rely upon this new approach, we discover that its "just as easy" to continue to support older framework versions.

That said, there are a number of pending ISSUES/suggestions for changes to Common.Logging 3.x that have been postponed to 4.0 because they would represent breaking changes (and SemVer precludes us from implementing them until we rev the major version number). We're thinking that since 4.0 will already be breaking changes, this is also the time to make any necessary 'clean breaks' with the past legacy supported frameworks as well.

Thanks again for your feedback/input; more perspectives are definitely better than fewer!

@marcselis
Copy link

Will the new .NET Standard version be compatible with previous versions?
I mean, will code that uses common.logging for logging still compile after upgrading the nuget package to the new 4.0 version?
Most importantly this code:
private static readonly ILog _log = LogManager.GetLogger<MyClass>();

@jovanpop-msft
Copy link

jovanpop-msft commented Mar 31, 2018

@sbohlen - does it means that Microsoft.Extensions.Logging in .net core will be eventually deprecated and replaced with new Common.Logging 4 from .net standard, or they are going to co-exists? I have some libraries working with Common.Logging and want to use them in .net core. Should I rewrite everything to match Microsoft.Extensions.Logging or wait for Common.Logging 4?

Since we need one abstraction, now is unclear what is the roadmap for Microsoft.Extensions.Logging.

@mnivet
Copy link

mnivet commented Jul 24, 2018

I'm a bit in the same case than @jovanpop-msft
We have multiple libraries that are based on Common.Logging, and have start to build new applications based on AspNetCore.
For that, we have already re-built multiple librairies against netstandard2.0, including a dependency to Common.Logging 3.4 which is compatible with the netstandard 1.3

But now what should we do in our AspNetCore applications ? Use Common.Logging or Microsoft.Extensions.Logging which is designed with DI in mind from the start ?

We use NLog as the concrete implementation, and NLog team has provided an adapter to use it above Microsoft.Extensions.Logging. But Common.Logging team seems moving slower and is not yet ready.

The solution I've start to develop is to create a Common.Logging.ILoggerFactoryAdapter that is based on Microsoft.Extensions.Logging, so I can inject NLog in Microsoft.Extensions.Logging with the package provided by the NLog team, and then inject Microsoft.Extensions.Logging in Common.Logging so our legacy code based on Common.Logging can continue to works.

With that approach we probably won't need a new version of Common.Logging, and we will probably consider Common.Logging as a legacy framework, because if NLog team maintain themselves the adaptation to the Microsoft.Extensions.Logging abstraction I will be much confident than a maintenance done by a third party team like it was with Common.Logging due to the inherent delay between a new release of the product and the adapter when it's done by different teams.

What the community though about that approach ?

nb: I didn't find any adapter that already do that. But If it's successful we can open source it and release it

@mnivet
Copy link

mnivet commented Jul 31, 2018

Creating an adapter between Common.Logging and Microsoft.Extensions.Logging was not a so good idea.
In some cases it doesn't work well. In particular when using nlog layouts like ${callsite} because it will render the name of the method in the adpater, and it's not fixable because we go trough 2 levels of adapters before accessing nlog.

So we choose to roll-back from that idea and compile internally an nlog adapteur for common.logging in netstandard. Which means that we are still interested by a new version of common.logging fully compatible with NetStandard which will sometimes live side by side with Microsoft.Extensions.Logging.Abstraction

@tonycoelho
Copy link

@sbohlen Can we get an update on this? Is there any movement on v4.0 with .NET Standard and .NET Core support or has this project been abandoned? I don't see any significant progress over the past year. We are trying to determine if we should continue with Common.Logging for .NET Core projects and create our own adapters or migrate to Microsoft.Extensions.Logging since NLog and other logging frameworks provide their own adapters for that API. Thanks!

@Defee
Copy link

Defee commented Oct 6, 2018

@sbohlen Could I help somehow with the move to version 4?

@Defee
Copy link

Defee commented Oct 25, 2018

@tonycoelho, @mnivet, is it a good idea to help with the project? Might be do some fork and work on this support (at least for Common.Logging -> NLog)? I personally love common logging and it is really good working with slightly different approach than Microsoft Logging extensions.

I personally will be able to look at the sources sometime next week.

@sbohlen
Copy link
Member Author

sbohlen commented Oct 25, 2018

All pull requests are always welcome!

For what its worth, current plans call for me to be able to return to focus more on this effort beginning at the start of November (after a number of current other distractions are resolved elsewhere in my life). My goal would be to have this (and other unaddressed issues) resolved by the end of calendar 2018 latest.

That said, if anyone wants to fork the repo and submit a PR for any open work (port to .net standard, support for later nlog, etc.), all pull requests are always welcomed!

@Defee
Copy link

Defee commented Oct 26, 2018

@sbohlen I will look what I can do in terms of the porting to .net standard over this weekend.
Do you have some particular requirements for the porting or 4.x will be supported from the latest .netstandard 2.0 on current date?

@tonycoelho, @mnivet do you also have some preferences in version terms in the terms of porting?

@mnivet
Copy link

mnivet commented Oct 26, 2018

I've just open to public the common.logging.nlognetstandard adapter that have been write internally in our company. You can found it here:
https://github.com/TalentSoft/common.logging.nlognetstandard

@sbohlen how do you prefer to integrate that ?

  • Continue to have a lot of projects in the same big repository ? In that case I let you copy paste the code in your repo.
  • Or keeping it in a dedicated repository ? In that case I can transfert you the ownership.

@Defee
Copy link

Defee commented Oct 28, 2018

@sbohlen

  1. Can we purge the support for Silverlight for the 4.x + version? As it seems that Microsoft are not willing to support it long term...
  2. Do we allowed for version 4.x to start support from 4.5 not from 4.0?
  3. Can I offer some reorganization for 4.x? I've listed the points below.

As new style of csproj, can utilize different build frameworks and generate the output correctly. I can suggest the following refactoring:

  1. Made few core packages:
    1. Common.Logging.Core (>=net45, >=netstandard(version X) ) Publishable to nuget
    2. Common.Logging (>=net45, >=netstandard(version X) ). Publishable to nuget
    3. Common.Logging.{Logger}(>=net45, >=netstandard(version X) ). Logger based implementations Publishable to nuget
    4. Common.Logging.{Logger}(>=net45, >=netstandard(version X) ). Logger based implementations Publishable to nuget
    5. Common.Logging.TestUtils (>=net45, >=netstandard(version X) ). Not Publishable to nuget
    6. Common.Logging.{LoggerTests} (>=net45, >=netstandard(version X) ). Not Publishable to nuget
  2. As nuget has improved on handling dependencies I would suggest to keep only one project for the loggers integrations, just keep minimal compatible version as dependency and update in case of need. (Currently I've tested this approach on one of my projects works like a charm). Drawback is that not all the projects seem to be on Nuget from what we have in Lib folder.

It may simplify the things a little bit and easy out the support.

@Defee
Copy link

Defee commented Nov 10, 2018

Hi all. I've done something in my fork.
@sbohlen could we have discussion about what is done (might be a skype call for the 15 minutes?) as there are some breaking changes which could include the breaking of old build pipeline.
Here are the link to the current breaking changes: https://github.com/Defee/common-logging/blob/version_4x/breaking-changes.md .

@mnivet did your version include the integration with Microsoft.Logger as well?

@mnivet
Copy link

mnivet commented Nov 12, 2018

@Defee no, That idea was abandoned as Iv'e explain a bit earlier in this thread
It doesn't work so well, and currently we just send CommonLogging traces to NLog and Microsoft.Extensions.Logging traces to NLog
NLog is a concrete log implementation that can correctly aggregate traces from that two abstractions (CommonLogging and Microsoft.Extensions.Logging) and provide an easy way to configure where all traces should be output.
Microsoft also provide native implementations above their abstraction but I'm not sure if we can use them directly in a CommonLogging adapter without re-writing a log configuration system because the configuration part in Microsoft logging system seem more tied to the log abstraction than to the log implementations

@Defee
Copy link

Defee commented Nov 15, 2018

Microsoft also provide native implementations above their abstraction but I'm not sure if we can use them directly in a CommonLogging adapter without re-writing a log configuration system because the configuration part in Microsoft logging system seem more tied to the log abstraction than to the log implementations

@mnivet I found something. Basically it is how NLog did their extension for the microsoft logging. I think for Common it should be the same way of doing it. It is not much work, but I need to find time to do that. Possibly early next month.

@mnivet
Copy link

mnivet commented Nov 15, 2018

@Defee interesting idea. I've trying to adapt microsoft logging to common logging with mixed results.
But I've not try adapting common logging to microsoft logging.
Is that a better idea ? I'm not sure that it will provide better results on nlog layout renderers like ${callsite} that are based on the callstack...

My tries make me think that common logging and Microsoft logging are both interfaces that should be implemented (through adapters) by a final framework (like nlog), but that there is no real advantage to adapt one above the other, even if you reverse the order that I have tried.

Then, each project should choose to use common logging or microsoft logging, and DI must bind the chosen implementation (nlog or any other logging framework) to the the interface(s) that are used in the dependencies of the final app.

To achieve that properly we need to have more modern Common Logging packages that support NetStandard, and that are more DI oriented (with less static entry points and generic logger interface) so all types of projects can have a real choice between common logging and microsoft logging, because for instance netstandard project are limited to Microsoft logging, and net framework projects are limited to common logging, which is not a real choice

@snakefoot
Copy link

Created PR #176 for resolving this for NLog. (Includes custom build of nuget-package as attachment)

@Defee
Copy link

Defee commented Jun 17, 2019

Hi all!
I plan to have the forked version published on nuget with some kind of prefix somewhere over the next weekend. I will tear it down (de-list it) when the official decision (or announcement for upgrade) will be made from @sbohlen.

@snakefoot, could we have might be a Skype call/or other call just to see our diffs for NLog implementation ( I think it shouldn't be much)?

@snakefoot
Copy link

@Defee I have pretty much just taken the Serilog Implementation and renamed it to NLog. You can see my actual changes in PR #176 and compare to your own.

@Defee
Copy link

Defee commented Jun 17, 2019

@Defee I have pretty much just taken the Serilog Implementation and renamed it to NLog. You can see my actual changes in PR #176 and compare to your own.

Sure will do over the week.

P.S. Tested now it seems it builds as of now and tests pass.
P.P.S. I was actually thinking to add ILogEvent interface addition to abstractions as for some things it is really useful (ex: app insights.)

@Defee
Copy link

Defee commented Jun 24, 2019

@snakefoot
I saw that the stakeframe test is failing for the implementation when I merged your code.
I think I faced it before and added a workaround for it from my side:

        /// <summary>
        /// Actually sends the message to the underlying log system.
        /// </summary>
        /// <param name="logLevel">the level of this log event.</param>
        /// <param name="message">the message to log</param>
        /// <param name="exception">the exception to log (may be null)</param>
        protected override void WriteInternal(LogLevel logLevel, object message, Exception exception)
        {
            LogLevelNLog level = GetLevel(logLevel);
            LogEventInfo logEvent =
                LogEventInfo.Create(level, _logger.Name, exception, null,"{0}", new object[] {message});

            _logger.Log(declaringType, logEvent);
        }

        private static LogLevelNLog GetLevel(LogLevel logLevel)
        {
            switch (logLevel)
            {
                case LogLevel.All:
                    return LogLevelNLog.Trace;
                case LogLevel.Trace:
                    return LogLevelNLog.Trace;
                case LogLevel.Debug:
                    return LogLevelNLog.Debug;
                case LogLevel.Info:
                    return LogLevelNLog.Info;
                case LogLevel.Warn:
                    return LogLevelNLog.Warn;
                case LogLevel.Error:
                    return LogLevelNLog.Error;
                case LogLevel.Fatal:
                    return LogLevelNLog.Fatal;
                case LogLevel.Off:
                    return LogLevelNLog.Off;
                default:
                    throw new ArgumentOutOfRangeException("logLevel", logLevel, "unknown log level");
            }
        }

What side-effects can it bring?
my fork

@snakefoot
Copy link

snakefoot commented Jun 24, 2019

@Defee I saw that the stakeframe test is failing for the implementation when I merged your code.

Thank you for reporting this. Guess that is the price of copy-paste-code. Guess it is time to divert from the Serilog-default-implementation.

What side-effects can it bring?

Well WriteInternal in my PR is never called (That is why it is empty). Because Common.Logging enforces use of string.Format but Serilog (And NLog) wants to use their own parsing of the format-string (To support message templates) . Therefore they put great effort in avoiding Common.Logging "helper"-logic.

I have now updated my PR #176 to include better handling of callsite.

@Defee
Copy link

Defee commented Jun 25, 2019

Thanks @snakefoot! Will merge it again.

I will also give a heads-up after I finish out this.

@Defee
Copy link

Defee commented Jun 28, 2019

@sbohlen @snakefoot
Some heads-up:

I uploaded some packages manually to nuget as pre-release with my prefix("defee.").:

  • Common.Logging.Core
  • Common.Logging
  • Common.Logging.NLog45

I checked following things and what have I done back in November 2018:

  • Migrating Common assemblies to the netframework2.0 (Common.Logging, Common.Loggin.Core)
  • Updating the CommonLoggingIntegration with NLog. Taking into consideration PR: Common.Logging.NLog45 with support for structured logging #176
  • Migration for the Serilog implementation.
  • Migration for the ApplicationInsigts.
  • Migration for the log4net.
  • Making log4net tests pass.
  • Migration EntLib to support versioning for the net45+
  • Check implementation and posssibility of the migration of the ETWLogger
  • Check that nothing was broken for the old assemblies and the old solutions still build and tests pass there.

I wanted to suggest the following approach for the 4.x.x versioning

  1. Use the .csproj for specifying nuget related properties (imho it is much more convinient then it was before with nuspec...)
  2. Use different approach to handle compatibility and issues for the particular version integrations. (I outlined it below)
  3. Decouple the solution/git repo a little bit so we have less mess with components.

Current Issues and breaking changes(as per my memory now):

  1. I migrated all the assemblies to the 4.5+ versioning, so newer versions will not support old frameworks.
  2. I plan to have separate solution which will have only new netstandard compatible assemblies and keep old solutions for a while.
  3. I didn't find a way to support portable so it will be broken and can be replaced with the netstandard versions I bellieve.
  4. As Silverlight will meet its sunset soon. I didn't migrate it to the netframework.
  5. As there are a lot of known issues for the AssemblyInfo attributes it was moved to the .csproj files.
  6. I haven't made any changes to nuspec files as I use .csproj to pack.
  7. I remember I implemented the following lookup files for the common.logging in netstandard via Microsoft Configuration extensions: "appsettings.json", "appsettings.xml", "CommonLoggingCfg.json", "CommonLoggingCfg.xml". If you specified one of the files it should be working as usual. In future we might want to have the setup for configuration or passing IConfigurationOptions to common logging from upper layers and changed it as the preferable lookup for the common logging.

Suggested approach for the versioning integrations and checking those separately.

  1. Separte the main Common.Logging assemblies which are related to the framework specificly with the integrations. Structure suggested:
    1. Common.Logging.Core, Common.Logging, Common.Logging.Common - current repo
    2. Common.Logging.NLog and relavant to NLog tests - separate repository which will have related integration assembly and Integrational tests.
    3. Common.Logging.log4net and relavant to log4net tests - separate repository which will have related integration assembly and Integrational tests.
    4. Common.Logging.EntLib and relavant to EntLib tests - separate repository which will have related integration assembly and Integrational tests. (if we still plan to support it)
    5. Common.Logging.Serilog and relavant to Serilog tests - separate repository which will have related integration assembly and Integrational tests.
    6. Common.Logging.ApplicationInsights and relevant tests - separate repository which will have related integration assembly and Integrational tests and also will have the integration with default ILogger from .netcore.
  2. Use the branches in the above repositories related to integrations to monitor compatibility, also it might be a compatibility builds which will restore particular versions. Ex for NLog:
    1. NLog - master - contains latest supported version, with common workarounds and files for all the versions. When the new versions is shipped by the NLog we just branch out new release and update the master to the latest.
    2. NLog - release4.5 - containing the package verisions and amendements related only to this version.
    3. NLog - release4.1 - containing the package verisions and amendements related only to this version, etc.
  3. Use PackageReferences in the independant projects instead of the project references. So we can test against particular version of common.logging. It will also give a touch of real environment :()

The pros of the suggested approach are following:

  1. Better management for the issues.
  2. Better maintainability for the framework as the whole.
  3. Faster delivery of particular version or integration. Better troubleshouting as you can just take relevant branch and see if it is version specific bug.
  4. More independence for the integrations of the main code base.
  5. Better use for the versions compatibility and builds as you can create a separate branch per integration version which will have the particular configuration or use CI builds to trigger other builds for the repos.

Cons:

  1. More headache and maintainance for the CI builds and release builds.
  2. Might be some code duplication for tests which can be resolved with the build tools.

PS.
@sbohlen, I can volonteer for the change above.

@robsosno
Copy link

I vote for original proposition: version 4.0 only for Dotnet Core.
In theory NetStandard should be soon supported anywhere so it seems perfect solution.
Beware of these issues: https://weblog.west-wind.com/posts/2019/Feb/19/Using-NET-Standard-with-Full-Framework-NET
In particular important issue is with configuration. Current dotnet implementation of Common.Logging doesn't support old configuration at all. Which is a mistake because of legacy applications.
I have following situation in my company:
Old applications (WCF, Web Forms) --> our common libraries --> Common.Logging
New applications (Asp.Net Core) --> our common libraries --> Common.Logging
So you see that we would prefer to have only one version of Common.Logging to be used anywhere. Otherwise we would have to multitarget common libraries.
But old applications require that app.config/web.config is handled properly by NetStandard version of Common.Logging

In my private clone of Common.Logging I've added support for old configurations. I've observed following:
ConfigurationManager is not supported in any version of NetStandard. It can be provided by NuGet package. This however have disadvantages: what if Nuget used for common.logging has incompatible version with that used by the client application?
Proposed solution: to create separate assembly Common.Logging.Configuration, It could be written in old .NET Framework as it would be used only for legacy applications. Disadvantage: configure will not be loaded automatically, client application have to call something like LogManager.Configure()
I think that it is acceptable.

Next thing is that Common.Logging should provide implementation for Microsoft.Extensions.Logging abstraction.
We need Common.Logging because we are using Spring.Net in our company, and we need Microsoft.Extensions.Logging because of ASP.NET Core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants