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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 4 commits into from
Feb 1, 2023
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7878c23
support multi elasticsearch endpoints
xiaoxuanzi Jan 3, 2023
fb5cb48
add a new field endpoint_addrs
xiaoxuanzi Jan 29, 2023
c009c92
add a test to show that different endpoints will be chosen randomly
xiaoxuanzi Jan 30, 2023
3ab6051
adjusted TEST 14
xiaoxuanzi Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions apisix/plugins/elasticsearch-logger.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,34 +24,27 @@ local plugin = require("apisix.plugin")
local ngx = ngx
local str_format = core.string.format
local math_random = math.random
local type = type

local plugin_name = "elasticsearch-logger"
local batch_processor_manager = bp_manager_mod.new(plugin_name)


local endpoint_schema = {
anyOf = {
{
-- deprecated, use "array" instead
local schema = {
type = "object",
properties = {
-- deprecated, use "endpoint_addrs" instead
endpoint_addr = {
type = "string",
pattern = "[^/]$",
},
{
endpoint_addrs = {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@soulbird soulbird Jan 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@xiaoxuanzi xiaoxuanzi Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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.

type = "array",
minItems = 1,
items = {
type = "string",
pattern = "[^/]$",
},
}
}
}

local schema = {
type = "object",
properties = {
endpoint_addr = endpoint_schema,
},
field = {
type = "object",
properties = {
Expand Down Expand Up @@ -85,7 +78,10 @@ local schema = {
}
},
encrypt_fields = {"auth.password"},
required = {"endpoint_addr", "field"},
oneOf = {
{required = {"endpoint_addr", "field"}},
{required = {"endpoint_addrs", "field"}}
},
}


Expand Down Expand Up @@ -145,10 +141,10 @@ local function send_to_elasticsearch(conf, entries)
end

local selected_endpoint_addr
if type(conf.endpoint_addr) == "string" then
if conf.endpoint_addr then
selected_endpoint_addr = conf.endpoint_addr
else
selected_endpoint_addr = conf.endpoint_addr[math_random(#conf.endpoint_addr)]
selected_endpoint_addr = conf.endpoint_addrs[math_random(#conf.endpoint_addrs)]
end
local uri = selected_endpoint_addr .. "/_bulk"
local body = core.table.concat(entries, "")
Expand Down
5 changes: 2 additions & 3 deletions docs/en/latest/plugins/elasticsearch-logger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ When the Plugin is enabled, APISIX will serialize the request context informatio

| Name | Type | Required | Default | Description |
| ------------- | ------- | -------- | --------------------------- | ------------------------------------------------------------ |
| endpoint_addr | string/array | True | | Elasticsearch API. |
| endpoint_addr | string | Deprecated | | Deprecated. Use `endpoint_addrs` instead. Elasticsearch API. |
| endpoint_addrs | array | True | | Elasticsearch API. If multiple endpoints are configured, they will be written randomly. |
| field | array | True | | Elasticsearch `field` configuration. |
| field.index | string | True | | Elasticsearch [_index field](https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-index-field.html#mapping-index-field). |
| field.type | string | False | Elasticsearch default value | Elasticsearch [_type field](https://www.elastic.co/guide/en/elasticsearch/reference/7.17/mapping-type-field.html#mapping-type-field). |
Expand All @@ -53,8 +54,6 @@ 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.

### Full configuration

The example below shows a complete configuration of the Plugin on a specific Route:
Expand Down
9 changes: 2 additions & 7 deletions docs/zh/latest/plugins/elasticsearch-logger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ description: 本文介绍了 API 网关 Apache APISIX 的 elasticsearch-logger

| 名称 | 类型 | 必选项 | 默认值 | 描述 |
| ------------- | ------- | -------- | -------------------- | ------------------------------------------------------------ |
| endpoint_addr | string/array | 是 | | Elasticsearch API。 |
| endpoint_addr | string | 废弃 | | Elasticsearch API。推荐使用 `endpoint_addrs` |
| endpoint_addrs | array | 是 | | Elasticsearch API。如果配置多个 `endpoints`，日志将会随机写入到各个 `endpoints`。 |
| field | array | 是 | | Elasticsearch `field`配置信息。 |
| field.index | string | 是 | | Elasticsearch `[_index field](https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-index-field.html#mapping-index-field)`。 |
| field.type | string | 否 | Elasticsearch 默认值 | Elasticsearch `[_type field](https://www.elastic.co/guide/en/elasticsearch/reference/7.17/mapping-type-field.html#mapping-type-field)` |
Expand Down Expand Up @@ -93,12 +94,6 @@ curl http://127.0.0.1:9180/apisix/admin/routes/1 \
}'
```

:::note 注意

如果配置多个 `endpoints`，日志将会随机写入到各个 `endpoints`。

:::

### 最小化配置示例

```shell
Expand Down
37 changes: 33 additions & 4 deletions t/plugin/elasticsearch-logger.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ __DATA__
--- response_body_like
passed
passed
property "endpoint_addr" is required
property "field" is required
value should match only one schema, but matches none
value should match only one schema, but matches none
property "field" validation failed: property "index" is required
property "endpoint_addr" validation failed: object matches none of the required
property "endpoint_addr" validation failed: failed to match pattern "\[\^/\]\$" with "http://127.0.0.1:9200/"



Expand Down Expand Up @@ -533,7 +533,7 @@ PTQvJEaPcNOXcOHeErC0XQ==
},
plugins = {
["elasticsearch-logger"] = {
endpoint_addr = {"http://127.0.0.1:9200", "http://127.0.0.1:9201"},
endpoint_addrs = {"http://127.0.0.1:9200", "http://127.0.0.1:9201"},
field = {
index = "services"
},
Expand All @@ -551,3 +551,32 @@ PTQvJEaPcNOXcOHeErC0XQ==
}
--- response_body
passed
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@xiaoxuanzi xiaoxuanzi Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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




=== TEST 14: to show that different endpoints will be chosen randomly
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
for i = 1, 10 do
local code, body = t('/hello', ngx.HTTP_GET)
ngx.say("code: ", code)
end

}
}
--- response_body
code: 200
code: 200
code: 200
Copy link
Member

Choose a reason for hiding this comment

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

We can count the code like

apisix/t/node/healthcheck2.t

Line 139 in 22be6d0

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

code: 200
code: 200
code: 200
code: 200
code: 200
code: 200
code: 200
--- error_log
http://127.0.0.1:9200/_bulk
http://127.0.0.1:9201/_bulk