-
Notifications
You must be signed in to change notification settings - Fork 130
Extract MetricsCategory interface #1573
Extract MetricsCategory interface #1573
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added comments on the bits that aren't just a trivial rename.
@@ -56,6 +56,6 @@ default void createLongGauge( | |||
Stream<Observation> streamObservations(MetricCategory category); | |||
|
|||
default Stream<Observation> streamObservations() { | |||
return Stream.of(MetricCategory.values()).flatMap(this::streamObservations); | |||
return Stream.of(PantheonMetricCategory.values()).flatMap(this::streamObservations); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still ties us to PantheonMetricCategory
but will be cleaned up in a follow-up PR. Trying to focus just on the rename in this one.
@@ -56,7 +59,8 @@ | |||
private final Map<String, LabelledMetric<tech.pegasys.pantheon.metrics.OperationTimer>> | |||
cachedTimers = new ConcurrentHashMap<>(); | |||
|
|||
private final EnumSet<MetricCategory> enabledCategories = EnumSet.allOf(MetricCategory.class); | |||
private final Set<MetricCategory> enabledCategories = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to use a Set
instead of EnumSet
now.
|
||
import org.junit.Test; | ||
|
||
public class PrometheusMetricsSystemTest { | ||
|
||
private static final Comparator<Observation> IGNORE_VALUES = | ||
Comparator.comparing(Observation::getCategory) | ||
Comparator.<Observation, String>comparing(o -> o.getCategory().getName()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Category is no longer an enum so the comparator has to be based off the name instead of the enum ordinal (which is probably a good thing anyway). Not sure why the generics is being difficult but it really wants to be told explicitly.
Actually, this was a step too far - the change to an interface requires all the detail. Will just do a simple rename without the enum as the next step. |
PR description
Extract a
MetricCategory
interface instead of having all categories in a single enum.The vast majority of this is just the rename of the enum to
PantheonMetricCategory
.Builds on #1572 but getting it lined up...