serilog-sinks-elasticsearch icon indicating copy to clipboard operation
serilog-sinks-elasticsearch copied to clipboard

Example in README is incorrect

Open Alexgolshtein opened this issue 3 years ago • 15 comments
trafficstars

".WriteTo.Elasticsearch(new ElasticsearchSinkOptions(new Uri("http://localhost:9200"))" .........

Alexgolshtein avatar Nov 29 '21 23:11 Alexgolshtein

Done

mivano avatar Jan 25 '23 10:01 mivano

@mivano That was fast! :) Is there anything else that is important to 9.0 release that needs to be fixed/finalized?

nenadvicentic avatar Jan 25 '23 10:01 nenadvicentic

Not that I m aware of. I normally let people use the beta packages for some while to see if there are no issues and then I merge to main to get an official release package. did you run with the latest prerelease package already?

mivano avatar Jan 25 '23 10:01 mivano

So far, I've been only running with source-code directly on dev machine. Just checked NuGet now, I will try <PackageReference Include="Serilog.Sinks.ElasticSearch" Version="9.0.0-beta7" /> on two different projects (net7.0 and net48):

  • Against Elasticserach 7.x on test environment and let you know if it works well.
  • Will do smoke-test against local Elasticsearch v8.x (on my machine, with little load).

Give me a day to test everything properly.

nenadvicentic avatar Jan 25 '23 10:01 nenadvicentic

@mivano 9.0.0-beta7 on the official NuGet feed is 9 months old: https://www.nuget.org/packages/serilog.sinks.elasticsearch/#versions-body-tab

Can you create/publish new beta and I am going to test it afterwards?

nenadvicentic avatar Jan 25 '23 10:01 nenadvicentic

The GitHub package registry has the latest. Although the beta should have been published to Nuget as well. I will have a look why that failed.

mivano avatar Jan 25 '23 10:01 mivano

For me would be simpler to have package on NuGet, but let me know if that is an issue. Looking at the Git log, perhaps you have to add new tag (e.g v9.0.0-beta8) to trigger publishing to NuGet?

nenadvicentic avatar Jan 25 '23 10:01 nenadvicentic

I will make sure it gets added to Nuget. That is how it is suppose to work. I hope to have some time in the afternoon to look into it. Verstuurd vanaf mijn iPhoneOp 25 jan. 2023 om 11:42 heeft Nenad Vićentić @.***> het volgende geschreven: For me would be simpler to have package on NuGet, but let me know if that is an issue. Looking at the Git log, perhaps you have to add new tag (e.g v9.0.0-beta8) to trigger publishing to NuGet?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

mivano avatar Jan 25 '23 11:01 mivano

There is a release and a nuget package: https://www.nuget.org/packages/Serilog.Sinks.Elasticsearch/9.0.0-beta8

mivano avatar Jan 25 '23 13:01 mivano

I have tried to upgrade the sink from version 8.4.1 to 9.0.0-beta8 in .NET Framework web project. When I try to start the web I get the following error:

Could not load file or assembly 'Serilog.Formatting.Elasticsearch' or one of its dependencies. Strong name signature could not be verified. The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045)

Is this something related with beta release? Or perhaps signing of the dll itself?

nenadvicentic avatar Jan 25 '23 15:01 nenadvicentic

Hmm we do not sign anything. So wonder why it would complain about that. I saw a similar message in the discussions. Will need to look into it.

mivano avatar Jan 25 '23 15:01 mivano

I checked a bit more and notices a few things.

There is a configuration to sign both packages (Serilog.Sinks.Elasticsearch and Serilog.Formatting.Elasticsearch): image

Settings can be seen in *.csproj XML as well:

    <AssemblyOriginatorKeyFile>../../assets/Serilog.snk</AssemblyOriginatorKeyFile>
    <SignAssembly>true</SignAssembly>

Next, I inspected all 4 NuGet packages, both versions 8.4.1 and 9.0.0-beta8. All NuGet packages have valid signature and all 4 dlls have strong name. NuGet package versions are also OK (two packages 9.0.0-beta8 and two 8.4.1)

However, versioning of the dlls inside the NuGet package does not look good. For Serilog.Sinks.Elasticsearch:

  • DLL inside of 8.4.1 has version of 8.4.1.0
  • DLL inside of 9.0.0-beta8 has version of 6.0.0.0 (which is hard-coded in AssemlyInfo.cs file).

For Serilog.Formatting.Elasticsearch:

  • DLL inside of 8.4.1 has version of 0.0.0.0, targeting net451, netstandard1.3 and netstandard2.0
  • DLL inside of 9.0.0-beta8 has version of 0.0.0.0, targeting netstandard2.0 only.

Most likely, the issue shows up in .NET Framework projects, because project has Serilog.Formatting.Elasticsearch DLL version 0.0.0.0 that targets net451 that belongs to 8.4.1 NuGet package and after the update to NuGet package 9.0.0-beta8 assembly-binder expects "the same" Serilog.Formatting.Elasticsearch DLL version 0.0.0.0, but new version is supposed to be netstandard2.0.

I suggest, if it's possible, to overwrite DLL version number on every build (perhaps using result from GitVersion?), so that for every new NuGet package version, we get assembly version that fits the package version. This way we avoid issues with cached dlls, wrong binding, etc. Something along the lines:

dotnet build build -c Release -p:Version="9.0.0" -p:InformationalVersion="9.0.0-beta9"

Bellow I provided screenshots from NuGet Package Explorer for all 4 dlls.

Serilog.Sinks.Elasticsearch 9.0.0-beta8 package: image

Serilog.Formatting.Elasticsearch 9.0.0-beta8 package: image

Serilog.Sinks.Elasticsearch 8.4.1 package: image

Serilog.Formatting.Elasticsearch 8.4.1 package:: image

nenadvicentic avatar Jan 26 '23 10:01 nenadvicentic

Thanks for your investigation. Looks like the version stamping goes wrong indeed.

That NuGet packaging changed in the last couple of years, so I have to find out how to pass the right version along.

mivano avatar Jan 26 '23 10:01 mivano

@mivano Thank you for the support.

The example of the dotnet build command I gave you would work. Only to replace versions I "hard coded" with proper build server variables. For -p:InformationalVersion it could be exact same variable that creates NuGet package suffix.

P.S. Do you prefer to open a new issue for this topic, instead hijacking this "ReadMe.md" issue for the conversation?

nenadvicentic avatar Jan 26 '23 10:01 nenadvicentic

Added new issue #498

nenadvicentic avatar Jan 26 '23 14:01 nenadvicentic