in_docker: first implementation

[ashutoshdhundhara]

This is for #319.
Just a draft. Also attaching a sample output. Am I moving in right direction?

[0] docker: [1537730279.000958364, {"id"=>"a7e9ae541af46e1", "cpu_used"=>88037091, "mem_used"=>1921024, "mem_limit"=>20971520}]
[1] docker: [1537730279.000966259, {"id"=>"b471533d3e517ce", "cpu_used"=>55996183, "mem_used"=>1417216, "mem_limit"=>10485760}]
[2] docker: [1537730279.000968985, {"id"=>"06d3de15fc7bcad", "cpu_used"=>127468132, "mem_used"=>1343488, "mem_limit"=>10485760}]

Config:

[INPUT]
  Name docker
  Tag docker

[edsiper]

hi, thanks for your work on this.

would you please specify how it operate and the plans to query metrics per containers ?

[ashutoshdhundhara]

Hi Eduardo,
This implementation is based on docker's official guide and is right now functional. You can pull this branch and try it out.
Here CPU/Memory metrics per container are fetched from cgroup metric files as follows:

• CPU Usage: /sys/fs/cgroup/cpu/docker/<docker_id>/cpuacct.usage
• Memory Usage: /sys/fs/cgroup/memory/docker/<docker_id>/memory.usage_in_bytes
• Memory Limit: /sys/fs/cgroup/memory/docker/<docker_id>/memory.limit_in_bytes

Configuration is very simple (as mentioned in previous comment) like in_cpu but still there is scope for more.
Right now, these metrics are iteratively read from their respective files. Can this be a performance issue?

[edsiper]

thanks, this looks definitely very good to be included in the next major version. Let's keep working in the implementation:

• append container name/id to the record
• add option to include or exclude containers:
  • containers: list of containers to include, if not set do all
  • _exclude_containers: list of containers to exclude, if not set include all

now from a dev cycle I recommend:

• work on top of GIT master
• when doing tests, run under valgrind (I saw some alerts)

[ashutoshdhundhara]

thanks, this looks definitely very good to be included in the next major version. Let's keep working in the implementation:

Thanks.

* append container name/id to the record

Short id (12 char long) is appended to the record

* add option to include or exclude containers:
  
  * _containers_: list of containers to include, if not set do all
  * _exclude_containers: list of containers to exclude, if not set include all

Done.

[INPUT]
  Name docker
# Include f735b07819a1 4fd301e1e402
  Exclude f735b07819a1 4fd301e1e402
  Tag docker

now from a dev cycle I recommend:

* work on top of GIT master

I created my current branch in_docker from master.

* when doing tests, run under valgrind (I saw some alerts)

Although I tried fixing some alerts by starting the binary using following command:

G_SLICE=always-malloc G_DEBUG=gc-friendly  valgrind -v --tool=memcheck --leak-check=full --num-callers=40 --log-file=valgrind.log fluent-bit -c fluent-bit.conf

Can you please guide my how to run tests with valgrind or point me to some resource?

[edsiper]

Valgrind can be used to test memory issues, it will slowdown the problem but is good to find problems when developing, I normally do:

valgrind --leak-check=full fluent-bit ....

just make sure that when exiting with ctrl-c there are not problems reported.

[ashutoshdhundhara]

Valgrind can be used to test memory issues, it will slowdown the problem but is good to find problems when developing, I normally do:

valgrind --leak-check=full fluent-bit ....

just make sure that when exiting with ctrl-c there are not problems reported.

Thanks Eduardo.
Finally I got the message:

==10655== All heap blocks were freed -- no leaks are possible

I have pushed my changes to the same branch.

[edsiper]

thanks.

I think would be good to have the container name, from a data analysis the container id/hash is not enough

[ashutoshdhundhara]

thanks.

I think would be good to have the container name, from a data analysis the container id/hash is not enough

I considered this as well, but wasn't able to get container name from any of the cgroup pseudo files or anything similar to this. Although docker stats does exactly what we want. Will now look more deeper into how this command collects metrics. If you have something in your mind in this direction, please suggest.

[ashutoshdhundhara]

@edsiper I've added container name to the record as well .

user@hostname:~/fluent-bit/build$ sudo valgrind --leak-check=full ./bin/fluent-bit --config=/home/user/confi
g.yaml
==21178== Memcheck, a memory error detector
==21178== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==21178== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==21178== Command: ./bin/fluent-bit --config=/home/user/config.yaml
==21178== 
Fluent-Bit v0.13.0
Copyright (C) Treasure Data
[2019/01/03 16:34:02] [ info] [engine] started
[0] docker.0: [1546533243.095125318, {"id"=>"b2e0d9268755", "name"=>"redis-1", "cpu_used"=>124548762542, "mem_used"=>2043904, "mem_limit"=
>4294963200}]
[1] docker.0: [1546533243.101521258, {"id"=>"be8144fe5cd8", "name"=>"redis-2", "cpu_used"=>124716612528, "mem_used"=>1761280, "mem_limit"=
>4294963200}]
[2] docker.0: [1546533244.001340963, {"id"=>"b2e0d9268755", "name"=>"redis-1", "cpu_used"=>124549692866, "mem_used"=>2043904, "mem_limit"=
>4294963200}]
[3] docker.0: [1546533244.001373850, {"id"=>"be8144fe5cd8", "name"=>"redis-2", "cpu_used"=>124717779757, "mem_used"=>1761280, "mem_limit"=
>4294963200}]
[4] docker.0: [1546533245.001376290, {"id"=>"b2e0d9268755", "name"=>"redis-1", "cpu_used"=>124550795330, "mem_used"=>2043904, "mem_limit"=
>4294963200}]
[5] docker.0: [1546533245.001409345, {"id"=>"be8144fe5cd8", "name"=>"redis-2", "cpu_used"=>124718936079, "mem_used"=>1761280, "mem_limit"=
>4294963200}]
[6] docker.0: [1546533246.005077304, {"id"=>"b2e0d9268755", "name"=>"redis-1", "cpu_used"=>124552048887, "mem_used"=>2043904, "mem_limit"=
>4294963200}]
[7] docker.0: [1546533246.005109867, {"id"=>"be8144fe5cd8", "name"=>"redis-2", "cpu_used"=>124719879755, "mem_used"=>1761280, "mem_limit"=
>4294963200}]
^C[engine] caught signal
[2019/01/03 16:34:11] [ info] [input] pausing docker.0
==21178== 
==21178== HEAP SUMMARY:
==21178==     in use at exit: 0 bytes in 0 blocks
==21178==   total heap usage: 527 allocs, 527 frees, 962,633 bytes allocated
==21178== 
==21178== All heap blocks were freed -- no leaks are possible
==21178== 
==21178== For counts of detected and suppressed errors, rerun with: -v
==21178== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[edsiper]

thanks. To start merging this we need a few changes:

• make sure changes are on top of GIT master
• on every flb_malloc() or flb_calloc() check if it returns NULL or not, if so trigger a flb_errno() and return.
• Every new file (.c or .h) in the header must have the copyright assigned to "The Fluent Bit Authors" like here: https://github.com/fluent/fluent-bit/blob/master/src/flb_config.c#L5

[ashutoshdhundhara]

thanks. To start merging this we need a few changes:

• make sure changes are on top of GIT master
• on every flb_malloc() or flb_calloc() check if it returns NULL or not, if so trigger a flb_errno() and return.
• Every new file (.c or .h) in the header must have the copyright assigned to "The Fluent Bit Authors" like here: https://github.com/fluent/fluent-bit/blob/master/src/flb_config.c#L5

@edsiper I have fixed these. But my tests are failing because no dockers are running on build machine and no dockers means no output. How should I handle this?

[edsiper]

can you emulate some running dockers through file system static files ? e,g: tests/runtime/data/...

[ashutoshdhundhara]

can you emulate some running dockers through file system static files ? e,g: tests/runtime/data/...

There are certain paths which are defined as macros in in_docker.h. How should I change them to those of test data directories when running the test suite? If I dynamically create dummy files without changing the actual paths before running the tests, then we will have to launch the make test command with sudo.

[edsiper]

@ashutoshdhundhara

I reviewed the latest changes and looks good, the only thing we need to show in a different way is the CPU metrics, ideally in % from 0-100 instead of the current value.

[edsiper]

ping.

note: please also rebase on top of master.

[ashutoshdhundhara]

Showing percentage from 0-100% makes sense instead of current value. I will need to change the logic a bit. Will update once done.

[hanoch]

@ashutoshdhundhara @edsiper what is the status of this PR?

[edsiper]

waiting for the latest changes requested.

[nigels-com]

Unfortunately this branch is non-trival (but not impossible) to rebase onto master.

One thing that has changed in the meanwhile is the flb_config_prop migration to the new kv interface:

commit 5a0f8a303161525f4fff3e22f8a6c462a8f78e6c
Author: Eduardo Silva <eduardo@treasure-data.com>
Date:   Mon Aug 5 17:27:11 2019 -0600

    input: use new kv interface to handle properties

It looks like a bit of work to have this suitable for merging to the mainline.

[edsiper]

@ashutoshdhundhara

first of all my apologies for the very long delay on this. I've just merged your new plugin and this one will be part of v1.3 release (this week), I've made small changes to adjust to the new internal API and now is fully compatible.

I've merged this plugin manually through d19be0d (you remain the author of the plugin in the commit). Since the plugin is ready I am closing this PR.

thanks for your contribution!!!

[edsiper]

Plugin merged manually, closing PR.

[Reevzee]

I'm not able to retrieve the metrics using this input whilst running fluentbit agent as a docker container, should i be mounting to the cgroup path for this to work ?

[Reevzee]

I'm not able to retrieve the metrics using this input whilst running fluentbit agent as a docker container, should i be mounting to the cgroup path for this to work ?

"I'm currently mounting path to /sys/fs/cgroup/ to get this working in a docker container"