in_tail: Support multiline functionality (not filter) in dockermode#2043
in_tail: Support multiline functionality (not filter) in dockermode#2043edsiper merged 2 commits intofluent:masterfrom SumoLogic:drosiek-docker-mulitline-parser
Conversation
|
Should we not have: instead of : I might miss something :-) |
|
@jujugrrr I think this is correct behavior at least for this parser and regex. Should returns full docker log as "log". This is output for code without changes: |
My bad I think it's just the way you stdout. By the way, It's a great missing feature, we would love to see this implemented |
|
Example connfiguration:
|
|
Debug log |
|
Valgrind Second report (with incresed stackframe) I didn't notice any more than before my changes |
|
@edsiper Could you take a look at this PR, please. I added new parser to avoid confusion and to keep backward compability as much as possible |
|
We noticed mem-leak. Please do not merge |
sumo-drosiek
changed the title
sumo-drosiek
changed the title
|
It'd be really amazing to get this merged and released. I have developers yelling at me almost daily for this and they think I'm blowing them off when I tell them this isn't supported yet. It's getting to the point of them asking us to abandon Fluent Bit. |
|
Memory leak is fixed, so code is ready to review and merge @AXington I hope it will be review soon. I encourage to test if the fix meet the expectations :) |
edsiper
left a comment
edsiper
left a comment
There was a problem hiding this comment.
Thanks for the patch. I did a quick review and added some comments.
In addition to the changes, would you please include some runtime tests for this feature ? (tests/runtime)
| struct flb_time out_time = {0}; | ||
|
|
||
| if (flb_sds_len(file->dmode_lastline) > 0 && file->dmode_complete) { | ||
| #ifdef FLB_HAVE_REGEX |
There was a problem hiding this comment.
this macro conditional should cover the if statement, otherwise the if does nothing.
plugins/in_tail/tail_dockermode.c
Outdated
| flb_sds_len_set(file->dmode_lastline, 0); | ||
|
|
||
| /* concatenate current log line with buffered one */ | ||
| file->dmode_buf = flb_sds_cat(file->dmode_buf, val, val_len); |
There was a problem hiding this comment.
please use a flb_sds_t tmp variable to check the return value of the flb_sds_*() calls. If one of them returns NULL due to problems allocating memory it needs to return right away to avoid unexpected behaviors in the bottom lines.
|
reviewed, comments above. |
edsiper
added
the
waiting-for-user
|
@edsiper Thanks for review. I already fixed that, but I got SIGSEGV for few ci tests (FLB_OPT="-DFLB_SMALL=On" twice and FLB_OPT="-DSANITIZE_ADDRESS=On" once) and I couldn't spot the reason. Would be grateful for quick look and comment if I should be afraid of that. I can back to the investigation early next week |
|
I fixed the runtime tests for in_tail and I believe the PR is ready to merge |
|
@edsiper - Any other items to fix in this PR before merging? Any update on when we can expect this fix? |
|
As many others mentioned on this thread, we are also currently blocked on this. Anything you can do to expedite this would be highly appreciated! |
|
Thanks everyone for your efforts on this. We are really looking forward to this functionality with our sumo collector chart. |
|
We are waiting for this fix as well. |
|
Kindly expedite this change. It would be much appreciated! |
|
+1
… On Jul 8, 2020, at 3:11 PM, Muhammad Salman Shaikh ***@***.***> wrote:
Kindly expedite this change. It would be much appreciated!
@edsiper
@sumo-drosiek
We and many others are blocked due to this.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
thanks, this PR looks good for v1.5 release. Before to merge it, would you please squash the commits as this:
to make it part of the release, pls do the adjustment ASAP |
|
(Adding in a comment since I want this feature to get out in 1.5... have been waiting for it for a while...)
@sumo-drosiek Just to be clear- he means like in the next 24 - 48 hours. |
Add Docker_Mode_Parser and use it to verify if line is valid
first line while usin dockermode.
For example input:
```
{"log":"2020-03-24 Test line of first log\n"}
{"log":" Another line of first log\n"}
{"log":"2020-03-24 Test line of second log\n"}
```
And regex: `Regex (?<log>^{"log":"\d{4}-\d{2}-\d{2}.*)`
Will output:
```
[0] containers.var.log.containers.test.log: [1585071971.732015500,
{"log"=>"{"log":"2020-03-24 Test line of first log
\n Another line of first log\n"}"}]
[1] containers.var.log.containers.test.log: [1585071975.000917200,
{"log"=>"{"log":"2020-03-24 Test line of second log\n"}"}]
```
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
|
@edsiper @PettitWesley Done 🚀 |
|
thanks! |
|
@sumo-drosiek I was testing out new Docker_Mode_Parser. It is not detecting Time_Key that is captured in regex. Is this expected behaviour or am I missing something? PS: I already checked my regex with sample data in rubular, and it is capturing the 2 fields properly. My parser config:[INPUT] [PARSER] stdout: [0] kubernetes.var.log.containers.loadtestfluent1-2667j_addon-oil-ns_loggingapp-526b62744ad55bbb46cf0ece1b636397311b9393dfd1763179b9a22b64ebb1ef.log: [1594664190.567404747, {"log"=>"{"log":"2020-07-13 18:15:53 +0000:loadtest:INFO:load_run_uuid=flbrun-pr-test1.1.3_302 zblnoucglj\nzblnoucglj\nzblnoucglj line=2924s\n","stream":"stdout","time":"2020-07-13T18:15:53.39885287Z"}"}] |
|
@sibidass This is expected. You can try |
|
@sumo-drosiek After testing Docker_Mode_Parser and updating the image to 1.5.0 i see its supporting multiline supporting but we see kubernetes labels are missing to the log output. We are using the same configuration when we are using 1.3.1 and old(here we see kubernetes labels are part of the output). [INPUT] [FILTER] [PARSER] |
|
my bad.. I thought, like in Multiline option, I will not be able to use Parser along Docker_Mode_Parser. however, the solution is working great.. kudos to all the great work @sumo-drosiek!! much appreciated! . finally I will be able to get rid of my lua code base, which is processing multilines right now in our log pipeline, which was kind of slowing us down. I am sure this will give a lot of performance boost. 🥳 🚀 |
@sateesh3550 kubernetes metadata enrichment is working fine for me. Are you sure you have not tagged it wrongly in tail? Tag test in your tail configuration and Kube_Tag_Prefix kube.var.log.containers. in kubernetes filter config. |
|
@sibidass i'm sure i'm tagging it correctly. The exact configuration is working fine, when i'm using 1.3.1 |
Does the issue occurs for both |
|
@sumo-drosiek |
|
@sateesh3550 |
|
@sumo-drosiek your right, but docker_mode_parser fixed the multiline functionality to parse it in a right way. But upgrading the image from 1.3.1 to 1.4.6 or 1.5.0 to use this feature called docker_mode_parser broke the functionality of kubernetes labels. |
|
@sateesh3550 So, I believe you should create separate issue for tracking it. You can try to figure out what is the first non working image and it will help a lot to find out the breaking change, but anyway it shouldn't be tracked here. |
|
I am trying to use Docker_Mode_Parser to parse multiline json logs, but two or more logs are being concatenated in the es output.
|
|
@logicflakes Looks like you are trying to match log from the es and not from the docker itself. Is it the content of the |
didn't get you
yes more details on issue #2471 |
Sorry, I thought the logs are from es. My point was that logs from but docker/kubectl logs will show: |
|
@sumo-drosiek |
|
You're welcome. I suppose I need to add documentation for that feature 😅 |
14 participants
Use Parser to verify if line is valid first line while using dockermode.
For example input:
And regex:
Regex (?<log>^{"log":"\d{4}-\d{2}-\d{2}.*)Will output:
Fixes #1115
Testing
Before we can approve your change; please submit the following in a comment:
Documentation
Documentation should be updated with information how parser is use to catch multiline docker log
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.