Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Extract MetricsCategory interface #1573

Closed

Conversation

ajsutton
Copy link
Contributor

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...

Copy link
Contributor Author

@ajsutton ajsutton left a 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);
Copy link
Contributor Author

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 =
Copy link
Contributor Author

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())
Copy link
Contributor Author

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.

@ajsutton
Copy link
Contributor Author

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.

@ajsutton ajsutton closed this Jun 18, 2019
@ajsutton ajsutton deleted the extract-metrics-category-interface branch June 28, 2019 00:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant