Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Elastic Api Version header #1147

Merged
merged 9 commits into from Sep 25, 2023

Conversation

kaisecheng
Copy link
Contributor

@kaisecheng kaisecheng commented Aug 31, 2023

This commit adds "Elastic-Api-Version" : "2023-10-31" to request header when the Elasticsearch is serverless
Rename "flavour" to "flavor"

…asticsearch into add_eav_header

# Conflicts:
#	CHANGELOG.md
Copy link
Contributor

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the current setup, we don't correctly handle cases where the api version doesn't match.

For example, if we tweak the constant value to something else and try, you get:

[2023-09-18T12:48:46,537][INFO ][logstash.javapipeline    ][main] Starting pipeline {:pipeline_id=>"main", "pipeline.workers"=>10, "pipeline.batch.size"=>125, "pipeline.batch.delay"=>50, "pipeline.max_inflight"=>1250, "pipeline.sources"=>["config string"], :thread=>"#<Thread:0x67ceb73c /private/tmp/logstash-8.10.0/logstash-core/lib/logstash/java_pipeline.rb:134 run>"}
[2023-09-18T12:48:46,586][ERROR][logstash.outputs.elasticsearch][main] Unable to retrieve Elasticsearch cluster uuid {:message=>"Got response code '400' contacting Elasticsearch at URL 'https://test-d89f31.es.eu-west-1.aws.qa.elastic.cloud:443/'", :exception=>LogStash::Outputs::ElasticSearch::HttpClient::Pool::BadResponseCodeError, :backtrace=>["/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/manticore_adapter.rb:84:in `perform_request'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:339:in `perform_request_to_url'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:325:in `block in perform_request'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:413:in `with_connection'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:324:in `perform_request'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:332:in `block in Pool'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client.rb:212:in `get'", "/tmp/logstash-output-elasticsearch/lib/logstash/plugin_mixins/elasticsearch/common.rb:179:in `discover_cluster_uuid'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch.rb:352:in `finish_register'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch.rb:326:in `block in register'", "/tmp/logstash-output-elasticsearch/lib/logstash/plugin_mixins/elasticsearch/common.rb:172:in `block in after_successful_connection'"]}
{"error":{"root_cause":[{"type":"status_exception","reason":"The requested [Elastic-Api-Version] header value of [2024-10-31] is not valid. Only [2023-10-31] is supported"}],"type":"status_exception","reason":"The requested [Elastic-Api-Version] header value of [2024-10-31] is not valid. Only [2023-10-31] is supported"},"status":400}
[2023-09-18T12:48:46,589][INFO ][logstash.outputs.elasticsearch][main] Using a default mapping template {:es_version=>8, :ecs_compatibility=>:v8}
[2023-09-18T12:48:46,594][INFO ][logstash.outputs.elasticsearch][main] ILM auto configuration (`ilm_enabled => auto` or unset) resolved to `false`. Serverless Elasticsearch cluster does not support Index Lifecycle Management.
[2023-09-18T12:48:46,647][ERROR][logstash.outputs.elasticsearch][main] Failed to install template {:message=>"Got response code '400' contacting Elasticsearch at URL 'https://test-d89f31.es.eu-west-1.aws.qa.elastic.cloud:443/_index_template/ecs-logstash'", :exception=>LogStash::Outputs::ElasticSearch::HttpClient::Pool::BadResponseCodeError, :backtrace=>["/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/manticore_adapter.rb:84:in `perform_request'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:339:in `perform_request_to_url'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:325:in `block in perform_request'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:413:in `with_connection'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:324:in `perform_request'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client/pool.rb:332:in `block in Pool'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client.rb:405:in `exists?'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client.rb:410:in `template_exists?'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/http_client.rb:81:in `template_install'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/template_manager.rb:44:in `install'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch/template_manager.rb:32:in `install_template'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch.rb:595:in `install_template'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch.rb:353:in `finish_register'", "/tmp/logstash-output-elasticsearch/lib/logstash/outputs/elasticsearch.rb:326:in `block in register'", "/tmp/logstash-output-elasticsearch/lib/logstash/plugin_mixins/elasticsearch/common.rb:172:in `block in after_successful_connection'"]}
[2023-09-18T12:48:46,857][INFO ][logstash.javapipeline    ][main] Pipeline Java execution initialization time {"seconds"=>0.32}
[2023-09-18T12:48:46,869][INFO ][logstash.javapipeline    ][main] Pipeline started {"pipeline.id"=>"main"}
The stdin plugin is now waiting for input:
[2023-09-18T12:48:46,884][INFO ][logstash.agent           ] Pipelines running {:count=>1, :running_pipelines=>[:main], :non_running_pipelines=>[]}

So the pipeline still starts, we see a 400 error, but no further details on the reason for the error.
This is because the discover of cluster uuid catches BadResponseCodeError exceptions but doesn't print the response_body in https://github.com/logstash-plugins/logstash-output-elasticsearch/blob/main/lib/logstash/plugin_mixins/elasticsearch/common.rb#L182.

I don't think the pipeline should continue if during register we know the api version isn't supported.

We could merge this now and create an issue to track handling of failures, but I don't think we'll come back to it, and error handling should be part of developing of the feature.

@kaisecheng
Copy link
Contributor Author

kaisecheng commented Sep 20, 2023

The updates handle custom_headers with an invalid version. The healthcheck! do two requests in the register phase. 1. health check request 2. fetch the info from main endpoint /. The second request is needed to get the failure reason as the first gives empty body.
The commits add response_body to log if data exists.

I have two thoughts on handling the failures

  1. We can add a test request in the register phase to make sure elastic api version in hand is useable. The plugin hardcode "2023-10-31" and I don't expect server returns failure for this version. A test request can make sure everything is set and can stop the pipeline fast when thing goes wrong. Currently, changing the version to "2024" in the source code won't shutdown the pipeline as it is after register. Edit: I will make the pipeline termination in another PR to unify the error behavior. When ConfigurationError is raised in finish_register(), the pipeline should stop.
  2. bulk request could get 400 invalid api version and retry indefinitely. Other 4xx and 5xx use the same retry mechanism. I don't think we want to shutdown the pipeline in this case.

@kaisecheng
Copy link
Contributor Author

summary: we decided to add a get "/" request in healthcheck! in register phase to make sure the first health check would fail when the header is invalid

Copy link
Contributor

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kaisecheng kaisecheng merged commit 0801b43 into logstash-plugins:main Sep 25, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants