Fix Elasticsearch index behavior in case of missing settings #42426

3kt · 2025-01-25T18:12:38Z

Proposed commit message

Fixes #42424

In more details:

reactivate test coverage for elasticsearch/index module
add a test case for missing routing table / index settings
add a fix to pass said test

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

Author's Checklist

~~Check in Elasticsearch source code what settings can be missing, and when this happens (creation_date and tier_preference)~~ Irrelevant, we decided to tolerate index setting failure
Verifies that the integration that uses the generated data behaves correctly if the aforementioned settings are missing.

How to test this PR locally

Related issues

Closes [Metricbeat] [Elasticsearch] Error in case of missing settings key #42424

mergify · 2025-01-25T18:13:20Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @3kt? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

mergify · 2025-01-25T18:13:21Z

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

3kt · 2025-01-25T18:31:17Z

@consulthys I'm unsure how to approach the missing setting case: theoretically a race condition can cause tis any time, and with enough iterations it is bound to happen on every cluster.

Indeed, a measure from MB is 3 API calls:

_stats
_cluster/state
_settings

If something changes between these calls, it can be impossible for Metricbeat to reconciliate the data structures.

Now the positive thing is that the errors are neither unrecoverable nor blocking, I would expect Metricbeat to continue shipping while reporting errors.

We could silently ignore these issues, keep the current behavior, or refine when/how we want to raise these problems.

What do you think?

consulthys · 2025-01-27T11:23:33Z

@3kt Good question! I think the main purpose of this metric set is to report _stats, so if either or both of the other two calls fail for any reason, I think we should log it in WARN but still return the index stats data, but without the settings fields... which also means the new usage dashboard might have some holes from time to time or for specific indices. What do you think?

3kt · 2025-01-27T18:59:20Z

@consulthys I agree, but how would this differ from the current behavior? AFAICT the eventsMapping function returns an error and nothing else, and upon returning such a thing, the agent would show up as unhealthy (which is the very thing we would like to prevent).

consulthys · 2025-01-28T11:40:49Z

how would this differ from the current behavior?

I would not add the error to errs, but just log it. Now, the problem is that for some indices we know it's going to fail every time, so we should probably log this as DEBUG in order not to deliberately increase the logs volume. Wdyt?

TristanAdams03 · 2025-01-28T18:16:00Z

what is the best way to fix this at the moment I am getting 4 errors in regards to "key not found" for the elasticsearch integration module? Thank you.

TristanAdams03 · 2025-01-28T18:21:17Z

Is this on master nodes only or the kibana node?

TristanAdams03 · 2025-01-28T18:39:43Z

is there a way to fix the issue with "key not found" within elastic without messing with the code it is causing my stack to pop up as degrading because the indexes in question are throwing back that error.

TristanAdams03 · 2025-01-28T18:47:38Z

i do not know where these files are located

consulthys

LGT Stack Monitoring

rdner

Lots of reformatting of Python code that does not relate to the fix but all in all looks good to me.

3kt · 2025-01-29T09:43:41Z

Lots of reformatting of Python code that does not relate to the fix but all in all looks good to me.

Yea, for some reason the make fmt triggered this - unsure why but let's not argue with the CI ;)

mergify · 2025-01-29T10:31:37Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b fix_missing_settings upstream/fix_missing_settings
git merge upstream/main
git push upstream fix_missing_settings

belimawr

LGTM.

nicholasberlin

packetbeat changes LGTM

* Unskip test * Bold move: assumes we can drop test coverage for unsupported versions * Added test case: index with no cluster state entry or settings * Added comment about "race situation" * Logged index settings error as debug, fixed test case * Handling for multi version tests * Added changelog entry --------- Co-authored-by: Valentin Crettaz <[email protected]> Co-authored-by: Valentin Crettaz <[email protected]> (cherry picked from commit 074a201)

…#42474) * Unskip test * Bold move: assumes we can drop test coverage for unsupported versions * Added test case: index with no cluster state entry or settings * Added comment about "race situation" * Logged index settings error as debug, fixed test case * Handling for multi version tests * Added changelog entry --------- Co-authored-by: Valentin Crettaz <[email protected]> Co-authored-by: Valentin Crettaz <[email protected]> (cherry picked from commit 074a201) Co-authored-by: Alexis Charveriat <[email protected]>

…#42475) * Unskip test * Bold move: assumes we can drop test coverage for unsupported versions * Added test case: index with no cluster state entry or settings * Added comment about "race situation" * Logged index settings error as debug, fixed test case * Handling for multi version tests * Added changelog entry --------- Co-authored-by: Valentin Crettaz <[email protected]> Co-authored-by: Valentin Crettaz <[email protected]> (cherry picked from commit 074a201) Co-authored-by: Alexis Charveriat <[email protected]>

3kt added 4 commits January 25, 2025 00:22

Unskip test

a7da4e7

Bold move: assumes we can drop test coverage for unsupported versions

59010d3

Added test case: index with no cluster state entry or settings

b276a98

Added comment about "race situation"

75e2c30

3kt added the bug label Jan 25, 2025

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 25, 2025

3kt requested a review from consulthys January 25, 2025 18:12

mergify bot assigned 3kt Jan 25, 2025

mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Jan 25, 2025

3kt added the backport-8.17 Automated backport with mergify label Jan 25, 2025

3kt added 3 commits January 28, 2025 17:53

Logged index settings error as debug, fixed test case

7bcf394

Handling for multi version tests

c1d6e24

Merge branch 'main' into fix_missing_settings

eda112a

consulthys added Feature:Stack Monitoring Team:Monitoring Stack Monitoring team labels Jan 28, 2025

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jan 28, 2025

3kt and others added 2 commits January 28, 2025 18:25

Added changelog entry

101a4df

Merge branch 'main' into fix_missing_settings

acd6b1c

make update & make fmt

d53dc95

Make update

b6f5c6a

3kt marked this pull request as ready for review January 28, 2025 22:32

3kt requested review from a team as code owners January 28, 2025 22:32

3kt requested review from belimawr and rdner January 28, 2025 22:32

Merge branch 'main' into fix_missing_settings

175da2b

consulthys approved these changes Jan 29, 2025

View reviewed changes

rdner approved these changes Jan 29, 2025

View reviewed changes

Merge branch 'main' into fix_missing_settings

9973414

3kt enabled auto-merge (squash) January 29, 2025 14:39

belimawr approved these changes Jan 29, 2025

View reviewed changes

nicholasberlin approved these changes Jan 29, 2025

View reviewed changes

3kt merged commit 074a201 into elastic:main Jan 29, 2025
142 checks passed

consulthys added the backport label Jan 29, 2025

This was referenced Jan 29, 2025

[8.x](backport #42426) Fix Elasticsearch index behavior in case of missing settings #42474

Merged

[8.17](backport #42426) Fix Elasticsearch index behavior in case of missing settings #42475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Elasticsearch index behavior in case of missing settings #42426

Fix Elasticsearch index behavior in case of missing settings #42426

3kt commented Jan 25, 2025 •

edited

Loading

mergify bot commented Jan 25, 2025

mergify bot commented Jan 25, 2025

3kt commented Jan 25, 2025

consulthys commented Jan 27, 2025

3kt commented Jan 27, 2025

consulthys commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

consulthys left a comment

rdner left a comment

3kt commented Jan 29, 2025

mergify bot commented Jan 29, 2025

belimawr left a comment

nicholasberlin left a comment

Fix Elasticsearch index behavior in case of missing settings #42426

Fix Elasticsearch index behavior in case of missing settings #42426

Conversation

3kt commented Jan 25, 2025 • edited Loading

Proposed commit message

Checklist

Disruptive User Impact

Author's Checklist

How to test this PR locally

Related issues

mergify bot commented Jan 25, 2025

mergify bot commented Jan 25, 2025

3kt commented Jan 25, 2025

consulthys commented Jan 27, 2025

3kt commented Jan 27, 2025

consulthys commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

TristanAdams03 commented Jan 28, 2025

consulthys left a comment

Choose a reason for hiding this comment

rdner left a comment

Choose a reason for hiding this comment

3kt commented Jan 29, 2025

mergify bot commented Jan 29, 2025

belimawr left a comment

Choose a reason for hiding this comment

nicholasberlin left a comment

Choose a reason for hiding this comment

3kt commented Jan 25, 2025 •

edited

Loading