line / line/armeria

Review and adjust the default `DistributionStatisticConfig`

Open
#4,792 4 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

defect
Dominant language
Java
Stars
5.1k
Forks
1k
Avg merge
2d 19h
Merged PRs (30d)
31

Description

Currently, Armeria uses the default `DistributionStatisticConfig` of:

```java
DistributionStatisticConfig
.builder()
.percentilesHistogram(false)
.sla()
.percentiles(new double[] { 0, 0.5, 0.75, 0.9, 0.95, 0.98, 0.99, 0.999, 1.0 })
.percentilePrecision(2)
.minimumExpectedValue(1L)
.maximumExpectedValue(Long.MAX_VALUE)
.expiry(Duration.ofMinutes(3))
.bufferLength(3)
.build();
```

When we define the above defaults, our intention was to rotate the buffer every minute, as explained in the comments:

```java
/**
* Export the percentile values only by default. We specify all properties so that we get consistent values
* even if Micrometer changes its defaults. Most notably, we changed {@code percentilePrecision},
* {@code expiry} and {@code bufferLength} due to the following reasons:
*


    *
  • The default {@code percentilePrecision} of 1 is way too inaccurate.

  • *
  • Histogram buckets should be rotated every minute rather than every some-arbitrary-seconds
    * because that fits better to human's mental model of time. Micrometer's 2 minutes / 3 buffers
    * (i.e. rotate every 40 seconds) does not make much sense.

  • *

*/
```

However, according to `TimeWindowMax.rotate()`: https://github.com/micrometer-metrics/micrometer/blob/v1.7.1/micrometer-core/src/main/java/io/micrometer/core/instrument/distribution/TimeWindowMax.java#L118-L145

it seems to me that the buffers are rotated every 3 minutes.

Action item: Double check if the buffers are rotated every minute with the current defaults. Change the defaults if we were mistaken.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Armeria's default DistributionStatisticConfig definition and trace Micrometer's TimeWindowMax.rotate() implementation at the linked entry point. Verify whether the current expiry and bufferLength rotate buffers every minute, then adjust the defaults and explanatory comments if the assumption is wrong. Done means the rotation interval is correct and the relevant metrics behavior is covered by existing tests, if available.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
observability
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.