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

eBPF profiler /proc/{id}/maps offset conversion using small bit size #3650

Closed
mahendrapaipuri opened this issue Oct 25, 2024 · 1 comment · Fixed by #3656
Closed

eBPF profiler /proc/{id}/maps offset conversion using small bit size #3650

mahendrapaipuri opened this issue Oct 25, 2024 · 1 comment · Fixed by #3656

Comments

@mahendrapaipuri
Copy link
Contributor

Describe the bug

We noticed the following errors from eBPF profiler when profiling Python processes:

ts=2024-10-25T12:27:37.474994587Z level=error msg="pyperf get python process data failed" component_path=/ component_id=pyroscope.ebpf.default err="GetPythonProcInfo error /proc/342561/maps: strconv.ParseInt: parsing \"ffffa6f1ce560000\": value out of range" pid=342561 target="{__name__=\"process_cpu\", __process_pid__=\"342561\", service_name=\"390048\"}"

Clearly the hex string is being converted to int type here which exceeds the bounds. Shouldn't it be converted to unit64 type to ensure we capture offsets for bigger files?

To Reproduce

Steps to reproduce the behavior:

We noticed this using Grafana Alloy 1.9.1. I think the processes need to map very huge files to reproduce this behaviour.

Expected behavior

File offsets converted correctly to int using a bit size big enough to accommodate the offset.

Environment

  • Bare-metal
  • Grafana Alloy 1.9.1 deployed using pre-compiled binary downloaded from repo

Additional Context

We are testing this on a HPC platform that has huge file systems and probably that is why we could trigger this bug. If developers agree that it needs to be a uint64, would be happy to put up a PR.

@korniltsev
Copy link
Collaborator

Wow, that is some huge executable 🔥

I agree it should be unit64 instead of int64

A PR would be great

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 a pull request may close this issue.

2 participants