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

The sample count displayed is wrong #3340

Open
4 tasks
simonswine opened this issue Jun 6, 2024 · 0 comments
Open
4 tasks

The sample count displayed is wrong #3340

simonswine opened this issue Jun 6, 2024 · 0 comments

Comments

@simonswine
Copy link
Contributor

Describe the bug

Our currently displayed sample count in the UI is wrong. It seems to represent the sum of values multiplied by 10e9. In order to correct this we need to signal the sampling rate from the collection point (SDK/alloy/agent) and persist it in Pyroscope.

The OG Pyroscope endpoint has this metadata, which is unused by the adapter:

SampleRate uint32

We then need to calculate the average sample rate for the profiles in a flamegraph (sample rate is a flamegraph wide parameter in flamebearer) and update our read path accordingly.

To Reproduce

Steps to reproduce:

  1. Query e.g. a CPU profile:
image

Both the value in the tool tip and and the total value, are wrong and just derived by

Expected behavior

This is the hard part to define the right count of samples in a flameGraph.

For time unit profiles:

  • Sum of values / sampleRate, while the sampleRate is only valid per profile

For memory/object unit profiles:

  • Unsure what we can do here, go for example scales up the sampled in-use space to represent the actual memory usage

Next steps

Variant A:

  • Update the default value for CPU to be more correct for defaults:

Variant B:

  • Persist sample rate per profile
  • Adapt query path to use avg. sampleRate for profiles in a flamegraph
  • Adapt query path display indivual sample counts per flamegraph box
simonswine added a commit to simonswine/pyroscope that referenced this issue Jun 6, 2024
This fixes the default sample rate for profiling and clarfies the field
value. Relates to grafana#3340.
simonswine added a commit to simonswine/pyroscope that referenced this issue Aug 21, 2024
This fixes the default sample rate for profiling and clarfies the field
value. Relates to grafana#3340.
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

No branches or pull requests

1 participant