cloudflare/ebpf_exporter

how to support struct type value

cdkunsong opened this issue · 10 comments

hi,
I also want to discuss a issue, whether it is possible to use struct type for value,
although I see in tcp-window-clamps.bpf.c

struct socktest {
     u32 value1;
     u32 value2;
     u32 value3;
}
struct {
    __uint(type, BPF_MAP_TYPE_LRU_HASH);
    __uint(max_entries, 1024);
    __type(key, u64);
    __type(value, struct socktest *);
} tcp_rmem_schedule_enters SEC(".maps");

but it don't use by metrics .
Currently, it seems that a struct type cannot be supported, and it can parse multiple member values.

Can you explain this problem, thanks!

bobrik commented

Please use markdown when you quote code, it's hard to read code without any indentation.

Prometheus metrics have basic types: counter, gauge, histograms (which is just a bunch of counters). All of these have just float64 as values, as described in prometheus data model:

Given that, I'm not sure what you would do with a struct in a map value.

Hello,

could you consider allowing struct values and similarly having labels and decoders for each struct member? I think this is very useful. I am counting a large amount of values in a struct because they all belong to the same key. For ebpf_exporter I would have to split this struct into a dozen maps with value u64. This will probably have a lot of lookup overhead.
If there is a better alternative or I'm misunderstanding something, please let me know.

Thank you.

bobrik commented

What is your concrete use-case? What kind of metrics and labels do you want to produce?

Let's say I'm collecting a dozen counters, that is u64 monotonically increasing variables, all related to the same block device - it makes perfect sense to group these together. I have a per-cpu hash map with a u32 key, decoded as majorminor and a struct disk_counters value type.

For example:

struct disk_counters {
    unsigned long counter;
    unsigned long another_counter;
    unsigned long yet_another_counter;
    // ...
}

Currently, I would have to split this struct by creating a new map for each field. I tried just that and profiling the two BPF programs with bpftool shows degraded performance on the ebpf_exporter-compatible program.
I suggest allowing the user to supply a struct and treating the data obtained from libbpfgo.BPFMap.GetValue as a u64 array. Then only one lookup would be performed, and each array member would be a new metric. The names of each metric could be described in the YAML configuration file, similary to how the static_map decoder works.

Another thing is that when I first tried writing a configuration with a struct as the value type, ebpf_exporter gave no warning or error at all. At least the silent failure should be avoided with a warning.

bobrik commented

Conceptually it makes sense. Can you quantify you performance degradation? How expensive is it with a single map vs multiple maps? Is it visible in overall CPU usage?

bobrik commented

It's unclear to me how to make complex values atomic. It's trivial to increment a single u64. How would you increment an array of them joined together?

bobrik commented

I opened #257 to validate value sizes for maps so that there are no silent failures.

It's unclear to me how to make complex values atomic. It's trivial to increment a single u64. How would you increment an array of them joined together?

This is a good point. So far, I've used per-CPU maps to solve this, so that could be a requirement.

bobrik commented

Could you share the benchmark you're using? I'm still very interesting in these questions: #253 (comment).

bobrik commented

I made #260, but I can't say I'm a big fan of this.