Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Allow configuring the trace source logger without having to flow a TraceListener instance #405

Closed
kevinchalet opened this issue Apr 13, 2016 · 6 comments
Assignees
Milestone

Comments

@kevinchalet
Copy link

Unlike Katana's DiagnosticsLoggerFactory, the vNext equivalent now requires a TraceListener, which makes the trace source logger a bit painful to use in practice, since you can't simply rely on the listeners defined at the static level (that may be registered via the .config file on .NET Desktop)

Please consider updating TraceSourceLoggerProvider to make this parameter optional and offering a simpler AddTraceSource that doesn't require a specific trace listener.

@davidfowl do you remember why you had removed the parameterless constructor? https://github.com/aspnet/Logging/pull/143/files#diff-ca74d694fa53328497b21b4a9528dae5L28

/cc @muratg

@muratg
Copy link

muratg commented Apr 21, 2016

@davidfowl why did we remove this constructor?

@muratg muratg added this to the 1.0.0 milestone Apr 28, 2016
@muratg
Copy link

muratg commented Apr 28, 2016

Tentatively putting it in 1.0.0. We'll discuss after RC2 ships.

@moozzyk
Copy link
Contributor

moozzyk commented May 23, 2016

Fixed in 5019dd2

@kevinchalet
Copy link
Author

@moozzyk thanks!

Quick question: any particular reason to not provide an overload of AddTraceSource taking no TraceListener parameter? Because it's great to make it optional in the provider class itself, but currently, an exception will be thrown if you try to use the AddTraceSource extensions:

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.TraceSource/TraceSourceFactoryExtensions.cs#L27
https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.TraceSource/TraceSourceFactoryExtensions.cs#L50

moozzyk referenced this issue May 23, 2016
@moozzyk
Copy link
Contributor

moozzyk commented May 23, 2016

@PinpointTownes - thanks for pointing this out. I will look into this.

@moozzyk moozzyk reopened this May 23, 2016
moozzyk pushed a commit that referenced this issue May 23, 2016
We enabled passing null TraceListener but extension methods were not updated accordingly and would still not allow to pass a null TraceListner.
moozzyk pushed a commit that referenced this issue May 23, 2016
We enabled passing null TraceListener but extension methods were not updated accordingly and would still not allow to pass a null TraceListner.
moozzyk pushed a commit that referenced this issue May 23, 2016
We enabled passing null TraceListener but extension methods were not updated accordingly and would still not allow to pass a null TraceListner.
@moozzyk
Copy link
Contributor

moozzyk commented May 23, 2016

Added overloads that don't take TraceListener e6af0dc

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

No branches or pull requests

3 participants