7.0.0-alpha2 breaking formatter backwards compatibility

[kanadaj]

ElasticLowLevelClient.Serializer.SerializeToString()

introduced a new IMemoryStreamFactory parameter in the middle of the parameter chain - this means that libraries compiled against ElasticSearch.Net 6.X don't work with references to 7.0; this includes libraries installed from NuGet. The AssemblyRedirects generated will try to call SerializeToString() with parameters (data, format) instead of (data, null, format) throwing an error.

It'd be nice getting a SerializeToString() implementation without the IMemoryStreamFactory parameter for backwards compatibility so that existing libraries don't break.

[russcam]

The development philosophy adopted for the client is to maintain binary backwards compatibility within a major version of the client, but not necessarily across major versions. A new major version of the client is an opportunity to address bugs and issues uncovered in a previous major version, and to make API changes that improve the usability and performance of the client going forward.

A considerable effort has been invested into the 7.x client so far to improve performance across the board and reduce the number of allocations. In making these kinds of changes, hopefully it's understandable that some of the public API surface may inevitably change too. In this particular example, an IMemoryStreamFactoryparameter has been introduced to allow the serialization method to reuse a memory stream backed by a shared pool of byte buffers.

Once 7.x client is GA released to Nuget, binary backwards compatibility will be maintained for it.

Do you have the name of the dependency compiled against Nest 6.x that you're looking to use?

[Mpdreamz]

This relates to: serilog-contrib/serilog-sinks-elasticsearch#249

@kanadaj we are looking to take a more active involvement in the sink again and are meeting @mivano next week. Thank you for that PR by the way, its something we have on the backlog to do in the coming weeks!

Some special care will need to be taken to ensure the sink works against 6.x Elasticsearch using the 7.x client and the sink will need to be stricter about not allowing major version bumps since these type of breakages will happen on major version changes.

Ideally we update the sinks dependency to 7.0.0 across the board and make sure all the TFM's are available to so. I am not sure netstandard1.3 needs to be carried forward but it'd be great to get some wider feedback on that.

[kanadaj]

@russcam I've hit this issue with both the serilog sink (6.X) and my own code using the low level library, and I believe any other library depending on the low-level client's serializer will hit this issue.

Introducing a parameter into the middle of the parameter chain makes it so assembly rebindings don't work on the method call anymore as assembly rebindings try link to a method call with identical parameter order - which doesn't exist anymore.

I believe it'd be better to put the memory stream at the end of the parameter list so it won't break older NuGet libraries. That, or introduce and [Obsolete] compatibility shim.

[ffMathy]

@Mpdreamz sounds great! Can't wait for 7.0 support!

[kanadaj]

@Mpdreamz Happy to help, Elasticsearch really does a lot for us as a free tool (I really want the paid features like alerts and the other machine learning goodies but we simply can't afford it, it's too steep :'( ) so I'm happy to contribute where I can.

Personally, I don't think netstandard1.3 needs to be carried forward - it's fairly simple to migrate from Core 1.X to Core 2.2 for any ASP.NET Core app (and let's be fair, Core 1.X wasn't really good for much else, and even then it lacked a lot of features, so I imagine almost all web apps are already on 2.X).

[ffMathy]

I agree with @kanadaj on his assessment.

[russcam]

I've opened #3809 to address

[russcam]

Closing this as #3809 has been merged into 7.x and will be in the next release