feat(elasticsearch-logger): support multi elasticsearch endpoints #8604

xiaoxuanzi · 2023-01-04T06:59:08Z

Description

When the apisix load is heavy, there are many logs, and the endpoint load of a single elasticsearch is too heavy. The elasticsearch-logger plugin supports multiple endpoints to share the load.

Fixes (#8431)

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 first)

soulbird · 2023-01-04T07:12:08Z

apisix/plugins/elasticsearch-logger.lua

        endpoint_addr = {
            type = "string",
            pattern = "[^/]$",
        },
+        endpoint_addrs = {


Extending endpoint_addr in a compatible way, and adding a new configuration is not a good choice

Thank you for your reply.

What is your suggestion?
Can you give an example?
I refer to PR #7517

feat(clickhouse-logger): support multi clickhouse endpoints

Looking at this: https://github.com/apache/apisix/pull/7517/files#diff-d8d1670bb4d48884ee796f652172591cc371e5da972d7581ca585e60ddaab0f2R35, it also indicates the need to unify into one. You can refer to the definition of nodes_schema to allow endpoint_addrs to be set to multiple types: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L289

Looking at this, it also indicates the need to unify into one. You can refer to the definition of nodes_schema to allow endpoint_addrs to be set to multiple types: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L289

Thanks for the helpful advice!

Hi @soulbird.
I fixed the code according to your suggestion. please review it.

spacewander · 2023-01-28T06:37:01Z

docs/en/latest/plugins/elasticsearch-logger.md

@@ -53,6 +53,8 @@ This Plugin supports using batch processors to aggregate and process entries (lo

 ## Enabling the Plugin

+If multiple endpoints are configured, they will be written randomly.


This should be put into the attribute table

spacewander · 2023-01-28T06:38:44Z

apisix/plugins/elasticsearch-logger.lua

+local schema = {
+    type = "object",
+    properties = {
+        endpoint_addr = endpoint_schema,


Better to use endpoint_addrs with the new type.
Using different types in the same field brings us lots of trouble when we need to work with static-type language, like writing a Go client.

Hi @spacewander , thank you for your reply.
In my opinion, to use endpoint_addrs will cause trouble, this change is not backward compatible.

The code submitted for the first time is modified as follows:

local schema = { type = "object", properties = { + -- deprecated, use "endpoint_addrs" instead endpoint_addr = { type = "string", pattern = "[^/]$", }, + endpoint_addrs = { + type = "array", + minItems = 1, + items = { + type = "string", + pattern = "[^/]$", + }, + },

I fixed the code according to @soulbird's suggestion.
#8604 (comment)

Keep endpoint_addr, add a new field endpoint_addrs to support both string and array better

this change is not backward compatible

I am confused. People can still use the old endpoint_addr configuration.

Keep endpoint_addr, add a new field endpoint_addrs to support both string and array better

It is not a good idea to have a field of multiple types. The upstream's nodes field is an example. People keep complaining that this field doesn't have a consistent type.

Understood. So we need to keep both endpoint_addr and endpoint_addrs, where endpoint_addrs is only specified as array type, is that right?

spacewander · 2023-01-28T06:40:55Z

docs/en/latest/plugins/elasticsearch-logger.md

@@ -37,7 +37,7 @@ When the Plugin is enabled, APISIX will serialize the request context informatio

 | Name          | Type    | Required | Default                     | Description                                                  |
 | ------------- | ------- | -------- | --------------------------- | ------------------------------------------------------------ |
-| endpoint_addr | string  | True     |                             | Elasticsearch API.                                            |
+| endpoint_addr | string/array  | True     |                             | Elasticsearch API.                                            |


Let's add a deprecated mark like https://github.com/apache/apisix/blob/release/2.15/docs/en/latest/plugins/mqtt-proxy.md#attributes

spacewander · 2023-01-28T06:41:33Z

t/plugin/elasticsearch-logger.t

+        }
+    }
+--- response_body
+passed


We need to add a test to show that different endpoints will be chosen randomly.

I agree with you, but I don't know how.
Can you give an example?
I take PR #7517 as reference.
=== TEST 5: add plugin on routes using multi clickhouse-logger

What about hitting the path multiple times and triggering multiple sending?
Like https://github.com/apache/apisix/blob/master/docs/en/latest/internal/testing-framework.md#send-request
Then we can check if all the endpoints are used via error log.

Hi @spacewander.
I fixed the code according to your suggestion. Please review it.

spacewander · 2023-01-31T02:47:28Z

t/plugin/elasticsearch-logger.t

+--- response_body
+code: 200
+code: 200
+code: 200


We can count the code like

apisix/t/node/healthcheck2.t

Line 139 in 22be6d0

[{"count":12,"port":"1980"}]

* upstream/master: feat(elasticsearch-logger): support multi elasticsearch endpoints (apache#8604) chore: use operator # instead of string.len (apache#8751) chore: hi 2023 (apache#8748) refactor(admin): stream_routes/upstreams/protos/services/global_rules/consumer_groups/plugin_configs (apache#8661) feat: support send error-log to kafka brokers (apache#8693) chore: upgrade `casbin` to `1.41.5` (apache#8744)

soulbird reviewed Jan 4, 2023

View reviewed changes

support multi elasticsearch endpoints

7878c23

xiaoxuanzi force-pushed the master branch from a373cbc to 7878c23 Compare January 9, 2023 08:36

soulbird previously approved these changes Jan 19, 2023

View reviewed changes

spacewander reviewed Jan 28, 2023

View reviewed changes

xiaoxuanzi added 2 commits January 29, 2023 16:20

add a new field endpoint_addrs

fb5cb48

add a test to show that different endpoints will be chosen randomly

c009c92

xiaoxuanzi dismissed soulbird’s stale review via c009c92 January 30, 2023 06:07

spacewander reviewed Jan 31, 2023

View reviewed changes

adjusted TEST 14

3ab6051

soulbird approved these changes Jan 31, 2023

View reviewed changes

spacewander approved these changes Feb 1, 2023

View reviewed changes

spacewander changed the title ~~feat(elasticsearch-logger): support multi elasticsearch endpoints(#8431)~~ feat(elasticsearch-logger): support multi elasticsearch endpoints Feb 1, 2023

spacewander merged commit 0e2b925 into apache:master Feb 1, 2023

xiaoxuanzi mentioned this pull request Feb 2, 2023

test(clickhouse-logger): to show that different endpoints will be chosen randomly #8777

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticsearch-logger): support multi elasticsearch endpoints #8604

feat(elasticsearch-logger): support multi elasticsearch endpoints #8604

xiaoxuanzi commented Jan 4, 2023 •

edited

Loading

soulbird Jan 4, 2023

xiaoxuanzi Jan 4, 2023

soulbird Jan 5, 2023 •

edited

Loading

xiaoxuanzi Jan 9, 2023 •

edited

Loading

spacewander Jan 28, 2023

spacewander Jan 28, 2023

xiaoxuanzi Jan 28, 2023 •

edited

Loading

soulbird Jan 29, 2023

spacewander Jan 29, 2023

soulbird Jan 30, 2023

spacewander Jan 30, 2023

spacewander Jan 28, 2023

spacewander Jan 28, 2023

xiaoxuanzi Jan 28, 2023

spacewander Jan 29, 2023

xiaoxuanzi Jan 30, 2023 •

edited

Loading

spacewander Jan 31, 2023

xiaoxuanzi Jan 31, 2023

		@@ -53,6 +53,8 @@ This Plugin supports using batch processors to aggregate and process entries (lo

		## Enabling the Plugin

		If multiple endpoints are configured, they will be written randomly.

feat(elasticsearch-logger): support multi elasticsearch endpoints #8604

feat(elasticsearch-logger): support multi elasticsearch endpoints #8604

Conversation

xiaoxuanzi commented Jan 4, 2023 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soulbird Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

xiaoxuanzi Jan 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiaoxuanzi Jan 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiaoxuanzi Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiaoxuanzi commented Jan 4, 2023 •

edited

Loading

soulbird Jan 5, 2023 •

edited

Loading

xiaoxuanzi Jan 9, 2023 •

edited

Loading

xiaoxuanzi Jan 28, 2023 •

edited

Loading

xiaoxuanzi Jan 30, 2023 •

edited

Loading