Skip to content

out_es: support write_operation option#4079

Merged
edsiper merged 8 commits into
fluent:masterfrom
liljaylj:master
Jan 7, 2022
Merged

out_es: support write_operation option#4079
edsiper merged 8 commits into
fluent:masterfrom
liljaylj:master

Conversation

@liljaylj
Copy link
Copy Markdown
Contributor

@liljaylj liljaylj commented Sep 10, 2021 •
edited
Loading

Adds write_operation option as in fluentd-plugin-elasticsearch (https://github.com/uken/fluent-plugin-elasticsearch#write_operation)

Fixes: #1430

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

Documentation

  • Documentation required for this feature

fluent/fluent-bit-docs#603

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Sep 10, 2021 •
edited
Loading

Configuration example

elasticsearch.conf.txt

@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Sep 10, 2021 •
edited
Loading

Test result

result of command (in ./build dir):

(cmake -DFLB_DEV=On -DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On .. && make && valgrind bin/flb-rt-out_elasticsearch) 2>&1 | tee /tmp/fluent-test.log

fluent-test.log

@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Sep 11, 2021

Valgrind output

result of command (in ./build dir):

(cmake -DFLB_DEV=On -DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On .. && make && valgrind bin/fluent-bit -c /tmp/elasticsearch.conf.txt) 2>&1 | tee /tmp/fluent-valgrind.log

fluent-valgrind.log

out_es: support write_operation option
8b5bb2c 
Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
@liljaylj liljaylj marked this pull request as ready for review September 11, 2021 03:20
nokute78
nokute78 suggested changes Oct 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@nokute78 nokute78 left a comment

Choose a reason for hiding this comment

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

Could you attach valgrind output again ?

Below file is incomplete.
There is no summary information.
#4079 (comment)

Comment thread plugins/out_es/es.c Outdated
Comment thread plugins/out_es/es.c Outdated
Comment thread plugins/out_es/es.c Outdated
Comment thread plugins/out_es/es_conf.c
Comment thread plugins/out_es/es.c Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 3, 2021

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions Bot added the Stale label Nov 3, 2021
@edsiper
Copy link
Copy Markdown
Member

edsiper commented Dec 12, 2021

@liljaylj ping

@edsiper edsiper mentioned this pull request Dec 12, 2021
Merged
@github-actions github-actions Bot removed the Stale label Dec 15, 2021
@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Dec 16, 2021 •
edited
Loading

Could you attach valgrind output again ?

Below file is incomplete. There is no summary information. #4079 (comment)

here you go) new valgrind log after latest commit
fluent-valgrind.log

@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Dec 16, 2021

@liljaylj ping

Sorry, it was tough lately. Multiple projects was going to production)

@liljaylj liljaylj requested a review from nokute78 December 16, 2021 15:11
@nokute78 nokute78 mentioned this pull request Dec 18, 2021
Closed
1 task
nokute78
nokute78 suggested changes Dec 18, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@nokute78 nokute78 left a comment

Choose a reason for hiding this comment

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

@liljaylj Thank you for updating.
I added some requests. Could you check them?

Comment thread plugins/out_es/es_conf.c
Comment thread plugins/out_es/es.h Outdated
@liljaylj liljaylj requested a review from nokute78 December 19, 2021 00:25
nokute78
nokute78 suggested changes Dec 19, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@nokute78 nokute78 left a comment

Choose a reason for hiding this comment

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

Thank you for fixing and sorry for several requests.

  1. Could you fix conflicts?
  2. Could you modify commit message ?
    It should be out_es: something not fix: something

Zhanibek Adilbekov added 5 commits December 19, 2021 07:17
out_es: move elasticsearch bulk action config to flb_elasticsearch
9b7d58e 
Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
out_es: use snprintf instead of printf
8eb0198 
Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
out_es: use create action by default
8a8dbc0 
Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
out_es: ctx->es_action should be freed on exit since flb_strdup alloc…
4da0fc9 
…ates memory.

Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
Merge remote-tracking branch 'upstream/master'
a22a99d
@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Dec 19, 2021

Thank you for fixing and sorry for several requests.

1. Could you fix conflicts?

2. Could you modify commit message ?
   It should be `out_es: something` not `fix: something`

oh, sorry, my bad.
done)

@liljaylj liljaylj requested a review from nokute78 December 19, 2021 01:28
@nokute78
Copy link
Copy Markdown
Contributor

nokute78 commented Dec 19, 2021

@liljaylj Thank you for quick response.

DCO error is reported for latest commit.
Could you check it? Sorry for multiple requests.
https://probot.github.io/apps/dco/

out_es: fix broken conf condition
3e2ddca 
Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
@liljaylj
Copy link
Copy Markdown
Contributor Author

liljaylj commented Dec 19, 2021

@liljaylj Thank you for quick response.

DCO error is reported for latest commit. Could you check it? Sorry for multiple requests. https://probot.github.io/apps/dco/

done

nokute78
nokute78 approved these changes Dec 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@nokute78 nokute78 left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM.

@edsiper edsiper merged commit d9a9196 into fluent:master Jan 7, 2022
0Delta pushed a commit to 0Delta/fluent-bit that referenced this pull request Jan 20, 2022
@0Delta
out_es: support write_operation option (fluent#4079)
ef47a5f 
* out_es: support write_operation option

Signed-off-by: Zhanibek Adilbekov <zhanibek.adilbekov@pm.me>
@liljaylj liljaylj mentioned this pull request Jan 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edsiper edsiper Awaiting requested review from edsiper edsiper is a code owner

@fujimotos fujimotos Awaiting requested review from fujimotos

@koleini koleini Awaiting requested review from koleini

@PettitWesley PettitWesley Awaiting requested review from PettitWesley PettitWesley is a code owner

1 more reviewer

@nokute78 nokute78 nokute78 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs-required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add the write_operation and upsert mode of the ES Fluentd plugin

3 participants

@liljaylj @edsiper @nokute78