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
Learning ebpf_exporter by adding examples/cgroup-rstat-flushing #457
Learning ebpf_exporter by adding examples/cgroup-rstat-flushing #457
Changes from 7 commits
20e65de
043a0d9
793ed07
c8bbfaa
727a241
c6ff257
f6855b0
99c6b9c
f5dabd2
623c9ad
bd96abc
e30576c
7168fd3
9959e13
ee3de80
eb44da9
c32df50
b02f687
dc4ba3e
eaba651
824cb1d
d9a98b3
139acaf
62bdba0
b9cf32c
4f14b2a
054842d
b80cdd8
7a12be4
ebe07e7
3397837
1f21ae1
c5e15de
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.
The tabs are making clang-format unhappy in CI.
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.
IMHO then BPF restricted-C code should be written with same tabs style as kernel code.
Lets change for formatting requirements, please!
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.
Let's leave it for another issue. For now let's just make clang-format happy.
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.
This projects
.clang-format
file states:# Minimal format loosely matching the kernel style.
IMHO it doesn't match kernel style at all...
As it contains:
Looking at kernel source in
.clang-format
this should have been:I will respect that this project requires its own special clang-format rule and obey to these, but I still think they are wrong.
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.
nit:
cgrp
->cgroup
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.
Sure to nit.
I was hoping to get your input to the contents of the comment:
When coding BPF, I usually do one map lookup to get a stats
struct
, that I will update with multiple stats.Here I end-up doing 3 map lookups for single counters.
When I started coding I though I could represent something like:
Question: But ...
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.
They do
Each line here is a separate key in a single map called
ebpf_exporter_cgroup_rstat_locked_total
:The mapping is effectively:
{contended="0", level="0"}
->6558
{contended="1", level="0"}
->0
{contended="0", level="1"}
->34
To encode each key, you turn the raw bytes of the key into the values with the help of decoders.
Working backwards from what you expect on the output, you want something like this in the config:
On the kernel side you'd want something like this to represent the key:
The names here do not matter, only sizes do (u8 corresponds to
size: 1
in the config).Given that your keys are complex structs and not small numbers, you cannot store these in an array, so you need a hashmap:
To actually increment a value, you need to construct the key and to the increment in the map:
Here's an example with a complex key:
Hope this 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.
These suggestions worked for encoding more information ("labels" in Prometheus terms).
I've coded it up and pushed as part of this patchset/PR.
(p.s. I've also encoded "yield" state)
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.
cgroup_rstat_locked_yields_total
, see https://prometheus.io/docs/practices/naming/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 link , as a Prometheus newbie I didn't realize that the _suffix had a meaning.
I now realize that general strucure is library_name_unit_suffix.
Jakub recommended: Prometheus: Up & Running, 2nd Edition