go_labels: introduce map scaler to handle more processes#1058
Conversation
Replace the hard coded limit size of the eBPF map that handled Go labels with a scale factor. Fixes open-telemetry#1055 Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
|
|
||
| // GoLabelsDefaultScaleFactor defines the default scale (1<<X) enabling the handling | ||
| // of labels for this number of Go processes. | ||
| GoLabelsDefaultScaleFactor = 8 |
There was a problem hiding this comment.
MapScaleFactor solves a problem that's expected to manifest infrequently.
In this case, it seems that 128 might be too low as a default value and given that the size of the structure is very small, bumping it to 512 or even 1024 might serve as a better default than yet another configuration option that needs to be documented and also relies on users understanding the issue and updating configuration / restarting the agent / potentially redeploying.
There was a problem hiding this comment.
I'm happy to push it to 9 or 10. Please let me know, which you prefer. With setting it to 8 here, I already increased it from 128 to 256. But looking at micro services and a full system view, even 1024 can be a low number - maybe @bobrik can share some numbers from their environment?
As these maps are preallocated and only used for Go processes, environments without Go processes don't benefit from it. Therefore, I think a configuration option serves here well, as these environment likely don't want to pay these extra memory cost.
There was a problem hiding this comment.
I think that cost is not that high. It's ~39KB for 128 entries:
$ sudo bpftool map show name go_labels_procs
9462681: hash name go_labels_procs flags 0x0
key 4B value 28B max_entries 128 memlock 38784B
btf_id 9483878
pids ebpf-profiler(621922)
If we increase the default to 1024, that would be 310,272 bytes. At 4096 entries it's 1,241,088 bytes. Having an option to raise the number is definitely useful for more extreme environments.
I think 1024 is very much a reasonable default. For environments without Go, we probably shouldn't create the map to begin with if Go interpreter is disabled.
There was a problem hiding this comment.
I'm running with the combined scale factor of 12 with this patch and memlock reports 451,456 bytes, which is lower than the estimate above:
$ sudo bpftool map show name go_labels_procs
9546443: hash name go_labels_procs flags 0x0
key 4B value 28B max_entries 4096 memlock 451456B
btf_id 9568242
pids ebpf-profiler(1414875)
There was a problem hiding this comment.
@christos68k and @bobrik would the increase from 128 to 4096 be fine for both of you?
There was a problem hiding this comment.
I'd pick 1024 as a default, 4096 seems like overkill and locks up too much memory.
There was a problem hiding this comment.
dropped the configuration option and increased the default size from 128 to 1024 with be17679.
There was a problem hiding this comment.
[..] we probably shouldn't create the map to begin with if Go interpreter is disabled.
Adressing with #1063
There was a problem hiding this comment.
I'd keep the config option, but I can also just reopen the issue if we run into it again with the new max.
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Replace the hard coded limit size of the eBPF map that handles Go labels with a scale factor.
Fixes #1055