feat: clickhouse logger by qizhendong1 · Pull Request #6215 · apache/apisix

qizhendong1 · 2022-01-27T06:25:06Z

What this PR does / why we need it:

feat: push access log to clickhouse DB

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

qizhendong1 · 2022-01-29T06:54:13Z

Hi, @spacewander
How fix this test case ?
https://github.com/apache/apisix/runs/4989817705?check_suite_focus=true#step:7:22

Installing collected packages: zhon
Successfully installed zhon-1.1.5
find broken newline in file: docs/zh/latest/plugins/clickhouse-logger.md
cat: /tmp/error.log: No such file or directory
Error: Process completed with exit code 1.

spacewander · 2022-01-30T03:31:02Z

Hi, @spacewander How fix this test case ? apache/apisix/runs/4989817705?check_suite_focus=true#step:7:22
Installing collected packages: zhon
Successfully installed zhon-1.1.5
find broken newline in file: docs/zh/latest/plugins/clickhouse-logger.md
cat: /tmp/error.log: No such file or directory
Error: Process completed with exit code 1.

Sorry, it is the bug in our lint script. Fixed in #6239

tzssangglass · 2022-02-07T08:24:22Z

hi @zhendongcmss CI is broken

qizhendong1 · 2022-02-07T09:09:54Z

hi @zhendongcmss CI is broken

@spacewander @tzssangglass
sorry， I don't know how to fix. I need help, can you tell me how to fix it ?

#   Failed test 'TEST 4: add plugin on routes - status code ok'
#   at /apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
#          got: '401'
#     expected: '200'

#   Failed test 'TEST 5: access local server - status code ok'
#   at /apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
#          got: '404'
#     expected: '200'

#   Failed test 'TEST 5: access local server - response_body - response is expected (repeated req 0, req 0)'
#   at /apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1589.
#          got: '{"error_msg":"404 Route Not Found"}
# '
#     expected: 'opentracing

tzssangglass · 2022-02-08T02:27:51Z

hi @zhendongcmss CI is broken

@spacewander @tzssangglass sorry， I don't know how to fix. I need help, can you tell me how to fix it ?

#   Failed test 'TEST 4: add plugin on routes - status code ok'
#   at /apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
#          got: '401'
#     expected: '200'

#   Failed test 'TEST 5: access local server - status code ok'
#   at /apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
#          got: '404'
#     expected: '200'

#   Failed test 'TEST 5: access local server - response_body - response is expected (repeated req 0, req 0)'
#   at /apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1589.
#          got: '{"error_msg":"404 Route Not Found"}
# '
#     expected: 'opentracing

My comments above solved the 401 problem with test 4, you can continue to see the problem with test 5, which is related to the code and configuration.

spacewander · 2022-02-14T06:45:11Z

+"clickhouse body: INSERT INTO t FORMAT JSONEachRow"
+"clickhouse headers: X-ClickHouse-Key:a"
+"clickhouse headers: X-ClickHouse-User:default"
+"clickhouse headers: X-ClickHouse-Database:default"


I have reconsidered the test.
What about using error_log like

apisix/t/plugin/loggly.t

Lines 572 to 574 in 537a8da

--- error_log

loggly body: {"route_id":"1"}

loggly tags: "apisix"

?

It doesn't work.

It doesn't work. I don't know why. I think --- grep_error_log_out is easier to use.

Use --- error_log fialed. https://github.com/apache/apisix/runs/5197064703?check_suite_focus=true#step:14:891

spacewander · 2022-02-15T03:38:27Z

+                            },
+                            "plugins":{
+                                "clickhouse-logger":{
+                                    "batch_max_size":1000,


Suggested change

"batch_max_size":1000,

"batch_max_size":1,

So the report can be triggered in the CI

I thought there was a timer that called this callback function periodically and also if the batch_max_size value was reached. But I added a wait of --- wait: 5 below, it seem doesn'twork.

spacewander

Could you apply this patch:

--- t/plugin/clickhouse-logger.t
+++ t/plugin/clickhouse-logger.t
@@ -149,7 +149,9 @@ property "endpoint_addr" is required
                                 "password": "a",
                                 "database": "default",
                                 "logtable": "t",
-                                "endpoint_addr": "http://127.0.0.1:10420/clickhouse-logger/test"
+                                "endpoint_addr": "http://127.0.0.1:10420/clickhouse-logger/test",
+                                "batch_max_size":1,
+                                "inactive_timeout":1
                             }
                         },
                         "upstream": {
@@ -173,7 +175,7 @@ property "endpoint_addr" is required
                             },
                             "plugins":{
                                 "clickhouse-logger":{
-                                    "batch_max_size":1000,
+                                    "batch_max_size":1,
                                     "max_retry_count":0,
                                     "retry_delay":1,
                                     "ssl_verify":true,
@@ -185,7 +187,7 @@ property "endpoint_addr" is required
                                     "name":"clickhouse-logger",
                                     "database":"default",
                                     "logtable":"t",
-                                    "inactive_timeout":5
+                                    "inactive_timeout":1
                                 }
                             },
                             "id":"1"
@@ -213,7 +215,7 @@ GET /opentracing
 opentracing
 --- error_log
 clickhouse body: INSERT INTO t FORMAT JSONEachRow
-clickhouse headers: X-ClickHouse-Key:a
-clickhouse headers: X-ClickHouse-User:default
-clickhouse headers: X-ClickHouse-Database:default
---- wait: 5
+clickhouse headers: x-clickhouse-key:a
+clickhouse headers: x-clickhouse-user:default
+clickhouse headers: x-clickhouse-database:default
+--- wait: 2

Now the test should pass.
There is another issue, the headers we get are all lowercase.

spacewander · 2022-02-16T13:02:46Z

@zhendongcmss
Let's add the logger in https://github.com/apache/apisix/blob/master/README.md!

juzhiyuan · 2022-02-17T07:01:08Z

+
+## Attributes
+
+| 名称             | 类型    | 必选项  | 默认值         | 有效值  | 描述                                             |


Hi @zhendongcmss, here have some Chinese 😄 Would you like to translate them?

Hi, @yzeng25, do you have interest to do this? 🤗

I created an issue, let's fix it: #6414

qizhendong added 4 commits January 27, 2022 14:22

feat: clickhouse logger

a2d395b

fix code lint error

234ac04

fix code lint error

26830e5

add doc on sidebar config.json

ac2a148

qizhendong1 marked this pull request as ready for review January 27, 2022 07:48

shuaijinchao reviewed Jan 27, 2022

View reviewed changes

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

Comment thread conf/config-default.yaml Outdated

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

spacewander reviewed Jan 28, 2022

View reviewed changes

Comment thread docs/en/latest/plugins/clickhouse-logger.md Outdated

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

Comment thread t/plugin/clickhouse-logger.t Outdated

qizhendong added 2 commits January 28, 2022 11:27

using batch process manager

5a3ed37

fix misspell

bdd1893

tzssangglass reviewed Jan 28, 2022

View reviewed changes

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

Comment thread apisix/plugins/clickhouse-logger.lua

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

qizhendong1 commented Jan 28, 2022

View reviewed changes

Comment thread apisix/plugins/clickhouse-logger.lua

qizhendong added 3 commits January 28, 2022 14:35

add ssl verify

d9a8985

using clickhouse-logger metadata

c268d5c

fix test cases fail

f234955

fix test case 'auth configure undefined'

e0b5f3d

spacewander reviewed Jan 30, 2022

View reviewed changes

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

Comment thread docs/en/latest/plugins/clickhouse-logger.md Outdated

fix test case 'auth configure undefined'

27995aa

qizhendong1 force-pushed the clickhouse-logger branch from 6d728a2 to c47613a Compare February 2, 2022 14:07

use batch processer manager wrap

74d59b5

qizhendong1 force-pushed the clickhouse-logger branch from c47613a to 74d59b5 Compare February 3, 2022 01:22

spacewander reviewed Feb 6, 2022

View reviewed changes

Comment thread conf/config-default.yaml Outdated

Comment thread t/plugin/clickhouse-logger.t Outdated

Comment thread t/plugin/clickhouse-logger.t

enable clickhouse-logger by default

d4a9c2c

qizhendong1 requested a review from spacewander February 7, 2022 03:21

tzssangglass reviewed Feb 8, 2022

View reviewed changes

Comment thread docs/zh/latest/plugins/clickhouse-logger.md Outdated

tzssangglass reviewed Feb 8, 2022

View reviewed changes

Comment thread t/plugin/clickhouse-logger.t Outdated

Comment thread t/plugin/clickhouse-logger.t

starsz reviewed Feb 11, 2022

View reviewed changes

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

fix doc and test cases

22c3fb0

qizhendong1 force-pushed the clickhouse-logger branch from cfdc24f to 22c3fb0 Compare February 11, 2022 06:30

spacewander reviewed Feb 11, 2022

View reviewed changes

Comment thread t/plugin/clickhouse-logger.t Outdated

Comment thread t/plugin/clickhouse-logger.t Outdated

qizhendong1 force-pushed the clickhouse-logger branch from d8ef45b to daedfd1 Compare February 11, 2022 13:28

using reg tp match response body

34f3dcb

qizhendong1 force-pushed the clickhouse-logger branch from daedfd1 to 34f3dcb Compare February 12, 2022 12:59

spacewander reviewed Feb 13, 2022

View reviewed changes

Comment thread t/plugin/clickhouse-logger.t Outdated

Comment thread t/plugin/clickhouse-logger.t

starsz reviewed Feb 14, 2022

View reviewed changes

Comment thread apisix/plugins/clickhouse-logger.lua Outdated

qizhendong1 force-pushed the clickhouse-logger branch from b8852fa to bc00ad3 Compare February 14, 2022 02:50

spacewander reviewed Feb 14, 2022

View reviewed changes

verify clickhouse headers on test cases

9118fb0

qizhendong1 force-pushed the clickhouse-logger branch from bc00ad3 to 9118fb0 Compare February 14, 2022 09:53

spacewander reviewed Feb 15, 2022

View reviewed changes

qizhendong1 force-pushed the clickhouse-logger branch from 041d8b3 to 4ad14a0 Compare February 15, 2022 09:52

tzssangglass previously approved these changes Feb 16, 2022

View reviewed changes

spacewander previously approved these changes Feb 16, 2022

View reviewed changes

qizhendong1 dismissed stale reviews from spacewander and tzssangglass via 7f67ae3 February 16, 2022 07:18

qizhendong1 force-pushed the clickhouse-logger branch from 4ad14a0 to 7f67ae3 Compare February 16, 2022 07:18

spacewander reviewed Feb 16, 2022

View reviewed changes

Comment thread t/plugin/clickhouse-logger.t Outdated

qizhendong1 force-pushed the clickhouse-logger branch from 7f67ae3 to 14e29f5 Compare February 16, 2022 08:53

set batch_max_size to 1 on test cases

cede016

qizhendong1 force-pushed the clickhouse-logger branch from 14e29f5 to cede016 Compare February 16, 2022 09:49

spacewander approved these changes Feb 16, 2022

View reviewed changes

starsz approved these changes Feb 16, 2022

View reviewed changes

spacewander merged commit a933c0b into apache:master Feb 16, 2022

juzhiyuan reviewed Feb 17, 2022

View reviewed changes

mangoGoForward mentioned this pull request Feb 22, 2022

docs: change to English version #6414

Closed

	--- error_log
	loggly body: {"route_id":"1"}
	loggly tags: "apisix"


		## Attributes

		\| 名称 \| 类型 \| 必选项 \| 默认值 \| 有效值 \| 描述 \|

Uh oh!

Conversation

qizhendong1 commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Pre-submission checklist:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qizhendong1 commented Jan 29, 2022

Uh oh!

spacewander commented Jan 30, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tzssangglass commented Feb 7, 2022

Uh oh!

qizhendong1 commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tzssangglass commented Feb 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spacewander Feb 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

spacewander Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

spacewander left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spacewander commented Feb 16, 2022

Uh oh!

juzhiyuan Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

juzhiyuan Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

membphis Feb 22, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 commented Jan 27, 2022 •

edited

Loading

qizhendong1 commented Feb 7, 2022 •

edited

Loading

spacewander Feb 14, 2022 •

edited

Loading

juzhiyuan Feb 22, 2022 •

edited

Loading