-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
feat(error-log-logger): add clickhouse for error-log-logger #6256
feat(error-log-logger): add clickhouse for error-log-logger #6256
Conversation
cbb5f53
to
a7bff92
Compare
a7bff92
to
ce3a6d6
Compare
214de6f
to
f0f8e30
Compare
Now that we have the clickhouse plugin, can we implement it in this plugin? I am concerned that this will introduce duplicate code. |
## 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this sentence mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's means that the plugins insert error logs to the clickhouse table with data
column default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
┌─data──────────────────────────────────────────────────────────────────┬
│ 2022/02/16 16:45:07 [error] 4279#4279: *7272 [lua] batch-processor.lua:73: Batch ... │
└───────────────────────────────────────────────────────────────────────┴
GET /tg | ||
--- response_body | ||
--- error_log | ||
clickhouse error log body: INSERT INTO t FORMAT JSONEachRow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to verify the reported error log in the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ... "}
I think it is a good idea. One side we can reduce duplicate code, antoher avoid @spacewander How about you ? |
No. |
I got. Let's keep it that way for now and leave the optimization for later. |
GET /tg | ||
--- response_body | ||
--- error_log | ||
clickhouse error log body: INSERT INTO t FORMAT JSONEachRow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there log body releated to "this is a warning message for test5."
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I will add it.
1e72ba6
to
a3f86d1
Compare
There is a test case failed. How to fix it ? @spacewander @tzssangglass @leslie-tsang
|
Thanks for your report. I am fixing it via #6411 |
a3f86d1
to
b99e156
Compare
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. |
b99e156
to
091c721
Compare
LGTM, @zhendongcmss CI/build was failed, I will rerun it and if it still fails, we need to focus on it. |
091c721
to
aaaca0e
Compare
What this PR does / why we need it:
Add error-log-logger supports clickhouse.
Pre-submission checklist: