Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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(quickwit_output): implement the quickwit output #736
feat(quickwit_output): implement the quickwit output #736
Changes from all commits
2e9f3f5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
smart, but it also means we need to update all refs to
NewClient
in another PR.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'll handle that in a next PR if you want.
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.
Falco payload contains more fields:
hostname
(string)output_fields
(map[string]string)tags
([]string)Can you also manage them?
FYI the UUID is added by falcosidekick, it's not from Falco itself, you can let it or not, I did that for outputs which require it (like falcosidekick-ui)
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 done for
hostname
andtags
but for a nested object we need to know the name of the subfields to be able to define a mapping (here's the documentation):So we need to know some constant keys in the map to define a mapping. I think it would be the same for defining an Elasticsearch mapping :)
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.
Ok makes sense.
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.
Btw I think it'd be interesting to propose this feature (autocreate index with the mapping) for creating the Elasticsearch index, I can do that in a new PR too after this one :)
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.
@idrissneumann you can use a
json
field for theoutput_fields
.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.
@Issif I printed some object values with the
%v
format which is not supposed to render multi-lined json and I checked with docker stdout/stderr like that:Or:
Seems not multi-line to me, isn't it? Or were you referring to something else otherwise? Thanks
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.
I saw it in that screenshot
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.
@Issif Those logs are coming from the common http client used by all the outputs and we said earlier we wanted to avoid refactoring it in this particular PR ^^
If you want I can also open an issue just saying the common http client need to inline the backend body response when there is an http error.
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 please, we'll tackle that point right after, it's a quick win.
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.
Thanks for the merge. I'll now open three issues (for the
InitClient
method, for the elasticsearch's mapping and for this one) and start some PR :p