-
Notifications
You must be signed in to change notification settings - Fork 334
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: support elasticsearch _bulk
API to ingest logs
#5261
feat: support elasticsearch _bulk
API to ingest logs
#5261
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
7ca043b
to
c201c86
Compare
c201c86
to
a0ad334
Compare
_bulk
API to ingest logs by logstash_bulk
API
_bulk
API_bulk
API to ingest logs
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5261 +/- ##
==========================================
- Coverage 84.07% 83.87% -0.20%
==========================================
Files 1199 1203 +4
Lines 225088 226393 +1305
==========================================
+ Hits 189233 189882 +649
- Misses 35855 36511 +656 |
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.
Looks mostly good to me. Is there any chance to specify and invoke pipeline during the ingestion process? We can address it in next patch.
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.
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
67a30ae
to
b4f3ef3
Compare
Signed-off-by: zyy17 <[email protected]>
b4f3ef3
to
b8a48de
Compare
It's already in the request parameter. You can use &pipeline_name=xxx to specify the pipeline. Default it's greptime_identity. |
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.
Good job! Can we add a http integration test for this API? Please refer to https://github.com/GreptimeTeam/greptimedb/blob/main/tests-integration/tests/http.rs
a5a321c
to
f55c962
Compare
f55c962
to
29e3429
Compare
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.
LGTM. Good job!
…5261) * feat: support elasticsearch '_bulk' API to ingest logs Signed-off-by: zyy17 <[email protected]> * refactor: code review * refactor: add metrics --------- Signed-off-by: zyy17 <[email protected]>
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
_bulk
API to ingest logs;PR Checklist
Please convert it to a draft if some of the following conditions are not met.