Skip to content

Fixes #97, #83, #98 #100

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

Merged
merged 13 commits into from
Apr 21, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,41 @@ For example, to set the minimum log level using the _Windows_ command prompt:
set Serilog:MinimumLevel=Debug
dotnet run
```

### Nested configuration sections
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the additional docs 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to remove nested config "hint" hack -- but I left the blurb there about nested config support, figured it can't hurt to document it.


Some Serilog packages require a reference to a logger configuration object. These must be "hinted" by appending a `>` character to the end of the argument name. The sample program in this project illustrates this with the following entry configuring the _Serilog.Sinks.Async_ package to wrap the _Serilog.Sinks.File_ package.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a bit of a UX regression, and it's not immediately clear when this will or won't be a breaking change; can't we work out what kind of argument we're dealing with by reflecting over the parameters? I am afraid I'm missing something here, sorry - struggling to keep up :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take another look, but other than the Async wrapper package, frankly I'm not clear on the use cases for nested config. (For example, the reason for the logger nested config in the sample code and tests makes no sense to me.) I don't really like the hack myself, but at the point the code is "deciding" how to store the config section (the parameter value, effectively), we don't yet know what the parameter type is. "How to store" means the various parameter-value classes (string, config, or my new object class). I suspect the config value class can go away and that can be added to the object value class.

Copy link
Contributor Author

@MV10 MV10 Apr 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Very glad you questioned this -- the "hint" hack is gone and the code is simplified! I took another look at what was going on in ConfigurationSectionArgumentValue and it turns out I could get rid of that and move the functionality into ObjectArgumentValue. The trick was deferring the new ConfigurationReader pass to read the nested config until we are actually calling the target method. Now the read is triggered by the Action<> parameter's type rather than hinting and/or "assuming" based on config structure.

This made me notice #103


Note the `configure>` parameter name with the trailing hint character:

```json
"WriteTo:Async": {
"Name": "Async",
"Args": {
"configure>": [
{
"Name": "File",
"Args": {
"path": "%TEMP%\\Logs\\serilog-configuration-sample.txt",
"outputTemplate": "{Timestamp:o} [{Level:u3}] ({Application}/{MachineName}/{ThreadId}) {Message}{NewLine}{Exception}"
}
}
]
}
},
```

### IConfiguration parameter

If a Serilog package requires additional external configuration information (for example, access to a `ConnectionStrings` section, which would be outside of the `Serilog` section), the sink should include an `IConfiguration` parameter in the configuration extension method. This package will automatically populate that parameter. It should not be declared in the argument list in the configuration source.

### Complex parameter value binding

When the configuration specifies a discrete value for a parameter (such as a string literal), the package will attempt to convert that value to the target method's declared CLR type of the parameter. Additional explicit handling is provided for parsing strings to `Uri` and `TimeSpan` objects and `enum` elements.

If the parameter value is not a discrete value, the package will use the configuration binding system provided by _Microsoft.Extensions.Options.ConfigurationExtensions_ to attempt to populate the parameter. Almost anything that can be bound by `IConfiguration.Get<T>` should work with this package. An example of this is the optional `List<Column>` parameter used to configure the .NET Standard version of the _Serilog.Sinks.MSSqlServer_ package.

### IConfigurationSection parameters

Certain Serilog packages may require configuration information that can't be easily represented by discrete values or direct binding-friendly representations. An example might be lists of values to remove from a collection of default values. In this case the method can accept an entire `IConfigurationSection` as a call parameter and this package will recognize that and populate the parameter. In this way, Serilog packages can support arbitrarily complex configuration scenarios.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in the long run that one is going to be very useful to sink authors.


4 changes: 2 additions & 2 deletions sample/Sample/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"WriteTo:Sublogger": {
"Name": "Logger",
"Args": {
"configureLogger": {
"configureLogger>": {
"WriteTo": [
{
"Name": "Console",
Expand All @@ -28,7 +28,7 @@
"WriteTo:Async": {
"Name": "Async",
"Args": {
"configure": [
"configure>": [
{
"Name": "File",
"Args": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,18 @@ namespace Serilog
/// </summary>
public static class ConfigurationLoggerConfigurationExtensions
{
const string DefaultSectionName = "Serilog";
/// <summary>
/// Configuration section name required by this package.
/// </summary>
public const string DefaultSectionName = "Serilog";

/// <summary>
/// Reads logger settings from the provided configuration object using the default section name.
/// Reads logger settings from the provided configuration object using the default section name. Generally this
/// is preferable over the other method that takes a configuration section. Only this version will populate
/// IConfiguration parameters on target methods.
/// </summary>
/// <param name="settingConfiguration">Logger setting configuration.</param>
/// <param name="configuration">A configuration object with a Serilog section.</param>
/// <param name="configuration">A configuration object which contains a Serilog section.</param>
/// <param name="dependencyContext">The dependency context from which sink/enricher packages can be located. If not supplied, the platform
/// default will be used.</param>
/// <returns>An object allowing configuration to continue.</returns>
Expand All @@ -42,28 +47,32 @@ public static LoggerConfiguration Configuration(
DependencyContext dependencyContext = null)
{
if (configuration == null) throw new ArgumentNullException(nameof(configuration));
return settingConfiguration.ConfigurationSection(configuration.GetSection(DefaultSectionName), dependencyContext);
return settingConfiguration.Settings(
new ConfigurationReader(
configuration,
dependencyContext ?? (Assembly.GetEntryAssembly() != null ? DependencyContext.Default : null)));
}

/// <summary>
/// Reads logger settings from the provided configuration section.
/// Reads logger settings from the provided configuration section. Generally it is preferable to use the other
/// extension method that takes the full configuration object.
/// </summary>
/// <param name="settingConfiguration">Logger setting configuration.</param>
/// <param name="configuration">The Serilog configuration section</param>
/// <param name="configSection">The Serilog configuration section</param>
/// <param name="dependencyContext">The dependency context from which sink/enricher packages can be located. If not supplied, the platform
/// default will be used.</param>
/// <returns>An object allowing configuration to continue.</returns>
public static LoggerConfiguration ConfigurationSection(
this LoggerSettingsConfiguration settingConfiguration,
IConfigurationSection configuration,
IConfigurationSection configSection,
DependencyContext dependencyContext = null)
{
if (settingConfiguration == null) throw new ArgumentNullException(nameof(settingConfiguration));
if (configuration == null) throw new ArgumentNullException(nameof(configuration));
if (configSection == null) throw new ArgumentNullException(nameof(configSection));

return settingConfiguration.Settings(
new ConfigurationReader(
configuration,
configSection,
dependencyContext ?? (Assembly.GetEntryAssembly() != null ? DependencyContext.Default : null)));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
<PackageProjectUrl>https://github.com/serilog/serilog-settings-configuration</PackageProjectUrl>
<PackageLicenseUrl>https://www.apache.org/licenses/LICENSE-2.0</PackageLicenseUrl>
<RootNamespace>Serilog</RootNamespace>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<Version>3.0.0</Version>
<AssemblyVersion>3.0.0</AssemblyVersion>
<FileVersion>3.0.0</FileVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these should be necessary - will have a shot now at getting this to build without them, then merge 👍

</PropertyGroup>

<ItemGroup>
Expand All @@ -26,10 +30,12 @@

<ItemGroup Condition="'$(TargetFramework)' == 'net451'">
<PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="1.1.2" />
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="1.1.2" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="2.0.1" />
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="2.0.0" />
</ItemGroup>

</Project>
Loading