Make Serilog.Sinks.Elasticsearch version 9.0.0 work with Elasticsearch version <7, 7 and 8 by default, without need to specify TypeName

[nenadvicentic]

This is a suggestion for a new feature in version 9. Simplification of the way Serilog.Sinks.Elasticserach handles different version of Elasticsearch backend.

If TypeName parameter has not been explicitly specified in code or config, handle _type parameter sent to a different versions of Elasticsearch automatically! That means:

• Set _type: "logevent" for <v7,
• Set _type: "_doc" for v7
• Omit _type for v8 (or higher).

We could do this by:

1. Not setting any default ElasticsearchSinkOptions.TypeName during the Sink initialization in the constructors (so the value keep being null).
2. Modifying DiscoverClusterVersion method like shown bellow. Method could be triggered by exposing "detectElasticsearchVersion": true configuration option, since currently DetectElasticsearchVersion is not used at all (DiscoverClusterVersion() method never goes through). Alternatively, more intuitive option name could be used, like automaticallyResolveTypeName.

public void DiscoverClusterVersion()
{
    if (!_options.DetectElasticsearchVersion) return;
    if (_options.TypeName != null) return; // Do not do anything if user explicitly configured `TypeName`

    try
    {
        var response = _client.Cat.Nodes<StringResponse>(new CatNodesRequestParameters()
        {
            Headers = new[] { "v" }
        });
        if (!response.Success) return;

        _discoveredVersion = response.Body.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)
            .FirstOrDefault();

        if (_discoveredVersion == null)
            return;

        if (int.TryParse(_discoveredVersion.Substring(0, _discoveredVersion.IndexOf('.')), out int majorVersion))
        {
            if (majorVersion < 7)
                _options.TypeName = "logevent"; // maybe this should be `ElasticsearchSinkOptions.DefaultTypeName`
            if (majorVersion == 7)
                _options.TypeName = "_doc";
            // Version 8 or higher leave `_options.TypeName` to be `null` (condition is already at the start of the method), so that `_type` is not sent to Elasticsearch at all.
        }
    }
    catch (Exception ex)
    {
        SelfLog.WriteLine("Failed to discover the cluster version. {0}", ex);
    }
}

Perhaps ElasticsearchSinkOptions.DefaultTypeName should be reverted back to "logevent", from "_doc" and used, as described in the summary: "The default Elasticsearch type name used for Elasticsearch versions prior to 7."

[nenadvicentic]

I tried this approach with my fork. This particular commit.

It works for my use-case (tested against Elasticsearch 5.6.16, 7.17.5 and 8.3.3). Not sure if it creates problems for other use-cases.

[jonasmidstrup]

@nenadvicentic Had the same idea to start implementing this, but you beat me to it by a few minutes 😆

Can we start a PR here with your changes and get it reviewed?

Another thing is that we need an auto registered template that works for Elasticsearch v8.

[nenadvicentic]

@jonasmidstrup Yes, I can make PR out of this. However, I think I would need someone who has a bit more context and historical knowledge of the project.

For example, I'm not sure what kind of auto registered template is needed. It seems that v7 template works fine against Elasticsearch v8.

Also DiscoverClusterVersion() method that I used was not possible to set to true by configuration. No clue why.

[jonasmidstrup]

We should take the discussion in the PR 😃 and maybe involve @mivano for context and historical knowledge.

When I try to use the auto-registered template, it shows up in Kibana under "Index Management" as a "Legacy index template". So I would guess that something needs to be done here as well.

[nenadvicentic]

@jonasmidstrup I agree! There is the PR request #462

[mivano]

I m too far out on ES and the specifics for the needed options. I do know indeed that the current way is not optimal. We also strived for zero config (so pointing to ES cluster should have been enough to start logging). Discovering version and applying default settings should help with that.

Elastic is moving forward with new versions and it would be strange not to keep up the pace with this sink as well. If you have a nice PR, are able to test and validate it, then I m more than happy to get it in! Breaking changes are okay, as long as we document and version.

[nenadvicentic]

@mivano PR is there #462 , and it's already capable to automatically handle different versions, but it requires option: options.DetectElasticsearchVersion to be set to true explicitly.

It does seem like good idea to have version-discovery + TypeName handling + Template version done automatically. Perhaps if nothing but the nodeUris setting is configured?

I can make some of those improvements, but there are two areas where I would need help.

• index_template for Elasticsearch v8, since I am completely unfamiliar with those.
• testing, I would need some guidance what you wanted me to test and how. The way current implementation of tests and integration tests in the project is structured is not what I'm used to. Seems hard to be extended and hard to add tests. Seems that there is no way to fake to subsequent Elasticsearch response-streams (e.g. version response 1st, then subsequent request/response)

Maybe take a look at the PR and we can discuss how to extend/finalize it.

[Schoof-T]

Any progress on this?

[nenadvicentic]

Any progress on this?

Unfortunately, nothing conclusive.

I have tried to finish this feature, but insistence to add Elasticsearch v8 template in the same pull request also lead to a 2nd issue, that there is a new endpoint of v8 templates. That means, as far as I can see, that Serilog.Sinks.Elasticsearch now needs to accurately detect version of Elasticsearch (instead of relying on version of template that user has set manually). On top of all this netcoreapp3.1 is now obsolete, so that needed update as well. Finally, tests on CI are breaking - my guess due to outdated scripts/dependencies.

Shortly, I would like to finish this, but without anyone who knows whats and whys of the project I don't think I proceed and be sure that I do not break something that is supposed to work.

[mivano]

@nenadvicentic thanks for your efforts. Regarding the what and why's of this project; I m afraid I can't help that much either anymore. See #486 for background.

That also means that the how can be open to a lot of options as well. The main intent of this sink was always to get started with a minimum of parameters (only location of ES) and everything would be handled. During the years (since 2014), a lot has changed, not only in the amount of options, but also the different ES versions and .NET versions.

Without understanding the full extent of this specific issue, there are a couple of ways forward; like dropping support for netframework, for 3.1 etc. Same for no longer sticking with ES smaller than e.g. 7. Unfortunately we do not know what people are using and needing, on the other hand; this is an open source project, not a long term support project. It makes life a lot easier if we can skim down on a couple of features, bring it to a newer framework and continue from there. If there is still a need for e.g. ES 6, then just stick with a specific version of this sink, or make a fork.

So if you (and/or anybody else) have the time, energy and interest, then please take the opportunity to bring this sink to a new level, even if that means we need to make concessions.

[nenadvicentic]

@mivano Thank you for context and additional explanation. It helps to wrap my head around some of the issues I faced.

I will put additional into this feature and try to finish it best I can, if it is OK for everyone here. Company I work for agreed to give me time to do this.

Regarding maintenance of this project, at this point it would not fit with my schedules. Also, my Elasticsearch knowledge is on "occasional consumer" level.

[Schoof-T]

@mivano I agree with your consensus, it's better to skim down on a couple of features and try to get this project running with the newer frameworks. The current project works well with the previous versions, so people can stay on those versions if they so wish.

@nenadvicentic Thank you for taking the time to try and finish this feature. I'm sure it will be very appreciated by everyone using this project .

[nenadvicentic]

#462 has been merged to dev