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

Support for Always Encrypted - Microsoft.Data.SqlClient #208

Closed
Mich-b opened this issue Jul 17, 2019 · 12 comments · Fixed by #287
Closed

Support for Always Encrypted - Microsoft.Data.SqlClient #208

Mich-b opened this issue Jul 17, 2019 · 12 comments · Fixed by #287

Comments

@Mich-b
Copy link

Mich-b commented Jul 17, 2019

It seems this Serilog sink depends on System.Data.SqlClient, which is not compatible with Always Encrypted as described in dotnet/SqlClient#11 .

Are there plans to migrate to Microsoft.Data.SqlClient in the near future, since that seems to have support for Always Encrypted (dotnet/SqlClient#11 (comment))?

@mivano
Copy link
Contributor

mivano commented Oct 11, 2019

No real plans, but open for a PR of course!

@Slavuta851109
Copy link

Slavuta851109 commented Jul 7, 2020

Looks like this is the reason for exception:
Exception thrown: 'System.PlatformNotSupportedException' in Microsoft.Data.SqlClient.dll
2020-07-07T16:01:05.1680936Z Unable to write 1 log events to the database due to following error: Microsoft.Data.SqlClient is not supported on this platform.

My .net standard 2.0 lib + serilog used as reference for web api with configuration like this:
<system.web>
compilation debug="true" targetFramework="4.7.2"
httpRuntime targetFramework="4.5"
</system.web>

@ckadluba
Copy link
Member

ckadluba commented Jul 7, 2020

Thanks for the feedback. Which framework is your executable assembly targeting?

@Slavuta851109
Copy link

Updated my prev comment by removing html braces, so it is visible now.

@ckadluba
Copy link
Member

ckadluba commented Jul 7, 2020

Thanks! Can you supply a minimal version of your solution to reproduce the problem?

@Slavuta851109
Copy link

Slavuta851109 commented Jul 10, 2020

https://github.com/Slavuta851109/SerilogBugOn452
To reproduce simply run web host.
Currently I decided to use working v.5.3.0 in prod.
Thank you for checking!

@ckadluba
Copy link
Member

@Slavuta851109 Thanks for the sample. I was able to reproduce your problem but have not yet had enough time to investigate the reason. I will continue looking into it as soon as I can and keep you posted.

@ckadluba
Copy link
Member

@Slavuta851109 It seems due to an open issue in Microsoft.Data.SqlClient (see my mention above).

Currently there is no information about when or if this will be fixed in the future. I asked in their github issue for an update but the last comments were from late 2019, which does not give me much hope to see a fix soon.

Therefore I would like to revert the sink back to System.Data.SqlClient for .NET Standard. This means that the specific features of Microsoft.Data.SqlClient (like Always Encrypted) can only be used when targeting .NET Core or .NET Framework but not when using the sink in your .NET Standard library. But to me this seems to be less of a problem than to make the sink unusable from a .NET Standard library at all due to the exception.

@ckadluba
Copy link
Member

After some further testing I found that this problem seems to only occurr when using the sink in a .NET Standard lib that is called from a .NET Framework app. It seems not to occur when the .NET Standard lib is called from a .NET Core app (see sample/NetStandardDemo).

Sink users with .NET Standard scenarios, can you please state if my overvations are correct?

I believe there are not as many users using the sink in the affected scenario while on the other hand there are more users using the sink in the working .NET Standard + .NET Core scenario. If we would now revert the sink for .NET Standard back to System.Data.SqlClient, this would probably take features away from many while solving a problem only for a few. So I would rather leave the code that way and hope the SqlClient team comes up with a fix soon. In the meantime, if you are on .NET Framework and using this sink, do not call it from a .NET Standard library.

@ckadluba
Copy link
Member

@Slavuta851109 Good news! I could now understand and fix the problem using the sample app you provided. I was then able to generate log data with the sample app. Here is the proof:

image

The problem seems to be the same as described here: #283 (comment)

There seems to be a problem with Microsoft.Data.SqlClient when it is used in a classlib (e.g. Serilog MSSqlServer sink) and this classlib is then used in a .NET Framework application.

So, to fix the issue I had to add a reference to the NuGet package Microsoft.Data.SqlClient to the .NET Framework app (WebApplication1 in your sample). I'm afraid there is not much we can do about this in the sink but at least we have a workaround.

@duncancoppedge
Copy link

I'm not expecting anything out of this, just throwing it out there for other users to google and hopefully save a few hours.
Issue we had Serilog fails "silently" and nothing is logged in our db.
Workaround is roll back to v5.4 before Microsoft.Data.SqlClient was implemented or fix our server certificates.

details:
We've integrated serilog with our .net framework 4.72 application.
Issue is when using an IP address in our connectionstring against a server without a certificate (I know).. serilog doesn't write.
image

Pulled in source (thanks serilog open source 👍, also oops we didn't know about SelfLog.Enable()) and found it failing with this error:
A connection was successfully established with the server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - The certificate chain was issued by an authority that is not trusted.)

serilog-sinks-mssqlserver\src\Serilog.Sinks.MSSqlServer\Sinks\MSSqlServer\Platform\SqlBulkBatchWriter.cs:line 43
Here:
image

@ckadluba
Copy link
Member

Thank you for the research and documentation of this.

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

Successfully merging a pull request may close this issue.

5 participants