eclipse / microprofile-metrics

microprofile-metrics
Apache License 2.0
100 stars 66 forks source link

MetricRegistry API - Don't assure sorted property for returned maps and sets #627

Open jbee opened 3 years ago

jbee commented 3 years ago

Currently the MetricRegistry API often returns a SortedSet and SortedMap.

While it is understandable from user (caller) point of view that a sorted set or map can be more convenient it is not needed is many situations and can easily be constructed from an "ordinary" Set or Map. Implementations could still opt to actually return SortedSet or SortedMap and the eventual applied conversion to a SortedSet or SortedMap as done by the caller can be a NOOP like in this pseudo-code example:

<T> SortedSet<T> toSortedSet(Set<T> set) {
  return set instanceof SortedSet ? return (SortedSet<T>) set : new TreeSet<>(set));
}

Gist is: replacing SortedSet with mere Set in the MetricRegistry API does not remove any capability. It only comes with less convenience in the case we actually want to rely on the additional property that the set or map is indeed sorted.

On the other hand if we require the sets and maps returned by the MetricRegistry to be SortedSet and SortedMap the implementation cannot return read-only views onto its internal data organisation that are cheap to produce unless these are also sorted. This excludes for example sets of keys or values as returned by concurrent maps and concurrent map implementations which are a likely candidates to implement the registry.

Example

Implementations could use the existing API method

SortedMap<MetricID, Metric> getMetrics(MetricFilter filter);

to effectively offer a not yet existing lookup by name

Map<MetricID, Metric> getMetrics(String name);

by recognising the filter implementation being one that only wants to filter on the name in which case it can use a more efficient path returning a view. The reason this is not possible is that the internal map is not sorted as the main concern of the implementation are concurrent reads and inserts. So it would need to construct a new map from the view.