feat(error-log-logger): add clickhouse for error-log-logger by qizhendong1 · Pull Request #6256 · apache/apisix

qizhendong1 · 2022-02-07T06:57:04Z

What this PR does / why we need it:

Add error-log-logger supports clickhouse.

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

tzssangglass · 2022-02-18T04:20:59Z

Now that we have the clickhouse plugin, can we implement it in this plugin?

I am concerned that this will introduce duplicate code.

spacewander · 2022-02-18T06:04:29Z

+## How to set the clickhouse
+
+The plugin sends the error log as a string to the `data` field of the clickhouse table.
+Step: update the attributes of the plugin


What does this sentence mean?

it's means that the plugins insert error logs to the clickhouse table with data column default.

┌─data──────────────────────────────────────────────────────────────────┬ │ 2022/02/16 16:45:07 [error] 4279#4279: *7272 [lua] batch-processor.lua:73: Batch ... │ └───────────────────────────────────────────────────────────────────────┴

spacewander · 2022-02-18T06:06:12Z

+GET /tg
+--- response_body
+--- error_log
+clickhouse error log body: INSERT INTO t FORMAT JSONEachRow


Is it possible to verify the reported error log in the test?

It is difficult because the error logs are different each other.
clickhouse error log body: INSERT INTO t FORMAT JSONEachRow {"data": " 2022/02/16 16:45:07 [error] 4279#4279: *7272 [lua] batch-processor.lua:73: Batch ... "}

qizhendong1 · 2022-02-18T09:43:58Z

Now that we have the clickhouse plugin, can we implement it in this plugin?

I am concerned that this will introduce duplicate code.

I think it is a good idea. One side we can reduce duplicate code, antoher avoid error-log-logger.lua oversize. It is friendly for new plugin.

@spacewander How about you ？

spacewander · 2022-02-20T10:30:35Z

No.
@tzssangglass
Handling the error log and access log are two different things. The former needs to be configured at the global level, the latter can be done in a smaller scope.

tzssangglass · 2022-02-20T12:43:01Z

The former needs to be configured at the global level

I got. Let's keep it that way for now and leave the optimization for later.

tzssangglass · 2022-02-21T03:34:11Z

+GET /tg
+--- response_body
+--- error_log
+clickhouse error log body: INSERT INTO t FORMAT JSONEachRow


is there log body releated to "this is a warning message for test5."?

yes, I will add it.

qizhendong1 · 2022-02-22T03:11:41Z

There is a test case failed. How to fix it ? @spacewander @tzssangglass @leslie-tsang
https://github.com/apache/apisix/runs/5282480987?check_suite_focus=true#step:6:4

Run ./utils/check-category.py
Entry docs/zh/latest/plugins/datadog.md in the sidebar can't be found. Please remove it from docs/zh/latest/config.json.
Error: Process completed with exit code 2[5](https://github.com/apache/apisix/runs/5282480987?check_suite_focus=true#step:6:5)5.

spacewander · 2022-02-22T03:25:34Z

Thanks for your report. I am fixing it via #6411

leslie-tsang · 2022-02-25T03:22:30Z

Hello there, Plz don't force push during the review.

Commit history allows reviewers to see what has changed since the last review, which is useful for them.

A forced push could ruin the review experience.

tzssangglass · 2022-02-28T02:24:52Z

LGTM, @zhendongcmss CI/build was failed, I will rerun it and if it still fails, we need to focus on it.

qizhendong added 7 commits February 2, 2022 21:46

feat: clickhouse error logger

d4eb386

add test cases and english doc

56b3f7e

refine code

5924e89

fix test cases and code style

cc38be7

fix test cases

c07c16b

fix ssl handshake

8186203

fix test case errors

b69cea1

qizhendong1 force-pushed the error-clickhouse-logger1 branch 2 times, most recently from cbb5f53 to a7bff92 Compare February 14, 2022 07:32

fix test cases

ce3a6d6

qizhendong1 force-pushed the error-clickhouse-logger1 branch from a7bff92 to ce3a6d6 Compare February 14, 2022 07:35

use error_log replace grep_error_log

5907d69

qizhendong1 marked this pull request as ready for review February 17, 2022 05:44

qizhendong added 2 commits February 18, 2022 09:37

remove ssl verify judgment

d08b328

using core.table module

f0f8e30

qizhendong1 force-pushed the error-clickhouse-logger1 branch from 214de6f to f0f8e30 Compare February 18, 2022 01:54

tzssangglass reviewed Feb 18, 2022

View reviewed changes

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

tzssangglass changed the title ~~feat(error-log-logger): add clickouse for error-log-logger~~ feat(error-log-logger): add clickhouse for error-log-logger Feb 18, 2022

spacewander reviewed Feb 18, 2022

View reviewed changes

qizhendong added 2 commits February 21, 2022 10:26

use httpc:request_uri

d9ecb5f

use httpc:request_uri

f2adaeb

tzssangglass reviewed Feb 21, 2022

View reviewed changes

leslie-tsang requested changes Feb 21, 2022

View reviewed changes

qizhendong1 force-pushed the error-clickhouse-logger1 branch from 1e72ba6 to a3f86d1 Compare February 22, 2022 02:43

qizhendong1 force-pushed the error-clickhouse-logger1 branch from a3f86d1 to b99e156 Compare February 23, 2022 06:27

qizhendong1 force-pushed the error-clickhouse-logger1 branch from b99e156 to 091c721 Compare February 25, 2022 06:08

spacewander requested a review from tzssangglass February 27, 2022 12:21

spacewander previously approved these changes Feb 28, 2022

View reviewed changes

Comment thread docs/zh/latest/plugins/error-log-logger.md

update test cases

aaaca0e

qizhendong1 dismissed spacewander’s stale review via aaaca0e February 28, 2022 06:38

qizhendong1 force-pushed the error-clickhouse-logger1 branch from 091c721 to aaaca0e Compare February 28, 2022 06:38

spacewander approved these changes Feb 28, 2022

View reviewed changes

tzssangglass approved these changes Feb 28, 2022

View reviewed changes

spacewander merged commit b41a2e0 into apache:master Feb 28, 2022

Uh oh!

Conversation

qizhendong1 commented Feb 7, 2022

What this PR does / why we need it:

Pre-submission checklist:

Uh oh!

Uh oh!

tzssangglass commented Feb 18, 2022

Uh oh!

Uh oh!

spacewander Feb 18, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 18, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 18, 2022

Choose a reason for hiding this comment

Uh oh!

spacewander Feb 18, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 21, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 commented Feb 18, 2022

Uh oh!

spacewander commented Feb 20, 2022

Uh oh!

tzssangglass commented Feb 20, 2022

Uh oh!

Uh oh!

Uh oh!

tzssangglass Feb 21, 2022

Choose a reason for hiding this comment

Uh oh!

qizhendong1 Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

spacewander commented Feb 22, 2022

Uh oh!

leslie-tsang commented Feb 25, 2022

Uh oh!

tzssangglass commented Feb 28, 2022

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qizhendong1 commented Feb 22, 2022 •

edited

Loading