CPM handle 404 response gracefully with user-friendly log#17052
CPM handle 404 response gracefully with user-friendly log#17052kaisecheng merged 6 commits intoelastic:mainfrom
Conversation
|
es-output 12.0.2 leaves the caller to make the decision on what to consider a bad response code error. Although this PR allow CPM to handle 404 response gracefully with user-friendly log, the released versions with es-output 12.0.2 are still getting the stack trace in log An alternative solution is to restore 404 behaviour in es-output. |
|
Out of curiousity... Why not handle the 404 in |
Co-authored-by: Cas Donoghue <cas.donoghue@gmail.com>
To handle in fetcher, it requires changes on SystemIndicesFetcher, LegacyHiddenIndicesFetcher, the internal state of Thanks for review. The pipelines should be Array |
|
I'm not quite sure I'm following your comment here #17052 (comment). I think that the behavior we are trying to preserve is that logstash/x-pack/lib/config_management/elasticsearch_source.rb Lines 213 to 224 in 5573b5a fetch_config linked in #17052 (comment) ). It seems to me like the responsibility of that is fully in the fetch_config method as it is the consumer of the plugin's client. Moving the required knowledge of an error type defined in logstash-output-elasticsearch to the source loader seems to perpetuate the underlying tech debt issue of using the plugin's client rather than our own.
It seems to me that the path forward would be to for now fix up instances of |
|
I see what you mean. We have two fetchers using agree your point that it is better to put try catch block close to the caller class. In this case, it should be inside xpack.
probably not, we have another use case licence checker which is also in xpack |
| begin | ||
| fetcher.fetch_config(es_version, pipeline_ids, client) | ||
| rescue LogStash::Outputs::ElasticSearch::HttpClient::Pool::BadResponseCodeError => e | ||
| return [] if e.response_code == 404 |
There was a problem hiding this comment.
Will this early return cause a difference in behavior whereby the @cached_pipelines are not updated on 404 now? Does that matter?
There was a problem hiding this comment.
It doesn't matter. The pipeline is empty anyway and the cache is purely optional
There was a problem hiding this comment.
Reviewing the flow, I decide to remove the cache because the existing pipelines can keep running without cache when error is raised. It was there to prevent cleaning up the running pipelines.
|
Thanks for the explanation. Given this is a short term solution anyway with the intent to get a dedicated client I think its OK if we are concerned about the risks/burdens you mentioned. |
|
💛 Build succeeded, but was flaky
Failed CI StepsHistory
|
donoghuc
left a comment
There was a problem hiding this comment.
I couldn't quite trace back the git history for the addition of the cache (dead ends with a generic "migrate x-pack" bulk commit 93cad10.
Its kind of hard to trace through exactly what all calls this (or how many times), but i dont see anything egregiously consuming it.
|
@logstashmachine backport 9.0 |
|
@logstashmachine backport 8.18 |
Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7)
Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7)
|
@logstashmachine backport 8.17 |
|
@logstashmachine backport 8.16 |
Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7)
Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7)
…17097) Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7) Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
…17098) Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7) Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
…17099) Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7) Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
…17100) Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7) Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
) Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: elastic#17035
…gify * upstream/main: (27 commits) Add Windows 2025 to CI (elastic#17133) Update container acceptance tests with stdout/stderr changes (elastic#17138) entrypoint: avoid polluting stdout (elastic#17125) Fix acceptance test assertions for updated plugin remove (elastic#17126) Fix acceptance test assertions for updated plugin `remove` (elastic#17122) plugins: improve `remove` command to support multiple plugins (elastic#17030) spec: improve ls2ls spec (elastic#17114) allow concurrent Batch deserialization (elastic#17050) CPM handle 404 response gracefully with user-friendly log (elastic#17052) qa: use clean expansion of LS tarball per fixture instance (elastic#17082) Allow capturing heap dumps in DRA BK jobs (elastic#17081) Use centralized source of truth for active branches (elastic#17063) Update logstash_releases.json (elastic#17055) fix logstash-keystore to accept spaces in values when added via stdin (elastic#17039) Don't honor VERSION_QUALIFIER if set but empty (elastic#17032) Release note placeholder might be empty, making parsing lines nil tolerant. (elastic#17026) Fix BufferedTokenizer to properly resume after a buffer full condition respecting the encoding of the input string (elastic#16968) Add short living 9.0 next and update main in CI release version definition. (elastic#17008) Core version bump to 9.1.0 (elastic#16991) Add 9.0 branch to the CI branches definition (elastic#16997) ...
Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error.
This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message.
It also removes the redundant cache of pipeline in CPM
Fixes: #17035