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

ConfigurationSection() adds sub-section in version 3.0.0; configurable default section name #143

Closed
takerukoushirou opened this issue Oct 11, 2018 · 7 comments · Fixed by #144
Labels

Comments

@takerukoushirou
Copy link

I've been using the ReadFrom.ConfigurationSection() extension method solely for reading the logger configuration from a configuration section with a name that differs from the hard-coded default (e.g. "Logging" instead of "Serilog").

Version 3.0.0 no longer uses the provided configuration section directly, but instead calls GetSection(DefaultSectionName), adding a hard-coded sub-section. This change was seemingly introduced with commit 80b2ec9 or one thereafter. I am not sure whether this is on purpose, but using the provided configuration section directly as in version 2.6.1 and earlier seems more logical and also fits the documented behaviour of the extension method.
After having a quick look at the code path in the debugger, it seems that probably just the wrong constructor overload for ConfigurationReader is called in the 3.0.0 NuGet package assembly.

Regardless of the descrived issue, it would be great if the default section name could be configured (or provided as an optional parameter). If that were possible, the recommended ReadFrom.Configuration() extension method could be used instead if the only difference is just a different section name.

@nblumhardt
Copy link
Member

Thanks for the report @takerukoushirou - sounds like a bug 👍

@nblumhardt nblumhardt added the bug label Oct 11, 2018
@tsimbalar
Copy link
Member

tsimbalar commented Oct 13, 2018

FYI, a repro :

The following tests fails with Serilog.Settings.Configuration v3.0.0.
It passed with Serilog.Settings.Configuration v2.6.1

[Fact]
[Trait("BugFix","https://github.com/serilog/serilog-settings-configuration/issues/143")]
public void ReadFromConfigurationSectionReadsFromAnArbitrarySection()
{
	LogEvent evt = null;

	var json = @"{
		""NotSerilog"": {            
			""Properties"": {
				""App"": ""Test""
			}
		}
	}";

	var config = new ConfigurationBuilder()
		.AddJsonString(json)
		.Build();

	var log = new LoggerConfiguration()
		.ReadFrom.ConfigurationSection(config.GetSection("NotSerilog"))
		.WriteTo.Sink(new DelegatingSink(e => evt = e))
		.CreateLogger();

	log.Information("Has a test property");

	Assert.NotNull(evt);
	Assert.Equal("Test", evt.Properties["App"].LiteralValue());
}

tsimbalar added a commit to tsimbalar/serilog-settings-configuration that referenced this issue Oct 13, 2018
fixes serilog#143 where we are trying to read a subsection of config section passed as parameter
@tsimbalar
Copy link
Member

A fix should be available in the pre-release nuget package for v3.0.1:

https://www.nuget.org/packages/Serilog.Settings.Configuration/3.0.1-dev-00160

It would be great if you could give it a try !

@tsimbalar
Copy link
Member

Leaving it open until we are sure it is fixed

@tsimbalar tsimbalar reopened this Oct 15, 2018
@takerukoushirou
Copy link
Author

@tsimbalar I upgraded to the 3.0.1 pre-relase in a project that previously used version 2.6.1 and all logs seem to have retained their configuration. A quick look with the IDE into the decompiled code also shows that the section overload is now used.

Thanks a lot for the quick fix!

@tsimbalar
Copy link
Member

Thanks for looping back !

@tsimbalar
Copy link
Member

tsimbalar commented Oct 17, 2018

Package v3.0.1 has now been released. - https://www.nuget.org/packages/Serilog.Settings.Configuration/3.0.1

Thanks for the feedback !

PS: another issue has been created regarding the section name and configuration, see #148

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