Skip to content
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

Adds histogram prometheus metrics #112

Merged
merged 14 commits into from
Jun 9, 2024

Conversation

albertlockett
Copy link
Contributor

@albertlockett albertlockett commented Feb 19, 2024

Adds histogram prometheus metrics.

Users can specify histogram metrics using map type ringbuf by using the prefix hist_

struct event {
        char fname[255];
        // by convention struct member 'le' will be the measurement 
        u64 le; 
};

struct {
        __uint(type, BPF_MAP_TYPE_RINGBUF);
        __uint(max_entries, 1 << 24);
        __type(value, struct event);
} hist_file_read SEC(".maps");

Histogram buckets, as well as the field containing the measurment value, can be provided as flags -b/--hist-buckets and -k/--hist-value-key

for example:

go run ./bee run --no-tty -b "hist_file_read,[1000,2000,5000,10000,20000]" myprog:latest

output:

curl -XGET -s localhost:9091/metrics | grep ebpf 
# HELP ebpf_solo_io_hist_file_read 
# TYPE ebpf_solo_io_hist_file_read histogram
ebpf_solo_io_hist_file_read_bucket{fname="test.txt",le="1000"} 7
ebpf_solo_io_hist_file_read_bucket{fname="test.txt",le="2000"} 8
ebpf_solo_io_hist_file_read_bucket{fname="test.txt",le="5000"} 14
ebpf_solo_io_hist_file_read_bucket{fname="test.txt",le="10000"} 16
ebpf_solo_io_hist_file_read_bucket{fname="test.txt",le="20000"} 19
ebpf_solo_io_hist_file_read_bucket{fname="test.txt",le="+Inf"} 20
ebpf_solo_io_hist_file_read_sum{fname="test.txt"} 107008

Also fixes bug where chars would get decoded as ints

@solo-build-bot
Copy link

Waiting for approval from someone in the solo-io org to start testing.

Copy link
Contributor

@krisztianfekete krisztianfekete left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you looked into native histograms as well? It's becoming more and more supported/adopted and can help with the problem of fixed sized buckets.

If we were to add this, this should go behind a flag that is disabled by default, keeping regular histograms as a default for now.

pkg/stats/stats.go Outdated Show resolved Hide resolved
pkg/stats/stats.go Outdated Show resolved Hide resolved
pkg/stats/stats.go Outdated Show resolved Hide resolved
pkg/cli/internal/commands/run/run.go Show resolved Hide resolved
pkg/cli/internal/commands/run/run.go Show resolved Hide resolved
examples/nfsstats/nfsstats.c Show resolved Hide resolved
@albertlockett albertlockett marked this pull request as ready for review March 1, 2024 17:58
krisztianfekete

This comment was marked as resolved.

pkg/loader/loader.go Show resolved Hide resolved
pkg/cli/internal/commands/run/run.go Outdated Show resolved Hide resolved
Copy link
Contributor

@krisztianfekete krisztianfekete left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it locally, and it works.

@soloio-bulldozer soloio-bulldozer bot merged commit 09bf384 into solo-io:main Jun 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants