Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on PR #12364: URL: https://github.com/apache/apisix/pull/12364#issuecomment-3007359024 > > > Do we need to remove `type` field in this PR? > > > > > > Yes. If we don't remove `type` field then we will need the compat headers that I removed > > pls update the doc too Removed `type` from docs -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on PR #12364: URL: https://github.com/apache/apisix/pull/12364#issuecomment-3007331614 > Do we need to remove `type` field in this PR? Yes. If we don't remove `type` field then we will need the compat headers that I removed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
membphis commented on PR #12364: URL: https://github.com/apache/apisix/pull/12364#issuecomment-3007333799 > > Do we need to remove `type` field in this PR? > > Yes. If we don't remove `type` field then we will need the compat headers that I removed pls update the doc too -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on code in PR #12364:
URL: https://github.com/apache/apisix/pull/12364#discussion_r2168170248
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -124,32 +124,94 @@ function _M.check_schema(conf, schema_type)
if schema_type == core.schema.TYPE_METADATA then
return core.schema.check(metadata_schema, conf)
end
+
local check = {"endpoint_addrs"}
core.utils.check_https(check, conf, plugin_name)
core.utils.check_tls_bool({"ssl_verify"}, conf, plugin_name)
-
return core.schema.check(schema, conf)
end
+local function get_es_major_version(uri, conf)
+local httpc = http.new()
+if not httpc then
+return nil, "failed to create http client"
+end
+local headers = {}
+if conf.auth then
+local authorization = "Basic " .. ngx.encode_base64(
+conf.auth.username .. ":" .. conf.auth.password
+)
+headers["Authorization"] = authorization
+end
+httpc:set_timeout(conf.timeout * 1000)
+local res, err = httpc:request_uri(uri, {
+ssl_verify = conf.ssl_verify,
+method = "GET",
+headers = headers,
+})
+if not res then
+return false, err
+end
+if res.status ~= 200 then
+return nil, str_format("server returned status: %d, body: %s",
+res.status, res.body or "")
+end
+local json_body, err = core.json.decode(res.body)
+if not json_body then
+return nil, "failed to decode response body: " .. err
+end
+if not json_body.version or not json_body.version.number then
+return nil, "failed to get version from response body"
+end
+
+local major_version = json_body.version.number:match("^(%d+)%.")
+if not major_version then
+return nil, "invalid version format: " .. json_body.version.number
+end
+
+return major_version
+end
+
+
local function get_logger_entry(conf, ctx)
local entry = log_util.get_log_entry(plugin_name, conf, ctx)
-return core.json.encode({
-create = {
-_index = conf.field.index,
-_type = conf.field.type
-}
-}) .. "\n" ..
+local body = {
+index = {
+_index = conf.field.index
+}
+}
+-- for older version type is required
+if conf._version == "6" or conf._version == "5" then
+body.index._type = "_doc"
+end
+return core.json.encode(body) .. "\n" ..
core.json.encode(entry) .. "\n"
end
+local function set_version(conf)
Review Comment:
done
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -124,32 +124,94 @@ function _M.check_schema(conf, schema_type)
if schema_type == core.schema.TYPE_METADATA then
return core.schema.check(metadata_schema, conf)
end
+
local check = {"endpoint_addrs"}
core.utils.check_https(check, conf, plugin_name)
core.utils.check_tls_bool({"ssl_verify"}, conf, plugin_name)
-
return core.schema.check(schema, conf)
end
+local function get_es_major_version(uri, conf)
+local httpc = http.new()
+if not httpc then
+return nil, "failed to create http client"
+end
+local headers = {}
+if conf.auth then
+local authorization = "Basic " .. ngx.encode_base64(
+conf.auth.username .. ":" .. conf.auth.password
+)
+headers["Authorization"] = authorization
+end
+httpc:set_timeout(conf.timeout * 1000)
+local res, err = httpc:request_uri(uri, {
+ssl_verify = conf.ssl_verify,
+method = "GET",
+headers = headers,
+})
+if not res then
+return false, err
+end
+if res.status ~= 200 then
+return nil, str_format("server returned status: %d, body: %s",
+res.status, res.body or "")
+end
+local json_body, err = core.json.decode(res.body)
+if not json_body then
+return nil, "failed to decode response body: " .. err
+end
+if not json_body.version or not json_body.version.number then
+return nil, "failed to get version from response body"
+end
+
+local major_version = json_body.version.number:match("^(%d+)%.")
+if not major_version then
+return nil, "invalid version format: " .. json_body.version.number
+end
+
+return major_version
+end
+
+
local function get_logger_entry(conf, ctx)
local entry = log_util.get_log_entry(plugin_name, conf, ctx)
-return core.json.encode({
-create = {
-_index = conf.field.index,
-_type = conf.field.type
-}
-}) .. "\n" ..
+local body = {
+index = {
+_index = conf.field.index
+}
+}
+-- for older version type is required
+if conf._version == "6" or conf._version == "5" then
+body.index._type = "_doc"
+end
+return core.json.encode(body) .. "\n" ..
core.json.encode(entry) .. "\n"
end
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on code in PR #12364:
URL: https://github.com/apache/apisix/pull/12364#discussion_r2168161940
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -159,9 +221,16 @@ local function send_to_elasticsearch(conf, entries)
local uri = selected_endpoint_addr .. "/_bulk"
local body = core.table.concat(entries, "")
local headers = {
-["Content-Type"] = "application/x-ndjson;compatible-with=7",
-["Accept"] = "application/vnd.elasticsearch+json;compatible-with=7"
+["Content-Type"] = "application/x-ndjson",
+["Accept"] = "application/vnd.elasticsearch+json"
}
+if conf._version == "8" then
+headers["Content-Type"] = headers["Content-Type"] .. compat_header_7
Review Comment:
removed it. not needed anymore
##
ci/pod/docker-compose.plugin.yml:
##
@@ -225,6 +225,66 @@ services:
http.port: 9201
xpack.security.enabled: 'true'
+ elasticsearch-noauth-2:
Review Comment:
i removed two of these containers(not needed)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on code in PR #12364: URL: https://github.com/apache/apisix/pull/12364#discussion_r2168161494 ## ci/pod/docker-compose.plugin.yml: ## @@ -225,6 +225,66 @@ services: http.port: 9201 xpack.security.enabled: 'true' + elasticsearch-noauth-2: +image: docker.elastic.co/elasticsearch/elasticsearch:9.0.2 +restart: unless-stopped +ports: + - "9400:9200" + - "9500:9300" +environment: + ES_JAVA_OPTS: -Xms512m -Xmx512m + discovery.type: single-node + xpack.security.enabled: 'false' + + elasticsearch-auth-2: +image: docker.elastic.co/elasticsearch/elasticsearch:9.0.2 +restart: unless-stopped +ports: + - "9301:9201" +environment: + ES_JAVA_OPTS: -Xms512m -Xmx512m + discovery.type: single-node + ELASTIC_USERNAME: elastic + ELASTIC_PASSWORD: 123456 + http.port: 9201 + xpack.security.enabled: 'true' + + elasticsearch-noauth-3: +image: docker.elastic.co/elasticsearch/elasticsearch:7.0.0 +restart: unless-stopped +ports: + - "9600:9200" + - "9700:9300" +environment: + ES_JAVA_OPTS: -Xms512m -Xmx512m + discovery.type: single-node + xpack.security.enabled: 'false' + + elasticsearch-auth-3: +image: docker.elastic.co/elasticsearch/elasticsearch:7.0.0 +restart: unless-stopped +ports: + - "9401:9201" +environment: + ES_JAVA_OPTS: -Xms512m -Xmx512m + discovery.type: single-node + ELASTIC_USERNAME: elastic + ELASTIC_PASSWORD: 123456 + http.port: 9201 + xpack.security.enabled: 'true' + + elasticsearch-auth-4: +image: docker.elastic.co/elasticsearch/elasticsearch:6.7.0 Review Comment: i removed two of these containers(not needed) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on code in PR #12364:
URL: https://github.com/apache/apisix/pull/12364#discussion_r2168129288
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -124,32 +124,94 @@ function _M.check_schema(conf, schema_type)
if schema_type == core.schema.TYPE_METADATA then
return core.schema.check(metadata_schema, conf)
end
+
local check = {"endpoint_addrs"}
core.utils.check_https(check, conf, plugin_name)
core.utils.check_tls_bool({"ssl_verify"}, conf, plugin_name)
-
return core.schema.check(schema, conf)
end
+local function get_es_major_version(uri, conf)
+local httpc = http.new()
+if not httpc then
+return nil, "failed to create http client"
+end
+local headers = {}
+if conf.auth then
+local authorization = "Basic " .. ngx.encode_base64(
+conf.auth.username .. ":" .. conf.auth.password
+)
+headers["Authorization"] = authorization
+end
+httpc:set_timeout(conf.timeout * 1000)
+local res, err = httpc:request_uri(uri, {
+ssl_verify = conf.ssl_verify,
+method = "GET",
+headers = headers,
+})
+if not res then
+return false, err
+end
+if res.status ~= 200 then
+return nil, str_format("server returned status: %d, body: %s",
+res.status, res.body or "")
+end
+local json_body, err = core.json.decode(res.body)
+if not json_body then
+return nil, "failed to decode response body: " .. err
+end
+if not json_body.version or not json_body.version.number then
+return nil, "failed to get version from response body"
+end
+
+local major_version = json_body.version.number:match("^(%d+)%.")
+if not major_version then
+return nil, "invalid version format: " .. json_body.version.number
+end
+
+return major_version
+end
+
+
local function get_logger_entry(conf, ctx)
local entry = log_util.get_log_entry(plugin_name, conf, ctx)
-return core.json.encode({
-create = {
-_index = conf.field.index,
-_type = conf.field.type
-}
-}) .. "\n" ..
+local body = {
+index = {
+_index = conf.field.index
+}
+}
+-- for older version type is required
+if conf._version == "6" or conf._version == "5" then
+body.index._type = "_doc"
+end
+return core.json.encode(body) .. "\n" ..
core.json.encode(entry) .. "\n"
end
+local function set_version(conf)
+if not conf._version then
+local selected_endpoint_addr
+if conf.endpoint_addr then
+selected_endpoint_addr = conf.endpoint_addr
+else
+selected_endpoint_addr =
conf.endpoint_addrs[math_random(#conf.endpoint_addrs)]
+end
+local major_version, err =
get_es_major_version(selected_endpoint_addr, conf)
+if err then
+return false, str_format("failed to get Elasticsearch version:
%s", err)
+end
+conf._version = major_version
+end
+end
+
local function send_to_elasticsearch(conf, entries)
local httpc, err = http.new()
if not httpc then
return false, str_format("create http error: %s", err)
end
-
+set_version(conf)
Review Comment:
this function is run later in batch processor. But we need the version in
the log phase when entry is created. That is why we need it in access phase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
membphis commented on code in PR #12364:
URL: https://github.com/apache/apisix/pull/12364#discussion_r2167915543
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -195,6 +264,11 @@ function _M.body_filter(conf, ctx)
log_util.collect_body(conf, ctx)
end
+function _M.access(conf)
+-- set_version will call ES server only the first time
+-- so this should not amount to considerable overhead
+set_version(conf)
Review Comment:
how about `fetch_and_update_es_version`?
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -159,9 +221,16 @@ local function send_to_elasticsearch(conf, entries)
local uri = selected_endpoint_addr .. "/_bulk"
local body = core.table.concat(entries, "")
local headers = {
-["Content-Type"] = "application/x-ndjson;compatible-with=7",
-["Accept"] = "application/vnd.elasticsearch+json;compatible-with=7"
+["Content-Type"] = "application/x-ndjson",
+["Accept"] = "application/vnd.elasticsearch+json"
}
+if conf._version == "8" then
+headers["Content-Type"] = headers["Content-Type"] .. compat_header_7
Review Comment:
can we remove `compat_header_7` and `compat_header_8`?
If yes, then we can move `if conf._version == "8" then` and `elseif
conf._version == "9" then`.
just use the default header, pls confirm if it is ok:
```
local headers = {
["Content-Type"] = "application/x-ndjson",
["Accept"] = "application/vnd.elasticsearch+json"
}
```
##
ci/pod/docker-compose.plugin.yml:
##
@@ -225,6 +225,66 @@ services:
http.port: 9201
xpack.security.enabled: 'true'
+ elasticsearch-noauth-2:
+image: docker.elastic.co/elasticsearch/elasticsearch:9.0.2
+restart: unless-stopped
+ports:
+ - "9400:9200"
+ - "9500:9300"
+environment:
+ ES_JAVA_OPTS: -Xms512m -Xmx512m
+ discovery.type: single-node
+ xpack.security.enabled: 'false'
+
+ elasticsearch-auth-2:
+image: docker.elastic.co/elasticsearch/elasticsearch:9.0.2
+restart: unless-stopped
+ports:
+ - "9301:9201"
+environment:
+ ES_JAVA_OPTS: -Xms512m -Xmx512m
+ discovery.type: single-node
+ ELASTIC_USERNAME: elastic
+ ELASTIC_PASSWORD: 123456
+ http.port: 9201
+ xpack.security.enabled: 'true'
+
+ elasticsearch-noauth-3:
+image: docker.elastic.co/elasticsearch/elasticsearch:7.0.0
+restart: unless-stopped
+ports:
+ - "9600:9200"
+ - "9700:9300"
+environment:
+ ES_JAVA_OPTS: -Xms512m -Xmx512m
+ discovery.type: single-node
+ xpack.security.enabled: 'false'
+
+ elasticsearch-auth-3:
+image: docker.elastic.co/elasticsearch/elasticsearch:7.0.0
+restart: unless-stopped
+ports:
+ - "9401:9201"
+environment:
+ ES_JAVA_OPTS: -Xms512m -Xmx512m
+ discovery.type: single-node
+ ELASTIC_USERNAME: elastic
+ ELASTIC_PASSWORD: 123456
+ http.port: 9201
+ xpack.security.enabled: 'true'
+
+ elasticsearch-auth-4:
+image: docker.elastic.co/elasticsearch/elasticsearch:6.7.0
Review Comment:
we started more instances of es
the github action, is it power enough to support this? I am little worried
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -124,32 +124,94 @@ function _M.check_schema(conf, schema_type)
if schema_type == core.schema.TYPE_METADATA then
return core.schema.check(metadata_schema, conf)
end
+
local check = {"endpoint_addrs"}
core.utils.check_https(check, conf, plugin_name)
core.utils.check_tls_bool({"ssl_verify"}, conf, plugin_name)
-
return core.schema.check(schema, conf)
end
+local function get_es_major_version(uri, conf)
+local httpc = http.new()
+if not httpc then
+return nil, "failed to create http client"
+end
+local headers = {}
+if conf.auth then
+local authorization = "Basic " .. ngx.encode_base64(
+conf.auth.username .. ":" .. conf.auth.password
+)
+headers["Authorization"] = authorization
+end
+httpc:set_timeout(conf.timeout * 1000)
+local res, err = httpc:request_uri(uri, {
+ssl_verify = conf.ssl_verify,
+method = "GET",
+headers = headers,
+})
+if not res then
+return false, err
+end
+if res.status ~= 200 then
+return nil, str_format("server returned status: %d, body: %s",
+res.status, res.body or "")
+end
+local json_body, err = core.json.decode(res.body)
+if not json_body then
+return nil, "failed to decode response body: " .. err
+end
+if not json_body.version or not json_body.version.number then
+return nil, "failed to get version from response body"
+end
+
+local major_version = json_body.version.number:match("^(%d+)%.")
+if not major_version then
+return nil, "invalid version format: " ..
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
nic-6443 commented on code in PR #12364:
URL: https://github.com/apache/apisix/pull/12364#discussion_r2167890351
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -124,32 +124,94 @@ function _M.check_schema(conf, schema_type)
if schema_type == core.schema.TYPE_METADATA then
return core.schema.check(metadata_schema, conf)
end
+
local check = {"endpoint_addrs"}
core.utils.check_https(check, conf, plugin_name)
core.utils.check_tls_bool({"ssl_verify"}, conf, plugin_name)
-
return core.schema.check(schema, conf)
end
+local function get_es_major_version(uri, conf)
+local httpc = http.new()
+if not httpc then
+return nil, "failed to create http client"
+end
+local headers = {}
+if conf.auth then
+local authorization = "Basic " .. ngx.encode_base64(
+conf.auth.username .. ":" .. conf.auth.password
+)
+headers["Authorization"] = authorization
+end
+httpc:set_timeout(conf.timeout * 1000)
+local res, err = httpc:request_uri(uri, {
+ssl_verify = conf.ssl_verify,
+method = "GET",
+headers = headers,
+})
+if not res then
+return false, err
+end
+if res.status ~= 200 then
+return nil, str_format("server returned status: %d, body: %s",
+res.status, res.body or "")
+end
+local json_body, err = core.json.decode(res.body)
+if not json_body then
+return nil, "failed to decode response body: " .. err
+end
+if not json_body.version or not json_body.version.number then
+return nil, "failed to get version from response body"
+end
+
+local major_version = json_body.version.number:match("^(%d+)%.")
+if not major_version then
+return nil, "invalid version format: " .. json_body.version.number
+end
+
+return major_version
+end
+
+
local function get_logger_entry(conf, ctx)
local entry = log_util.get_log_entry(plugin_name, conf, ctx)
-return core.json.encode({
-create = {
-_index = conf.field.index,
-_type = conf.field.type
-}
-}) .. "\n" ..
+local body = {
+index = {
+_index = conf.field.index
+}
+}
+-- for older version type is required
+if conf._version == "6" or conf._version == "5" then
+body.index._type = "_doc"
+end
+return core.json.encode(body) .. "\n" ..
core.json.encode(entry) .. "\n"
end
+local function set_version(conf)
+if not conf._version then
+local selected_endpoint_addr
+if conf.endpoint_addr then
+selected_endpoint_addr = conf.endpoint_addr
+else
+selected_endpoint_addr =
conf.endpoint_addrs[math_random(#conf.endpoint_addrs)]
+end
+local major_version, err =
get_es_major_version(selected_endpoint_addr, conf)
+if err then
+return false, str_format("failed to get Elasticsearch version:
%s", err)
+end
+conf._version = major_version
+end
+end
+
local function send_to_elasticsearch(conf, entries)
local httpc, err = http.new()
if not httpc then
return false, str_format("create http error: %s", err)
end
-
+set_version(conf)
Review Comment:
we already call `set_version` here, why still need it in `access` phase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
nic-6443 commented on code in PR #12364: URL: https://github.com/apache/apisix/pull/12364#discussion_r216754 ## ci/pod/docker-compose.plugin.yml: ## @@ -225,6 +225,66 @@ services: http.port: 9201 xpack.security.enabled: 'true' + elasticsearch-noauth-2: Review Comment: Has the auth method changed between these versions? If not, is it unnecessary to test the auth case separately for each version? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
nic-6443 commented on code in PR #12364: URL: https://github.com/apache/apisix/pull/12364#discussion_r2167887211 ## ci/pod/docker-compose.plugin.yml: ## @@ -225,6 +225,66 @@ services: http.port: 9201 xpack.security.enabled: 'true' + elasticsearch-noauth-2: Review Comment: Running so many Elasticsearch containers simultaneously in CI may strain computing resources, and causing tests to fail. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup commented on code in PR #12364: URL: https://github.com/apache/apisix/pull/12364#discussion_r2167030968 ## apisix/plugins/elasticsearch-logger.lua: ## @@ -195,6 +264,11 @@ function _M.body_filter(conf, ctx) log_util.collect_body(conf, ctx) end +function _M.access(conf) Review Comment: set_version calls ES server the first time and this http connection is not allowed in log phase so I had no other way but to add this here because version information is needed at the start of log phase when entries are being created. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: support multiple versions of Elasticsearch logger [apisix]
membphis commented on code in PR #12364:
URL: https://github.com/apache/apisix/pull/12364#discussion_r2161741034
##
apisix/plugins/elasticsearch-logger.lua:
##
@@ -156,12 +204,30 @@ local function send_to_elasticsearch(conf, entries)
else
selected_endpoint_addr =
conf.endpoint_addrs[math_random(#conf.endpoint_addrs)]
end
+if not conf._version then
+local major_version, err =
get_es_major_version(selected_endpoint_addr, conf)
+if err then
+return false, str_format("failed to get Elasticsearch version:
%s", err)
+end
+conf._version = major_version
+end
local uri = selected_endpoint_addr .. "/_bulk"
local body = core.table.concat(entries, "")
local headers = {
-["Content-Type"] = "application/x-ndjson;compatible-with=7",
-["Accept"] = "application/vnd.elasticsearch+json;compatible-with=7"
+["Content-Type"] = "application/x-ndjson",
+["Accept"] = "application/vnd.elasticsearch+json"
}
+if conf._version == "8" then
+headers["Content-Type"] = headers["Content-Type"] .. compat_header_7
+headers["Accept"] = headers["Accept"] .. compat_header_7
+elseif conf._version == "9" then
+headers["Content-Type"] = headers["Content-Type"] .. compat_header_8
+headers["Accept"] = headers["Accept"] .. compat_header_8
+if conf.field.type then
+core.log.warn("type is not supported in Elasticsearch 9, removing
`type`")
Review Comment:
I think we should use error log level
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
[PR] fix: support multiple versions of Elasticsearch logger [apisix]
Revolyssup opened a new pull request, #12364: URL: https://github.com/apache/apisix/pull/12364 Fixes #  It's the issue of header incompatibility. I have also reproduced the issue and the fix. If you're on X version of Elasticsearch server then you can allow requests coming from client with version X-1 by putting them in the header. This means server with version 8 can support requests from 7 with compatible-with=7. But when Elasticsearch server is version 9, this will break as it can only support compatible-with=8. This means our current implementation will need to support multiple versions and not hardcode to 7 or 8. Since our usage is pretty simple and we use the bulk API to POST logs, no change is needed in request body format. Although there is one change that I propose in the last point below. Proposed solution We should add a field version in the elasticsearch plugin config to allow users to configure which version they are using. Current supported versions will be 7,8,9 When user specifies 7, no compatibility headers needed. When user specifies 8, compatible-with=7 header. When user specifies 9, compatible-with=8 header. When user version 9, we should [not support the type field](https://github.com/apache/apisix/blob/7d5aeafdb442151667be95ad4ebc37cd3b1fa6bb/t/plugin/elasticsearch-logger.t#L692) as it is deprecated as shown in the docs. Till version 8, the problem can be avoided with compatible-with=7 heade but on version 9 we cant be compat with version 7 so it should be removed. Currently this field is set by plugin conf. We can enforce in schema that when Elasticsearch server version is 9, this field is not supported as it's deprecated. I have used the current tests with Elasticsearch version 9 to confirm the same error log as given in task. And by making the above changes, I can pass the tests. References: https://www.elastic.co/docs/reference/elasticsearch/rest-apis/compatibility https://www.elastic.co/guide/en/elasticsearch/reference/8.18/rest-api-compatibility.html https://github.com/elastic/elasticsearch-ruby/issues/2665 ### Checklist - [ ] I have explained the need for this PR and the problem it solves - [ ] I have explained the changes or the new features added to this PR - [ ] I have added tests corresponding to this change - [ ] I have updated the documentation to reflect this change - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
