Replace IConfigCatLogger with Microsoft.Extensions.Logging.ILogger

[damianh]

Is your feature request related to a problem? Please describe.

Minor inconvenience of having to author an adapter between IConfigCatLogger and Microsoft.Extensions.Logging.ILogger

Describe the solution you'd like

Be able to inject an Microsoft.Extensions.Logging.ILogger into ConfigCatClient factory method.

Describe alternatives you've considered

N/A

Additional context

N/A

[adams85]

Hi @damianh,

Although MS.Extensions.Logging is the standard logging facade for ASP.NET Core, it is not necessarily used in other types of applications. So, since we aim to provide as broad support for other application types / .NET platforms as possible, I don't think we want to force all of our users to adopt (or adapt to) MS.Extensions.Logging. E.g. for a user who has a WPF app which was built on, let's say, Serilog, MS.Extensions.Logging would be an extra, unnecessary dependency, for which they would need an adapter anyway.

So, in overall, it seems to be a better choice to not favor a single logging framework over the others but provide a uniform solution to all SDK users.

Of course, we are aware that MS.Extensions.Logging is a very common choice, so we already created an adapter for this one (which supports even structured logging). You can find it here.

I believe it is a tolerable DX to copy-paste a file into your project but let us know if you think otherwise.

[damianh]

Hi @adams85,

Thanks for response! I'm quite familiar with the topic of logger interfaces, frameworks, adapters, and in particular, dependencies - back in the day I shipped LibLog to solve this for library developers.

So, in overall, it seems to be a better choice to not favor a single logging framework over the others but provide a uniform solution to all SDK users.

I'm not saying favouring any single logging framework but instead use the abstraction the is now the defacto abstraction in .NET, for which every logging framework already has an adapter. Since it's shipped by MS (unlike Common.Logging of yore) it's a fairly safe dependency to take, I will argue. The issue with having your own unique interface, and not ship a suite of adapters, is that users will write their own and/or copy code. I'll argue that is unnecessary today when the ecosystem is coalescing around MSEL (for better or worse, only MS is enabled to ship ecosystem level interfaces).

If I can't convince you after this message, feel free to close the issue. I'll copy the adapter and move on. :)

[adams85]

Your arguments are perfectly valid, MSEL is indeed the de facto standard now.

However, in our case there are some other factors that also need to be taken into account when we are considering switching to MSEL.

• We still support .NET 4.5. The latest version of MSEL which is compatible with .NET 4.5 is v1.x. On top of that, those versions are deprecated in NuGet.
• The switch to MSEL would be a breaking change. We have a guideline not to break public APIs overnight so our existing users have time to prepare for the breaking changes. Because of this, we could do the switch in two steps only. As the first step, we would need to deprecate the existing APIs and introduce the new ones. That is, for some time we would need to support both the current and the MSEL way of logging. AFAICS, this would be pretty messy. Especially for the following two reasons:
  1. Both we and MSEL define a LogLevel enum, which would co-exist for a while.
  2. We need to provide simple console logging for newcomers, where they can set the log filter easily. We cannot expect them to have properly configured MSEL logging in their applications (which, IIRC, would also involve configuring DI nowadays...)
• We are rolling out a pretty big enhancement soon and we want the upgrade experience to be as smooth as possible. We don't want any confusion which our existing users need to deal with when they upgrade.

So, no matter how i look at it, the time is not right for this change at the moment. However, there are no hard blockers either, so nothing stops us from doing this in the future. We have plans to drop the support for .NET 4.5 sooner or later. When that time comes, we should definitely reconsider your idea, so I will leave open the issue.

[damianh]

.NET 4.5 is a very valid point and at the time of dropping that would seem a good time to consider this.

I certainly wasn't expecting a such breaking a change overnight; just wanted get the request in so it's considered at the right time. On a related note - yeah more alignment with DI and options patterns used nowadays would also be appreciated.

Cheers.

[github-actions[bot]]

This issue is marked stale because it has no activity in the last 3 weeks. The issue will be closed in one week. Please remove the stale flag to keep it open.

[luizbon]

Hi @adams85, I understand the backward compatibility need and how to keep that stability for ConfigCat customers. But on the other hand, this can also impact new customers evaluating the tool.

Knowing and being aligned with Microsoft's release and support cycles, it is fair to consider moving forward and keeping the SDK current with the new standards.

Would it be possible to decouple the current logging structure from the SDK and provide sub-packages? It would work like a Dependency Injection, which can follow the MSEL interfaces, and the current logging structure could adapt to those interfaces. Then, yes, it'll be a breaking change, but the fix would be as easy as upgrading the current package and importing a new package. With one extra line of code, the setup could be done and the forward logging approach would be solved, even not needing to maintain any logging code as any external logging framework would do the work.

This does the inverse of what every new customer needs to do by creating a logging adapter, but it'll serve legacy code instead, whereas, for new implementations, virtually nothing needs to be done.

[adams85]

Hey @luizbon,

You're right that the current solution might not be optimal for customers who are already on the .NET Core/NET 5+ train. I consulted with the team and we all agree that the situation could be improved. However, we still think that it would be better to keep the platform-agnostic logging facade in the core SDK package, for the reasons mentioned above.

We think the best solution would be something like what you suggest but in reverse: let's leave the ConfigCat.Client package as is and introduce an "integration" package (that depends on the former) for MS.Extensions.-based applications, where we could place all the glue code which is needed for making the SDK easy to integrate with newer .NET apps using MS.Extensions.\. (And here I'm thinking not only of logging but also of MS DI, the options API, etc.) On the one hand, this way we won't introduce a breaking change and won't force non-MS.Extensions users to have a (transitive) dependency on MSEL. On the other hand, newer customers would just need to reference the integration package instead of the core package to get a more convenient developer experience.

How do you like this idea? Would this be an acceptable solution for you?

[luizbon]

Thanks for evaluating the idea @adams85. I think it is a good idea, the goal is to keep decoupling and yet make it easy to integrate with external solutions, so this composition approach sounds better.

[damianh]

Sounds like a reasonable trade off.

[adams85]

Thank you both for sharing your opinions and suggestions.

We added this item to our backlog. I can't give you an ETA yet but we are going to implement it eventually.

[github-actions[bot]]

This issue is marked stale because it has no activity in the last 3 weeks. The issue will be closed in one week. Please remove the stale flag to keep it open.