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

Add additional buckets to fanout histogram #685

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

jcreixell
Copy link
Contributor

PR Description

  • The prometheus_fanout_latency is used to measure latencies for data moving between components.
  • The default buckets are limited to 10s, which limits the ability detect very high latencies.
  • Adds to additional buckets (30s, 60s) for additional resolution.

Notes to the Reviewer

  • I initially planned to replace some of the existing buckets instead of adding new ones, but I believe most components will fall into the smaller buckets and reducing the resolution there would lead to distortions in the quantile calculations (and potentially create the ilusion of performance degradation). Note however that the 2 new buckets will result in increased cardinality of 2 metrics per collector instance.

  - The prometheus_fanout_latency is used to measure latencies for data
    moving between components.
  - The default buckets are limited to 10s, which limits the ability
    detect very high latencies.
  - Adds to additional buckets (30s, 60s) for additional resolution.
  - Note: I initially planned to replace some of the existing buckets
    instead of adding new ones, but I believe most components will fall
    into the smaller buckets and reducing the resolution there would
    lead to distortions in the quantile calculations (and potentially
    create the ilusion of performance degradation). Note however that
    the 2 new buckets will result in increased cardinality of 2 metrics
    per collector instance.
Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

LGTM

@thampiotr thampiotr merged commit b581378 into main Apr 30, 2024
13 checks passed
@thampiotr thampiotr deleted the fanout-histogram-add-buckets branch April 30, 2024 16:16
hainenber pushed a commit to hainenber/alloy that referenced this pull request May 1, 2024
- The prometheus_fanout_latency is used to measure latencies for data
    moving between components.
  - The default buckets are limited to 10s, which limits the ability
    detect very high latencies.
  - Adds to additional buckets (30s, 60s) for additional resolution.
  - Note: I initially planned to replace some of the existing buckets
    instead of adding new ones, but I believe most components will fall
    into the smaller buckets and reducing the resolution there would
    lead to distortions in the quantile calculations (and potentially
    create the ilusion of performance degradation). Note however that
    the 2 new buckets will result in increased cardinality of 2 metrics
    per collector instance.
polyrain pushed a commit to polyrain/alloy that referenced this pull request May 1, 2024
- The prometheus_fanout_latency is used to measure latencies for data
    moving between components.
  - The default buckets are limited to 10s, which limits the ability
    detect very high latencies.
  - Adds to additional buckets (30s, 60s) for additional resolution.
  - Note: I initially planned to replace some of the existing buckets
    instead of adding new ones, but I believe most components will fall
    into the smaller buckets and reducing the resolution there would
    lead to distortions in the quantile calculations (and potentially
    create the ilusion of performance degradation). Note however that
    the 2 new buckets will result in increased cardinality of 2 metrics
    per collector instance.
@rfratto rfratto added the backport-to-agent:no PR should NOT be backported to the agent repo. label May 3, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport-to-agent:no PR should NOT be backported to the agent repo. frozen-due-to-age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants