MetricRegistryMetricReader produces unordered Iterable with metrics

[edudar]

This is minor but sometimes quite annoying issue. MetricRegistryMetricReader uses simple HashSet to collect metrics from registry. As result they appear it unpredictable order when exported to web/jmx view. It would be expected to see metrics ordered by name instead of

dw.a.run.snapshot.95thPercentile: 10,
dw.a.run.expexp.snapshot.99thPercentile: 82,
dw.a.run.dump.snapshot.max: 1,
dw.a.run.expexp.snapshot.95thPercentile: 9,
dw.a.run.dump.snapshot.999thPercentile: 1,
dw.a.run.count: 2,

[alexVengrovsk]

Maby it would be better to change HashSet to TreeSet

[alexVengrovsk]

But, than should be used constructor with predifined custom comparator TreeSet(Comparator<? super E> comparator), becouse class Metric does not implement Comparable interface.
I dont think, that it is right to add Comparable interface implementation to class Metric.

[alexVengrovsk]

I opened a Pool Request #3770

[jayanderson]

This issue still exists. It'd be nice to get it fixed. The commit above is one option. Another might be to change the implementation of the MetricRegistryMetricReader#names map from a ConcurrentHashMap to a LinkedHashMap wrapped in Collections.synchronizedMap() (or synchronized some other way) (see https://github.com/spring-projects/spring-boot/blob/master/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/reader/MetricRegistryMetricReader.java#L64). It looks like other spots in the actuator metrics use a LinkedHashMap to maintain insertion order as opposed to lexical ordering of the metric keys themselves.

Anyway here's my 👍 to getting this implemented.

[wilkinsona]

As I said in my comment on #3770, I'm not convinced that MetricRegistryMetricReader should be ordered. Many HTTP and JMX clients won't care about the ordering, yet everyone will pay the price of maintaining that order if it's done in MetricRegistryMetricReader.

[wilkinsona]

@jayanderson Using a LinkedHashMap wouldn't help with the raiser's requirement. Also, insertion order, while stable for the lifetime of the JVM, won't produce predictable results across multiple runs so I'm struggling to see the benefit.

[edudar]

I've mentioned that this is the minor thing because it's mostly visual convenience for developer rather than real production issue. We do not use HTTP endpoint for metrics submission in production. Maybe adding &pretty=true type of parameter will help here. If you want ordered output and ok to pay the price - you have an option. Don't need it - just use it as is.

[Aloren]

Hi. I have faced the same issue. My solution was following:

public class SortedMetricsEndpoint extends MetricsEndpoint {

    public SortedMetricsEndpoint(Collection<PublicMetrics> publicMetrics) {
        super(publicMetrics);
    }

    public SortedMetricsEndpoint(PublicMetrics publicMetrics) {
        super(publicMetrics);
    }

    @Override
    public Map<String, Object> invoke() {
        Map<String, Object> metrics = super.invoke();
        TreeMap<String, Object> sorted = new TreeMap<>();
        sorted.putAll(metrics);
        return sorted;
    }
}

Hope it helps. Thanks.

[thombergs]

The lack of sorting had me annoyed while developing on my local machine, too, but I can understand @wilkinsona in saying that sorting should be done on the client side.

As a tip for those being annoyed by viewing unsorted metrics in the browser: use the JSON Viewer chrome plugin and set its option sortKeys to true.

[wilkinsona]

This has been superseded by the planned move to Micrometer-based metrics (#9970)