serilog-mssql / serilog-sinks-mssqlserver

A Serilog sink that writes events to Microsoft SQL Server and Azure SQL
Apache License 2.0
278 stars 148 forks source link

ConnectionString not being read from ConnectionStrings section #150

Closed craigmoliver closed 4 years ago

craigmoliver commented 6 years ago

I do not want to duplicate my connection string so I attempted to do the following per your documentation:

"To use a connection string from the connectionStrings section of your application config, specify its name as the value of the connection string."

This does not work when setting configuration in the appSettings.json using the serilog/serilog-settings-configuration package. Only actual connection strings work, not the name of the settings from the ConnectionStrings section.

MV10 commented 6 years ago

Please post your config file and your init code. Also please post the versions you're using.

Are you configuring the sink directly or via ReadFromConfiguration with Serilog.Settings.Configuration?

If you're configuring it directly, it probably isn't clear from the docs (we're having a discussion now about improving Serilog docs in general), but to use a named connection string, you must use the appConfiguration parameter to pass the entire configuration rather than just the 'columnOptionsSection'. But that is populated automatically if you're configuring from the Settings package.

The only other two requirements that come to mind is that the connection string section must be named ConnectionStrings (case-sensitive) but I think the connection string name-search itself is case-insensitive (we use the GetConnectionString method built into Microsoft.Extensions.Configuration), and the name of the connection string must not contain an = symbol (that determines whether we treat it as a raw string or a name).

We actually have a passing test for this: https://github.com/serilog/serilog-sinks-mssqlserver/blob/dev/test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Microsoft.Extensions.Configuration/ConfigurationExtensions.cs#L34

MV10 commented 6 years ago

Using the dev version of the SQL sink and the 3.0.0 release of the config package, this code and config works:

var appConfig = new ConfigurationBuilder()
    .SetBasePath(Directory.GetCurrentDirectory())
    .AddJsonFile(path: "appsettings.json")
    .Build();

var logConfig = new LoggerConfiguration();
logConfig.ReadFrom.Configuration(appConfig);
Log.Logger = logConfig.CreateLogger();
Serilog.Debugging.SelfLog.Enable(Console.Error);

Log.Information("An Information message");

Log.CloseAndFlush();
{
  "ConnectionStrings": {
    "DevTest": "Data Source=(LocalDB)\\MSSQLLocalDB;Initial Catalog=SerilogTest;Integrated Security=true"
  },
  "Serilog": {
    "Using": [ "Serilog.Sinks.Console" ],
    "WriteTo": [
      { "Name": "Console" },
      {
        "Name": "MSSqlServer",
        "Args": {
          "connectionString": "DevTest",
          "tableName": "Issue81",
          "autoCreateSqlTable":  true,
          "columnOptionsSection": {
            "customColumns": [
              {
                "ColumnName": "EventType",
                "DataType": "int"
              },
              {
                "ColumnName": "Release",
                "DataType": "varchar",
                "DataLength": 32
              }
            ]
          }
        }
      }
    ]
  }
}
OculiViridi commented 6 years ago

@MV10 When is it expected to be released the final 5.1.3 version of serilog-sinks-mssqlserver package?

MV10 commented 6 years ago

@OculiViridi not my call, but once I'm done with this change I'm going to talk to Nick about doing a release. Cumulative dev changes are big enough to justify a bump to 5.2 ... I even think 6.0 could be warranted, but I don't work with .NET Framework any more (increasingly pointless) and that side has been neglected. I started working in this sink specifically because I needed it for Core but it didn't support modern config sources. But maybe I'll bite the bullet and bring that up to snuff, too.

MV10 commented 6 years ago

@craigmoliver I also set up the test with a bogus connection string, and this is what hits the console via SelfLog upon auto-create and then after a message is logged:

2018-10-10T14:19:41.3378268Z Exception System.Data.SqlClient.SqlException (0x80131904): A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: SQL Network Interfaces, error: 26 - Error Locating Server/Instance Specified)
   at System.Data.SqlClient.SqlInternalConnectionTds..ctor(DbConnectionPoolIdentity identity, SqlConnectionString connectionOptions, Object providerInfo, Boolean redirectedUserInstance, SqlConnectionString userConnectionOptions, SessionData reconnectSessionData, Boolean applyTransientFaultHandling)
   at System.Data.SqlClient.SqlConnectionFactory.CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, Object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection, DbConnectionOptions userOptions)
   at System.Data.ProviderBase.DbConnectionFactory.CreatePooledConnection(DbConnectionPool pool, DbConnection owningObject, DbConnectionOptions options, DbConnectionPoolKey poolKey, DbConnectionOptions userOptions)
   at System.Data.ProviderBase.DbConnectionPool.CreateObject(DbConnection owningObject, DbConnectionOptions userOptions, DbConnectionInternal oldConnection)
   at System.Data.ProviderBase.DbConnectionPool.UserCreateRequest(DbConnection owningObject, DbConnectionOptions userOptions, DbConnectionInternal oldConnection)
   at System.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, UInt32 waitForMultipleObjectsTimeout, Boolean allowCreate, Boolean onlyOneCheckConnection, DbConnectionOptions userOptions, DbConnectionInternal& connection)
   at System.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, TaskCompletionSource`1 retry, DbConnectionOptions userOptions, DbConnectionInternal& connection)
   at System.Data.ProviderBase.DbConnectionFactory.TryGetConnection(DbConnection owningConnection, TaskCompletionSource`1 retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, DbConnectionInternal& connection)
   at System.Data.ProviderBase.DbConnectionInternal.TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource`1 retry, DbConnectionOptions userOptions)
   at System.Data.SqlClient.SqlConnection.TryOpen(TaskCompletionSource`1 retry)
   at System.Data.SqlClient.SqlConnection.Open()
   at Serilog.Sinks.MSSqlServer.SqlTableCreator.CreateTable() in C:\Source\Repos\serilog-sinks-mssqlserver\src\Serilog.Sinks.MSSqlServer\Sinks\MSSqlServer\SqlTableCreator.cs:line 32
   at Serilog.Sinks.MSSqlServer.MSSqlServerSinkTraits..ctor(String connectionString, String tableName, String schemaName, ColumnOptions columnOptions, IFormatProvider formatProvider, Boolean autoCreateSqlTable) in C:\Source\Repos\serilog-sinks-mssqlserver\src\Serilog.Sinks.MSSqlServer\Sinks\MSSqlServer\MSSqlServerSinkTraits.cs:line 84
ClientConnectionId:00000000-0000-0000-0000-000000000000
Error Number:-1,State:0,Class:20 caught while creating table Issue81 to the database specified in the connection string.

[10:19:41 INF] An Information message

2018-10-10T14:19:41.4435681Z Unable to write 1 log events to the database due to following error: A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: SQL Network Interfaces, error: 26 - Error Locating Server/Instance Specified)
nlz242 commented 5 years ago

Hello,

I'm facing the same issue as @craigmoliver. Here's a minimalist repro using an Aspnet core project (running full framework, don't think it matters, but you never know).

    public class Program
    {
        public static void Main(string[] args)
        {
            CreateWebHostBuilder(args).Build().Run();
        }

        public static IWebHostBuilder CreateWebHostBuilder(string[] args) =>
            WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>()
                .UseSerilog((hostingContext, loggerConfiguration) =>
                {
                    loggerConfiguration.ReadFrom.Configuration(hostingContext.Configuration);
                });
    }

   public class Startup
    {
        private readonly ILoggerFactory _loggerFactory;
        public Startup(ILoggerFactory loggerFactory)
        {
            _loggerFactory = loggerFactory;
        }
        public void ConfigureServices(IServiceCollection services)
        {
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            _loggerFactory.AddSerilog();
            Serilog.Debugging.SelfLog.Enable(Console.Error);

            app.Run(async (context) =>
            {
                var logger = _loggerFactory.CreateLogger<Startup>();
                logger.LogInformation("Information message");
                await context.Response.WriteAsync("Hello World!");
            });
        }
    }
{
  "ConnectionStrings": {
    "DevTest": "Data Source=.\\sqlexpress;Database=SerilogTest;Integrated Security=true"
  },
  "Serilog": {
    "Using": [ "Serilog.Sinks.Console" ],
    "WriteTo": [
      { "Name": "Console" },
      {
        "Name": "MSSqlServer",
        "Args": {
          "connectionString": "DevTest",
          "tableName": "Issue81",
          "autoCreateSqlTable": true,
          "columnOptionsSection": {
            "customColumns": [
              {
                "ColumnName": "EventType",
                "DataType": "int"
              },
              {
                "ColumnName": "Release",
                "DataType": "varchar",
                "DataLength": 32
              }
            ]
          }
        }
      }
    ]
  }
}

If I replace "DevTest" with the actual connection string, it works, otherwise i get

WebApplication1> 2018-10-24T18:15:28.3438609Z Unable to write 3 log events to the database due to following error: Format of the initialization string does not conform to specification starting at index 0.

Only packages i have installed :

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore" Version="2.1.4" />
    <PackageReference Include="Serilog.AspNetCore" Version="2.1.1" />
    <PackageReference Include="Serilog.Settings.Configuration" Version="3.0.1" />
    <PackageReference Include="Serilog.Sinks.Console" Version="3.1.1" />
    <PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.2" />
  </ItemGroup>
OculiViridi commented 5 years ago

@nlz242 Did you tried using the dev package of Serilog.Sinks.MSSqlServer v5.1.3?

nlz242 commented 5 years ago

@OculiViridi I did and it crashes (NullReferenceException) on

    .UseSerilog((hostingContext, loggerConfiguration) =>
    {
        loggerConfiguration.ReadFrom.Configuration(hostingContext.Configuration);
    });

Stacktrace :

   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Serilog.Settings.Configuration.ConfigurationReader.CallConfigurationMethods(ILookup`2 methods, IList`1 configurationMethods, Object receiver, IReadOnlyDictionary`2 declaredLevelSwitches)
   at Serilog.Settings.Configuration.ConfigurationReader.ApplySinks(LoggerConfiguration loggerConfiguration, IReadOnlyDictionary`2 declaredLevelSwitches)
   at Serilog.Settings.Configuration.ConfigurationReader.Configure(LoggerConfiguration loggerConfiguration)
   at Serilog.Configuration.LoggerSettingsConfiguration.Settings(ILoggerSettings settings)
   at WebApplication1.Program.<>c.<CreateWebHostBuilder>b__1_0(WebHostBuilderContext hostingContext, LoggerConfiguration loggerConfiguration) in C:\Users\user\source\repos\WebApplication1\Program.cs:line 19
   at Serilog.SerilogWebHostBuilderExtensions.<>c__DisplayClass1_0.<UseSerilog>b__0(WebHostBuilderContext context, IServiceCollection collection)
   at Microsoft.AspNetCore.Hosting.WebHostBuilder.BuildCommonServices(AggregateException& hostingStartupErrors)
   at Microsoft.AspNetCore.Hosting.WebHostBuilder.Build()
   at WebApplication1.Program.Main(String[] args) in C:\Users\user\source\repos\WebApplication1\Program.cs:line 11

This happens with 5.1.3 prerelease 00231 & 00229. Up until 00226, it doesn't crash but i do get the same message : WebApplication1> 2018-10-25T12:21:05.5956941Z Unable to write 3 log events to the database due to following error: Format of the initialization string does not conform to specification starting at index 0.

OculiViridi commented 5 years ago

@nlz242 Like you said previously:

If I replace "DevTest" with the actual connection string, it works, otherwise i get

WebApplication1> 2018-10-24T18:15:28.3438609Z Unable to write 3 log events to the database due to following error: Format of the initialization string does not conform to specification starting at index 0.

Are you still using this?

"Args": {
          "connectionString": "DevTest",

I'm currently running without problems Serilog on Console and SQL Server 2016, using the following packages:

Maybe these links can also be useful. Please try to take a look:

nlz242 commented 5 years ago

Yes still using "DevTest" as the connection string, with a connection string named DevTest defined in the connectionstrings section.

I debugged a bit just to see if i could figure it out. It seems that the static ConfigurationManager just doesn't find the connection string. Which would make sense since this is an aspnet core project (running on full 4.7 framework). Looking at it, and correct me if im wrong, i think that for some reason, the wrong LoggerConfigurationMSSqlServerExtensions is picked up. Even tho i'm running full framework, it should still use LoggerConfigurationMSSqlServerExtensions found inside the Microsoft.Extensions.Configuration folder, but it uses the one found in NetFramework.ConfigurationManager, which relies on ConfigurationManager instead of IConfiguration.

nlz242 commented 5 years ago

Upon further investigation, it seems like this is a Nuget "issue". Nuget pickups the net45 assembly instead of the netstandard2.0 one, since the project is .NET Framework. I'll try choosing my DLL manually and report back with my findings.

Related to https://github.com/NuGet/Home/issues/7385

nlz242 commented 5 years ago

Referencing the netstandard dll directly, instead of the nuget packages, i was able to get the proper LoggerConfigurationMSSqlServerExtensions to run, however i was never able to actually get IConfiguration injection to work, even when creating the logger by hand:

        private static IConfiguration Configuration { get; set;  } 
        public static void Main(string[] args)
        {
            Configuration = new ConfigurationBuilder()
                .SetBasePath(Directory.GetCurrentDirectory())
                .AddCommandLine(args)
                .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
                .AddEnvironmentVariables().Build();

            Log.Logger = new LoggerConfiguration()
                .ReadFrom.Configuration(Configuration)
                .Enrich.FromLogContext()
                .WriteTo.Console()
            .CreateLogger();

            CreateWebHostBuilder(args).Build().Run();
        }

I always end up with a null IConfiguration so obviously, getting the connection string fails and returns a null.

I guess i'm gonna stick with providing the connection string directly in MSSqlServer's Args instead of trying to get Named connection string to work.

I've included my repro project.

I'd also like to mention that after my tests, i am VERY worried about the upcomming version. Current version (5.1.2) works properly with ASP.NET Core projects targetting Full frameworks, except for named connecting strings, however 5.1.3 previews stop working completly. You can use my repro to see it for yourself, just upgrading 5.1.2 to 5.1.3-dev-00231 breaks the application, Creating the logger is no longer possible.

WebApplication1.zip

nlz242 commented 5 years ago

Good news is, using @MV10 issue_161 branch, named connection string still don't work but at least ASPNET Core with Full Framework (4.7) works as expected. Less worried now.

MV10 commented 5 years ago

@nlz242 I'm merging PR #170 (fixes #161) now, a new dev build should be available shortly.

nelsonghezzi commented 5 years ago

Hi, I just hit this issue moving the sink configuration to the appsettings.json.

ASP.NET Core 2.1 project targeting .NET Core here (<TargetFramework> set to netcoreapp2.1).

Any chance to have a fix soon?

Thanks in advance.

craigmoliver commented 5 years ago

I am having the issue with .NET Core 2.1. I version 2.1.3 but it breaks the application completely with a named Connection String. Not using a named connection string works fine.

Error with named Connection String:

image

Appsettings.json

image

Hoping for a fix soon. Thanks all.

OculiViridi commented 5 years ago

@craigmoliver Please indicate which versions of the various Serilog packages you are using.

craigmoliver commented 5 years ago

I've since reverted to Serilog.Sinks.MSSqlServer 5.1.2. Otherwise all these:

image

EEParker commented 5 years ago

Edit: with further testing, I cannot get any version higher than Serilog.Settings.Configuration v3.0.0 to 3.0.1-dev-00160 work properly. .ReadFrom.Configuration(Configuration) does not seem to work with named connection strings from v3.0.1-dev-00163+.

I have had some success using name connection strings with the following: Serilog.Settings.Configuration v3.0.0 - 3.0.1-dev-00160

    Log.Logger = new LoggerConfiguration()
        .ReadFrom.Configuration(GetFileConfiguration())
        .CreateLogger();

Note, I am manually loading Configuration from a method

private static IConfigurationRoot GetFileConfiguration()
{
    return new ConfigurationBuilder()
        .SetBasePath(Directory.GetCurrentDirectory())
        .AddJsonFile("appsettings.json", optional: true, reloadOnChange: false)
        .AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")}.json", optional: true, reloadOnChange: false)
        .AddEnvironmentVariables()
        .Build();
}

Package References

    <PackageReference Include="Serilog.Settings.Configuration" Version="3.0.1-dev-00160" />
    <PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.3-dev-00232" />
craigmoliver commented 5 years ago

Does that mean lesser version of Serilog.Settings.Configuration 3.0.1-dev-00160 work? What's the difference between them?

angelcalvasp commented 5 years ago

I was having the same problem, but @EEParker's workaround does the job on an asp.net core 2.1 project

craigmoliver commented 5 years ago

I got it working with these versions:

<PackageReference Include="Serilog.Settings.Configuration" Version="3.0.1-dev-00160" />
<PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.3-dev-00232" />

It also works with these versions:

<PackageReference Include="Serilog.Settings.Configuration" Version="3.0.2-dev-00187" />
<PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.3-dev-00232" />

Interestingly these versions DO NOT work. Strange how the release version fails.

<PackageReference Include="Serilog.Settings.Configuration" Version="3.0.1" />
<PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.3-dev-00232" />
nlz242 commented 5 years ago

I can confirm @craigmoliver findings. I was able to get it working with these versions, in a brand new netcoreapp2.2 project:

<PackageReference Include="Serilog.Settings.Configuration" Version="3.0.2-dev-00187" />
<PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.3-dev-00232" />

I can also add that the following combination does NOT work :

<PackageReference Include="Serilog.Settings.Configuration" Version="3.0.2-dev-00187" />
<PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.2" />
johndcollins commented 5 years ago

I'm also having issues. Version 5.1.2 doesn't throw the error, but it also doesn't work. Version 5.1.3-dev-00232 throws the error below.

I'm hosting in Azure and using the Azure AppSettings and when I try to run the website I get a null exception on the connection string. If I hard code the connection string in the appsettings.json it works fine, but if I reference a connection string it doesn't work.

"Application startup exception: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentNullException: Value cannot be null.

Parameter name: connectionString

at Serilog.Sinks.MSSqlServe

r.MSSqlServerSinkTraits..ctor(String connectionString, String tableName, String schemaName, ColumnOptions columnOptions, IFormatProvider formatProvider, Boolean autoCreateSqlTable)

at Serilog.LoggerConfigurationMSSqlServerExtensions.MSSqlServer(LoggerSinkConfiguration loggerConfiguration, String connectionString, String tableName, IConfiguration appConfiguration, LogEventLevel restrictedToMinimumLevel, Int32 batchPostingLimit, Nullable`1 period, IFormatProvider formatProvider, Boolean autoCreateSqlTable, ColumnOptions columnOptions, IConfigurationSection columnOptionsSection, String schemaName)

--- End of inner exception stack trace ---

at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)

at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

at Serilog.Settings.Configuration.ConfigurationReader.CallConfigurationMethods(ILookup2 methods, IList1 configurationMethods, Object receiver, IReadOnlyDictionary`2 declaredLevelSwitches)

at Serilog.Settings.Configuration.ConfigurationReader.ApplySinks(LoggerConfiguration loggerConfiguration, IReadOnlyDictionary`2 declaredLevelSwitches)

at Serilog.Settings.Configuration.ConfigurationReader.Configure(LoggerConfiguration loggerConfiguration)

at Serilog.Configuration.LoggerSettingsConfiguration.Settings(ILoggerSettings settings)

at Serilog.ConfigurationLoggerConfigurationExtensions.Configuration(LoggerSettingsConfiguration settingConfiguration, IConfiguration configuration, DependencyContext dependencyContext)

at StartupHelpers.AddLogging(IApplicationBuilder app, ILoggerFactory loggerFactory, IConfiguration configuration) in IdentityServer4.STS.Identity\Helpers\StartupHelpers.cs:line 118

at Startup.Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory) in IdentityServer4.STS.Identity\Startup.cs:line 45

--- End of stack trace from previous location where exception was thrown ---

at Microsoft.AspNetCore.Hosting.ConventionBasedStartup.Configure(IApplicationBuilder app)

at Microsoft.AspNetCore.Mvc.Internal.MiddlewareFilterBuilderStartupFilter.<>c__DisplayClass0_0.g__MiddlewareFilterBuilder|0(IApplicationBuilder builder)

at Microsoft.AspNetCore.HostFilteringStartupFilter.<>c__DisplayClass0_0.b__0(IApplicationBuilder app)

at Microsoft.AspNetCore.Hosting.Internal.AutoRequestServicesStartupFilter.<>c__DisplayClass0_0.b__0(IApplicationBuilder builder)

at Microsoft.AspNetCore.Hosting.Internal.WebHost.BuildApplication()"

WahidBitar commented 5 years ago

From almost one year till now this issue still open! O_O That encourages staying with NLog as it's more mature.

codehunter13 commented 4 years ago

still open??? Time to change to something else....

nlz242 commented 4 years ago

Haven't tried in ASP.NET Core Targetting full framework (472 or 48) lately, but ASP.NET Core on .NET Core works as expected.

Versions tested : <PackageReference Include="Serilog.Settings.Configuration" Version="3.1.0" /> <PackageReference Include="Serilog.Sinks.MSSqlServer" Version="5.1.3" />

So this seems all good to me. Maybe someone else can chime in and say if they are still encountering this issue.

ckadluba commented 4 years ago

I verified it too with the current version Serilog.Sinks.MSSqlServer 5.5.0 and Serilog.Settings.Configuration version 3.1.0. I used the sample program in samples/WorkerServiceDemo which targets .NET Core 3.1. and adapted the configuration in appsettings.json so that it uses named connection strings for WriteTo and AuditTo configs. Everything worked as expected without problems.

Closing this issue now.

MrBjorn commented 7 months ago

Hi, does Serilog.Sinks.MSSqlServer have the feauture with named connection strings? What was the final word on this.

ckadluba commented 7 months ago

Hi @MrBjorn,

Judging from my last comment above (which is some years back so I cannot remember details :)) it is supported and works as expected.

MrBjorn commented 7 months ago

Thanks @ckadluba ! :-) I verified it in latest version and in .NET8. Can simplify the json's a bit.