[sdk-logs] Add Sdk.CreateLoggerProviderBuilder #4464
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4464 +/- ##
=======================================
Coverage 83.32% 83.32%
=======================================
Files 314 314
Lines 12524 12525 +1
=======================================
+ Hits 10435 10437 +2
+ Misses 2089 2088 -1
|
| /// to build a <see cref="LoggerProvider"/>.</returns> | ||
| internal static LoggerProviderBuilder CreateLoggerProviderBuilder() | ||
| { | ||
| return new LoggerProviderServiceCollectionBuilder(); |
There was a problem hiding this comment.
Could we rename LoggerProviderServiceCollectionBuilder to LoggerProviderBuilderBase to stay consistent with Traces and Metrics?
There was a problem hiding this comment.
We can, but I would prefer not to. Here is the reasoning...
If you look at the 1.3.1 branch, hosting used to have two classes TracerProviderBuilderHosting and MeterProviderBuilderHosting. Those used the TracerProviderBuilderBase & MeterProviderBuilderBase base classes which were shipped public.
Things changed in 1.4 and we no longer use inheritance to make this stuff work. The new LoggerProviderServiceCollectionBuilder doesn't need to ever be made public (it is sealed class).
So we could rename it LoggerProviderBuilderBase but that would be misleading IMO because it isn't a base class thing. I picked the current name because I thought it better reflected the design and made things more understandable (at the cost of some consistency). I would rename TracerProviderBuilderBase & MeterProviderBuilderBase if I could be we already shipped them stable 😭
Relates to #4433
Changes
Sdk.CreateLoggerProviderBuilderAPIMerge requirement checklist