Logstash Ingest Pipelines - Add preflight check ensuring the filter <-> ES version compatibility

[jsvd]

This filter should only be able to connect to ES cluster with either same or earlier major.minor versions.
This is to ensure that integration pipelines fetched from ES will work in Logstash.

[yaauie]

Strictly speaking the compatibility is about the version of Elasticsearch that is embedded in the plugin, not the version of Logstash that is running the plugin.

[andsel]

In case tests runs against 8.x snapshot, like in this two cases:

• logstash-filter-elastic_integration/.buildkite/pipeline.yml

  Line 34 in 1589cd9

  SNAPSHOT: true

• logstash-filter-elastic_integration/.buildkite/pipeline.yml

  Line 15 in 1589cd9

  SNAPSHOT: true

A version check that assert "minor or equal would fail". Suppose that the bundled version is 8.11.0 like in definition while the mainbranch of development for Elasticsearch switch to 8.12.0-SNAPSHOT any invocation of register would fail, because remote (8.12.0) is newer than the bundled one (8.11.0).

We could add a conditional check of SNAPSHOT environment variable inside the version checking method, that would skip the version check when it runs in CI, but I think that's a good way to circumnavigate the preflight check that also users would use.

The other option is to remove snapshot BK pipelines on CI.

WDYT?

[mashhurs]

In case tests runs against 8.x snapshot, like in this two cases:

• logstash-filter-elastic_integration/.buildkite/pipeline.yml

  Line 34 in 1589cd9

  SNAPSHOT: true

• logstash-filter-elastic_integration/.buildkite/pipeline.yml

  Line 15 in 1589cd9

  SNAPSHOT: true

A version check that assert "minor or equal would fail". Suppose that the bundled version is 8.11.0 like in definition while the mainbranch of development for Elasticsearch switch to 8.12.0-SNAPSHOT any invocation of register would fail, because remote (8.12.0) is newer than the bundled one (8.11.0).

We could add a conditional check of SNAPSHOT environment variable inside the version checking method, that would skip the version check when it runs in CI, but I think that's a good way to circumnavigate the preflight check that also users would use.

The other option is to remove snapshot BK pipelines on CI.

WDYT?

I think this requires a discussion with a team. I do see overall two caveats:

1. In my opinion, 8.x with SNAPSHOT: true here should indicate 8.11.2-SNAPSHOT. The 8.12.0-SNAPSHOT is from main branch. I am going to suggest updating our release definition, change 8.x to current minor snapshot and add main (8.12-SNAPSHOT for now) if team agrees. See my comment for details.

2. Current preflight version check should allow us a way to go further processing in the integration/unit test cases. Why?! Our goal is to detect the issues as earlier as possible. We are setting up a BK pipeline to build the plugin with ES 8.x, 8.minor-SNAPSHOT and main (this may indicate 8.major-SNAPSHOT or 9.x, etc..) version to detect the issues so that we can take an action to keep the consistency with ES versions.

[andsel]

Thanks @mashhurs for commenting!
I think that if we switch to new versioning convention in ci/logstash_releases.json and in CI definitions, then my concern ceases to be an issue.

[mashhurs]

Thanks @mashhurs for commenting! I think that if we switch to new versioning convention in ci/logstash_releases.json and in CI definitions, then my concern ceases to be an issue.

Hm., the case I was talking would be useful where ELASTICSEARCH_TREEISH will be defined in a {major.minor} format (ex: 8.11), which covers 8.11.*(including snapshots). However, as you mentioned, if full version like 8.11.0, it doesn't require to cover SNAPSHOT versions. Is there any way we can put a dynamic condition in integ tests, if version satisfies then runs rest of integration tests?

Another thing, I wonder if {es-cluster/version['number']} version API provides branch, commit, etc.. as we accept for ELASTICSEARCH_TREEISH ?

[andsel]

I wonder if {es-cluster/version['number']} version API provides branch, commit, etc.

No, it doesn't provide branch/commit/tag information, it just provides some build information like if it's snapshot or an hash which shouldn't be related to the origin. Here is an example response from ES /:

{
  "name" : "hosthosthost",
  "cluster_name" : "elasticsearch",
  "cluster_uuid" : "asdasdasdasdasdasdasdasd",
  "version" : {
    "number" : "8.10.3",
    "build_flavor" : "default",
    "build_type" : "tar",
    "build_hash" : "c63272efed16b5a1c25f3ce500715b7fddf9a9fb",
    "build_date" : "2023-10-05T10:15:55.152563867Z",
    "build_snapshot" : false,
    "lucene_version" : "9.7.0",
    "minimum_wire_compatibility_version" : "7.17.0",
    "minimum_index_compatibility_version" : "7.0.0"
  },
  "tagline" : "You Know, for Search"
}

[yaauie]

We are electing to defer this specific effort until we can address the user story more directly:

   As a: consumer of Elastic Integrations using Logstash
         who does not control the configuration of those
         Integrations or the upgrade schedule of the
         Elasticsearch cluster where those Integrations
         are applied

 I want: to avoid running running integrations that cannot
         be satisfied by the implementation that is available
         to this plugin

So that: when integrations are upgraded I don't encounter
         a flood of errors or incorrectly-enriched events

In terms of addressing the root goal, version-pinning is a sledgehammer when we need a scalpel. It does not address situations where Elasticsearch or an Integration is updated while this plugin is running, adds significant complexity to upgrades, and in most cases use of this plugin will not actually be affected by a minor upgrade of Elasticsearch.

---

We want our customers to upgrade with us, to benefit from our hard work across the entire stack as soon as they are able, and introducing complexity to upgrades runs counter to that goal.

When we introduce a happens-before dependency to ensure that this plugin is upgraded before Elasticsearch or after Logstash, or a transactional dependency to ensure that neither is upgraded before the other, that added complexity becomes a barrier to upgrades. In a simple system these dependencies can appear trivial, but in the real world they quickly combine to become an explosion of complexity that outweighs the benefit of having access to fixes and new features.

---

Currently, if this plugin fails to load a pipeline or fails while executing a loaded pipeline (such as when its definition uses a new processor or processor option that is unavailable to our implementation), the event is tagged and left unprocessed by this plugin, and then emitted downstream to the next filter or output in the pipeline. This gives the Logstash-pipeline maintainer enough control to follow the general Logstash best-practice of defining their pipelines in such a way that they route error-tagged events elsewhere for later processing.

Ideally, at some point we will become a more integrated part of Elastic Integrations, but what that looks like is still to be determined.

[mashhurs]

With recent aligning with stack versions, I believe this issue is no longer has a power.