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

Use Microsoft.Extensions.Logging.Abstractions for Logging #47

Closed
jimmymcpeter opened this issue Apr 26, 2018 · 2 comments
Closed

Use Microsoft.Extensions.Logging.Abstractions for Logging #47

jimmymcpeter opened this issue Apr 26, 2018 · 2 comments

Comments

@jimmymcpeter
Copy link
Contributor

Instead of NLog exclusively, the SDK should consider using Microsoft.Extensions.Logging.Abstractions instead.

@intacctcoleenho
Copy link
Contributor

Pros of using NLog directly

Best performance
More options with the logger API, e.g. Logger.WithProperty(..)
Works in all platforms
No Dependency Injection needed which saves complexity.

Pros of using NLog via Microsoft.Extensions.Logging:

Fully integrated with ASP.NET Core, e.g. Microsoft also writes to the logger API and that will be also captured (and possible filtered) by NLog
Writing to the logging abstraction will make your code log-library independent.
Works well with .NET Core Dependency injection
new: You could configure NLog with appsettings.json - so with JSON instead of XML

https://stackoverflow.com/questions/58209076/microsoft-extensions-logging-vs-nlog

@intacct intacct deleted a comment from coleenho Aug 18, 2020
@bmeredith
Copy link

bmeredith commented Sep 9, 2020

I would argue to not lock the consumers of Intacct.Sdk to a specific library (e.g. NLog), and instead leave it up to the consumer to decide how they want the logging done. Using a specific library for logging in library code such as this can cause issues with dependencies on the consumer side (e.g. diamond dependencies, versioning conflicts, etc.). It is something our team has struggled with in the past using this library.

Two ideas that other libraries use for removing a logging dependency:

  1. Expose an ILog interface so that the consumer can provide their own implementation and register it with the library. Then Intacct.Sdk can call the relevant log functions on the implementation registered by the consumer.
  2. Lean on Microsoft.Extensions.Logging.Abstractions and expose its ILogger that can be passed into the library for logging to the consumer. This still takes on a dependency to Microsoft.Extensions.Logging.Abstractions, but it's enough of an abstraction and common enough in .NET Core that it's not as big of a deal.

By removing the dependency, this will allow consumers that use other logging libraries (e.g. Serilog, log4net, etc.) to not have the possibility of running into the issues mentioned above.

NagBoranna pushed a commit that referenced this issue Nov 6, 2020
- Convert from .NET Standard 1.6 to .NET Standard 2.0 (#123, #101)
- Remove 1.0, 1.1, 2.0 .NET Core support due to EOL, and add 3.1 Core support
- Update to Visual Studio Version 16
- Add Purchasing Transactions for approve and decline
- Add Exchange Rate test coverage for Create ApPayment
- Change from NLog to Microsoft.Extensions.Logging (#47)
- APPYMT support implemented for ApPaymentCreate
- Refactor ApPaymentRequest* to ApPayment*
- Fix isnegate for InList in ReadByQuery
- Remove mutual exclusion rule for lot number and serial number for item detail (#108)
- Update dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants