Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Feature/7.x support #256

Merged
merged 16 commits into from
Jul 15, 2019
Merged

Conversation

Mpdreamz
Copy link
Contributor

@Mpdreamz Mpdreamz commented Jul 8, 2019

What issue does this PR address?

  • Adds 7.0 support (continuation of @kanadaj's PR ElasicSearch 7.0 support #248

  • Adds DetectElasticsearchVersion to the sink that will detect the running cluster version. Something we now use to support sending Esv6 templates to Elasticsearch 7.x and Esv7 templates to Elasticsearch 6.x which should simplify upgrades.

Adds an integration test project

  • Spins up a 6.x and 7.x elasticsearch single node cluster in succession and asserts the default and the mixed mode through DetectElasticsearchVersion works.

Does this PR introduce a breaking change?

The base TFM is now netstandard2.0 and net461.

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

@Mpdreamz Mpdreamz requested a review from mivano July 8, 2019 13:13
@Mpdreamz
Copy link
Contributor Author

Mpdreamz commented Jul 8, 2019

Note this PR deletes some Durable files that were duplicate by casing only, it does not remove the Durable feature.

Copy link
Contributor

@mivano mivano left a comment

Choose a reason for hiding this comment

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

Great work!

@Mpdreamz
Copy link
Contributor Author

Updated the PR @mivano thanks for the review.

I am on holiday for 2 weeks so won't be able to merge this in if all good so please feel free to do so!

@mivano
Copy link
Contributor

mivano commented Jul 15, 2019

Thanks @Mpdreamz and have a good holiday. I will merge it to dev so we have some test packages.

@mivano mivano merged commit c35e764 into serilog-contrib:dev Jul 15, 2019
@ghost
Copy link

ghost commented Jul 15, 2019

@mivano @Mpdreamz Any idea when a stable v7.2 of this NuGet will be deployed with these changes?

@mivano
Copy link
Contributor

mivano commented Jul 15, 2019

I pushed an alpha package to nuget so we can gather some feedback: https://www.nuget.org/packages/Serilog.Sinks.ElasticSearch/8.0.0-alpha0025
I rather not merge it to master just yet, certainly not with the holidays upcoming.

@kanadaj
Copy link
Contributor

kanadaj commented Jul 15, 2019

@mivano wouldn't it be better to try to match versions of the Sink to versions of the SDK and/or Elasticsearch itself? Just wondering.

@mivano
Copy link
Contributor

mivano commented Jul 15, 2019

It is breaking enough that it warrants a major version bump. The 7 or 8 does not also match the ES product version. It still has support for 6 for example.

@mivano
Copy link
Contributor

mivano commented Jul 30, 2019

Version 8 is now published: https://www.nuget.org/packages/Serilog.Sinks.ElasticSearch/8.0.0

@Mpdreamz Mpdreamz deleted the feature/7.x-support branch July 31, 2019 13:55
@hypertrends
Copy link

hypertrends commented Aug 19, 2019

Good Afternoon, I am trying to connect to Elastic Search using the latest 8.0.0 and I don't see any log entries in my Elastic Search. Is there a different configuration I have to do? Here's what my code looks like for Service Fabric w/ Serilog:

Log.Logger = new LoggerConfiguration() .WriteTo.ApplicationInsights(appInsightsKey, TelemetryConverter.Events) .WriteTo.Elasticsearch( new ElasticsearchSinkOptions( new Uri(elasticUrl)) { AutoRegisterTemplate = true, AutoRegisterTemplateVersion = AutoRegisterTemplateVersion.ESv7, //BufferBaseFilename = "./logs/buffer", IndexFormat = index, FailureCallback = e => { Console.WriteLine("Unable to submit event " + e.Exception); }, EmitEventFailure = EmitEventFailureHandling.WriteToSelfLog | EmitEventFailureHandling.WriteToFailureSink | EmitEventFailureHandling.RaiseCallback, ModifyConnectionSettings = x => x.BasicAuthentication(elasticUsername, elasticPassword) }) .CreateLogger();

The index is of a simple 'test-{0:yyyy.MM}' format. I have commented out the BufferBaseFileName in the snippet above but uncommenting it doesn't make any changes.

What's interesting is that I have an ASP.NET Core project that does this just fine so I know it isn't a v7 issue. I read somewhere in an issues list that only public facing IP systems are able to make logs? Please also note that there are 5 instances of the same service in Service Fabric. Could that be affecting things?

Would love to get some feedback. I've spent quite a bit of time trying to figure this out.

@staticdev
Copy link

staticdev commented Aug 21, 2019

@hypertrends I had the same problem, I have to configure:
AutoRegisterTemplate = true,
AutoRegisterTemplateVersion = AutoRegisterTemplateVersion.ESv7
Also check your log levels.

@hypertrends
Copy link

Hello @staticdev - let me check that. As you can see in my snippets, I definitely have that information set right. I will review my log levels to see what's up.

@SamilMehdiyev
Copy link

@staticdev , @hypertrends

I had the same problem. After few hours investigation and debugging, I founded that Assembly Version of Serilog.Sinks.Elasticsearch was old. I am using 8.0.0 version, but in file displayed 6.0.0.

To fix this problem, I did the followings:

  1. Clone the repository
  2. Open it in MS Visual Studio
  3. Changed version from 6.0.0 to 8.0.0 in AssemblyInfo.cs file
  4. Build the project
  5. Use new dll in my own project instead of package which downloaded from NuGet

Hi @mivano , I would like to push my change in dev brach. Or Kindly ask you to update assembly version and publish new version, so we can use it from NuGet.

Thanks, hope it will help.

@hypertrends
Copy link

Good Morning @SamilMehdiyev - wow, I hadn't even thought about that. Looking at it right now :) Thank you so much!

@bbqchickenrobot
Copy link

Getting smae issue as @hypertrends - were you able to resolve this issue?

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

Successfully merging this pull request may close these issues.

7 participants