Logging improvements when plugin version mismatches with connected ES version #247

mashhurs · 2025-01-23T14:38:14Z

Description

Plugin aligns with stack {major.minor} now. This gives an opportunity to highlight in the logs if plugin is running against the different ES version from the version plugin is built.
This PR considers:

Connected ES is newer major (warn: pipelines using new features won't work)
Connected ES is older major (warn: pipelines using deprecated-in-old-major removed-in-new-major options will fail to load)
Connected ES is same major, but:
- Connected ES is newer minor (warn: pipelines using new features won't work)
- Connected ES is older minor (info: low-risk mismatch)
- Connected ES is same minor (debug: everything is okay)

Example results

When plugin and ES major.minor versions are same

[2025-01-22T15:11:12,861][INFO ][logstash.filters.elasticintegration][main] This 8.17.0 version of plugin embedded Ingest node components from Elasticsearch 8.17

When plugin minor version is ahead of ES minor version

2025-01-22T15:16:57,997][INFO ][logstash.filters.elasticintegration][main] This 8.18.0 version of plugin embedded Ingest node components from Elasticsearch 8.18
[2025-01-22T15:16:57,997][INFO ][logstash.filters.elasticintegration][main] This 8.18.0 version of plugin is compiled with newer Elasticsearch version than currently connected Elasticsearch 8.17 version

When plugin major version is ahead of ES major version

[2025-01-22T15:27:06,285][INFO ][logstash.filters.elasticintegration][main] This 9.0.0 version of plugin embedded Ingest node components from Elasticsearch 9.0
[2025-01-22T15:27:06,285][INFO ][logstash.filters.elasticintegration][main] This 9.0.0 version of plugin is compiled with newer Elasticsearch version than currently connected Elasticsearch 8.17 version. Upgrade the plugin and/or stack to the same `major.minor` to get the minimal disruptive experience.

- When plugin major version is behind of ES major version
[2025-01-22T15:28:59,232][INFO ][logstash.filters.elasticintegration][main] This 8.17.0 version of plugin embedded Ingest node components from Elasticsearch 8.17
[2025-01-22T15:28:59,233][WARN ][logstash.filters.elasticintegration][main] This 8.17.0 version of plugin is compiled with older Elasticsearch version than currently connected Elasticsearch 9.0 version. Upgrade the plugin and/or stack to the same `major.minor` to get the minimal disruptive experience.

When plugin minor version is behind of ES minor version

[2025-01-23T07:23:26,577][INFO ][logstash.filters.elasticintegration][main] This 8.16.0 version of plugin embedded Ingest node components from Elasticsearch 8.16
[2025-01-23T07:23:26,595][INFO ][logstash.filters.elasticintegration][main] This 8.16.0 version of plugin is compiled with older Elasticsearch version than currently connected Elasticsearch 8.17 version. Upgrade the plugin and/or stack to the same `major.minor` to get the minimal disruptive experience

…ding this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance.

lib/logstash/filters/elastic_integration.rb

yaauie

The approach makes sense. +1 to absorbing this into the existing http call.

I left a comment and suggestion for making the guidance composition for version alignments less D.R.Y., so that we can make it more accurately descriptive.

lib/logstash/filters/elastic_integration.rb

+
+    es_version_parts = connected_es_version_info["number"].split('.')
+    es_major_version = es_version_parts[0].to_i
+    es_minor_version = es_version_parts[1].to_i


lib/logstash/filters/elastic_integration.rb

…h is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]>

yaauie

LGTM. One comment about code readability that is a result of my prior suggestion, but feel free to resolve or ignore at your discretion.

yaauie · 2025-01-24T18:05:21Z

lib/logstash/filters/elastic_integration.rb

+      logger.warn <<~WARNING
+        This plugin v#{VERSION} is connected to a newer MAJOR version of Elasticsearch v#{es_full_version}, and may have trouble loading or running pipelines that use new features; for the best experience, update this plugin to at least v#{es_major_version}.#{es_minor_version}
+      WARNING


The intent of my prior suggestion to use heredocs was to enable the text to be readable with line-breaks, but I missed an important bit where even squiggle-heredocs keep the newlines 🤦🏼.

I don't think that the squiggle-heredocs are inherently more readable though, so it might be worth going back to normal strings.

Suggested change

logger.warn <<~WARNING

This plugin v#{VERSION} is connected to a newer MAJOR version of Elasticsearch v#{es_full_version}, and may have trouble loading or running pipelines that use new features; for the best experience, update this plugin to at least v#{es_major_version}.#{es_minor_version}

WARNING

logger.warn "This plugin v#{VERSION} is connected to a newer MAJOR version of Elasticsearch v#{es_full_version}, and may have trouble loading or running pipelines that use new features; for the best experience, update this plugin to at least v#{es_major_version}.#{es_minor_version}"

or with concatenation (each needs a trailing space)

Suggested change

logger.warn <<~WARNING

This plugin v#{VERSION} is connected to a newer MAJOR version of Elasticsearch v#{es_full_version}, and may have trouble loading or running pipelines that use new features; for the best experience, update this plugin to at least v#{es_major_version}.#{es_minor_version}

WARNING

logger.warn "This plugin v#{VERSION} is connected to a newer MAJOR " +

"version of Elasticsearch v#{es_full_version}, and may " +

"have trouble loading or running pipelines that use new " +

"features; for the best experience, update this plugin " +

"to at least v#{es_major_version}.#{es_minor_version}"

or a heredoc, but with trickery to turn the newlines into spaces:

Suggested change

logger.warn <<~WARNING

This plugin v#{VERSION} is connected to a newer MAJOR version of Elasticsearch v#{es_full_version}, and may have trouble loading or running pipelines that use new features; for the best experience, update this plugin to at least v#{es_major_version}.#{es_minor_version}

WARNING

logger.warn(<<~WARNING.tr("\n", " "))

This plugin v#{VERSION} is connected to a newer MAJOR version of

Elasticsearch v#{es_full_version}, and may have trouble loading or

running pipelines that use new features; for the best experience,

update this plugin to at least v#{es_major_version}.#{es_minor_version}

WARNING

This was the hardest part actually!

heredocs had a power here to highlight the warnings with multiline texts. However, when I did an actual test

the console look so ugly-cut-sentenced, especially when I made a half screen console - sentences alignment;

logs didn't align with our logging standard, in a kind of jumpy way - easy to lose concentration;

Overall, it looks to me our current standard loggings has a better readability (smooth reading experience) and concentration (not jump words) points.

In case of readability, yes a) I forgot to remove heredocs styles b) made unit tests readable instead actual logic (had a sleep disorder 5AM commit, sorry) - thank you for revisiting

My last commit rearranges (your 2nd suggestion, concatenation with trailing space) and removes heredocs style!

…e source readability.

elasticmachine · 2025-01-24T20:17:32Z

💚 Build Succeeded

Buildkite Build
Commit: d58fb8c

History

💚 Build #497 succeeded e4dc62d
💚 Build #496 succeeded b61b374
💚 Build #494 succeeded d03f20e
💔 Build #492 failed e373d9e
💔 Build #491 failed 215fc61

cc @mashhurs

yaauie

👍 LGTM

mashhurs · 2025-01-27T21:49:46Z

@logstashmachine backport 8.x

… version (#247) * Logging improvements: informing which ES version considered when building this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance. * Preflight check initialization optimization. * Refactored messages in a way for better visibility. Any major mismatch is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]> * Standardize log outputs on console. --------- Co-authored-by: Ry Biesemeyer <[email protected]> (cherry picked from commit 021e8c7)

mashhurs · 2025-01-27T23:21:54Z

@logstashmachine backport 8.16

… version (#247) * Logging improvements: informing which ES version considered when building this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance. * Preflight check initialization optimization. * Refactored messages in a way for better visibility. Any major mismatch is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]> * Standardize log outputs on console. --------- Co-authored-by: Ry Biesemeyer <[email protected]> (cherry picked from commit 021e8c7)

mashhurs · 2025-01-28T00:09:44Z

@logstashmachine backport 8.17

… version (#247) * Logging improvements: informing which ES version considered when building this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance. * Preflight check initialization optimization. * Refactored messages in a way for better visibility. Any major mismatch is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]> * Standardize log outputs on console. --------- Co-authored-by: Ry Biesemeyer <[email protected]> (cherry picked from commit 021e8c7)

… version (#247) (#250) * Logging improvements: informing which ES version considered when building this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance. * Preflight check initialization optimization. * Refactored messages in a way for better visibility. Any major mismatch is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]> * Standardize log outputs on console. --------- Co-authored-by: Ry Biesemeyer <[email protected]> (cherry picked from commit 021e8c7) Co-authored-by: Mashhur <[email protected]>

… version (#247) (#253) * Logging improvements: informing which ES version considered when building this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance. * Preflight check initialization optimization. * Refactored messages in a way for better visibility. Any major mismatch is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]> * Standardize log outputs on console. --------- Co-authored-by: Ry Biesemeyer <[email protected]> (cherry picked from commit 021e8c7) Co-authored-by: Mashhur <[email protected]>

… version (#247) (#255) * Logging improvements: informing which ES version considered when building this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance. * Preflight check initialization optimization. * Refactored messages in a way for better visibility. Any major mismatch is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]> * Standardize log outputs on console. --------- Co-authored-by: Ry Biesemeyer <[email protected]> (cherry picked from commit 021e8c7) Co-authored-by: Mashhur <[email protected]>

Logging improvements: informing which ES version considered when buil…

ddaa1a3

…ding this plugin version, when plugin and connected ES versions do not match warn/inform incompatibility risk and show the guidance.

mashhurs linked an issue Jan 23, 2025 that may be closed by this pull request

Logging improvements when embedded vs querying ES version mismatch #246

Closed

Preflight check initialization optimization.

e373d9e

mashhurs force-pushed the log-improvements-when-versions-mismatch branch from 215fc61 to e373d9e Compare January 23, 2025 17:23

Unit tests added, failed integration test cases fixed.

d03f20e

mashhurs self-assigned this Jan 23, 2025

mashhurs marked this pull request as ready for review January 23, 2025 23:12

mashhurs commented Jan 23, 2025

View reviewed changes

lib/logstash/filters/elastic_integration.rb Outdated Show resolved Hide resolved

mashhurs changed the title ~~Logging improvements when plugin version mismatch with connected ES version~~ Logging improvements when plugin version mismatches with connected ES version Jan 23, 2025

mashhurs commented Jan 24, 2025

View reviewed changes

lib/logstash/filters/elastic_integration.rb Outdated Show resolved Hide resolved

yaauie reviewed Jan 24, 2025

View reviewed changes

mashhurs and others added 2 commits January 23, 2025 18:56

Refactored messages in a way for better visibility. Any major mismatc…

b61b374

…h is warning and connected ES minor ahead is warning as well. Co-authored-by: Ry Biesemeyer <[email protected]>

Standardize log outputs on console.

e4dc62d

mashhurs requested a review from yaauie January 24, 2025 13:52

yaauie previously approved these changes Jan 24, 2025

View reviewed changes

Remove forgotten heredocs style and rearrange warning texts to improv…

d58fb8c

…e source readability.

mashhurs dismissed yaauie’s stale review via d58fb8c January 24, 2025 19:59

mashhurs requested a review from yaauie January 24, 2025 20:36

yaauie approved these changes Jan 27, 2025

View reviewed changes

mashhurs merged commit 021e8c7 into elastic:main Jan 27, 2025
2 checks passed

mashhurs deleted the log-improvements-when-versions-mismatch branch January 27, 2025 21:49

github-actions bot added the v0.1.17 label Jan 27, 2025

github-actions bot mentioned this pull request Jan 27, 2025

Backport PR #247 to 8.x: Logging improvements when plugin version mismatches with connected ES version #250

Merged

github-actions bot mentioned this pull request Jan 27, 2025

Backport PR #247 to 8.16: Logging improvements when plugin version mismatches with connected ES version #253

Merged

github-actions bot added the v8.16.0 label Jan 27, 2025

mashhurs mentioned this pull request Jan 28, 2025

Removes the fixed version number and uses plugin version instead in the unit tests. #254

Merged

github-actions bot added the v8.17.0 label Jan 28, 2025

github-actions bot mentioned this pull request Jan 28, 2025

Backport PR #247 to 8.17: Logging improvements when plugin version mismatches with connected ES version #255

Merged

github-actions bot mentioned this pull request Jan 28, 2025

Backport PR #254 to 8.x: Removes the fixed version number and uses plugin version instead in the unit tests. #256

Merged

github-actions bot mentioned this pull request Jan 28, 2025

Backport PR #254 to 8.16: Removes the fixed version number and uses plugin version instead in the unit tests. #257

Merged

github-actions bot mentioned this pull request Jan 28, 2025

Backport PR #254 to 8.17: Removes the fixed version number and uses plugin version instead in the unit tests. #258

Merged

-      logger.warn <<~WARNING
-        This plugin v#{VERSION} is connected to a newer MAJOR version of Elasticsearch v#{es_full_version}, and may have trouble loading or running pipelines that use new features; for the best experience, update this plugin to at least v#{es_major_version}.#{es_minor_version}
-      WARNING
+      logger.warn "This plugin v#{VERSION} is connected to a newer MAJOR " +
+                  "version of Elasticsearch v#{es_full_version}, and may " +
+                  "have trouble loading or running pipelines that use new " +
+                  "features; for the best experience, update this plugin " +
+                  "to at least v#{es_major_version}.#{es_minor_version}"

Logging improvements when plugin version mismatches with connected ES version #247

Logging improvements when plugin version mismatches with connected ES version #247

Uh oh!

Conversation

mashhurs commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Example results

Uh oh!

Uh oh!

Uh oh!

yaauie left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

yaauie left a comment

Choose a reason for hiding this comment

Uh oh!

yaauie Jan 24, 2025

Choose a reason for hiding this comment

Uh oh!

mashhurs Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jan 24, 2025

💚 Build Succeeded

History

Uh oh!

yaauie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mashhurs commented Jan 27, 2025

Uh oh!

mashhurs commented Jan 27, 2025

Uh oh!

mashhurs commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mashhurs commented Jan 23, 2025 •

edited

Loading

mashhurs Jan 24, 2025 •

edited

Loading