diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java index b67f07995..e13a4acc1 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java @@ -71,9 +71,7 @@ public static void registerCache(TaggedMetricRegistry registry, Cache cach }); } - static > Map> createCacheGauges( - Cache cache, - Function metricNamer) { + static Map> createCacheGauges(Cache cache, Function metricNamer) { return InternalCacheMetrics.createMetrics(CaffeineStats.create(cache, 1, TimeUnit.SECONDS), metricNamer); } } diff --git a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/InternalCacheMetrics.java b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/InternalCacheMetrics.java index 75b5bbc3e..c9bd0f80a 100644 --- a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/InternalCacheMetrics.java +++ b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/InternalCacheMetrics.java @@ -21,7 +21,7 @@ import com.codahale.metrics.RatioGauge; import com.google.common.annotations.Beta; import com.google.common.cache.Cache; -import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableMap; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.Map; @@ -40,10 +40,8 @@ public final class InternalCacheMetrics { private InternalCacheMetrics() {} - public static > Map> createMetrics( - Stats stats, - Function metricNamer) { - ImmutableSortedMap.Builder> builder = ImmutableSortedMap.naturalOrder(); + public static Map> createMetrics(Stats stats, Function metricNamer) { + ImmutableMap.Builder> builder = ImmutableMap.builder(); stats.forEach((name, gauge) -> builder.put(metricNamer.apply(name), gauge)); return builder.build(); } diff --git a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/AbstractTaggedMetricRegistry.java b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/AbstractTaggedMetricRegistry.java index cc83f4d08..ff9132c83 100644 --- a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/AbstractTaggedMetricRegistry.java +++ b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/AbstractTaggedMetricRegistry.java @@ -25,7 +25,7 @@ import com.codahale.metrics.Metric; import com.codahale.metrics.Reservoir; import com.codahale.metrics.Timer; -import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; @@ -143,7 +143,7 @@ public final Timer timer(MetricName metricName, Supplier timerSupplier) { @Override public final Map getMetrics() { - ImmutableSortedMap.Builder result = ImmutableSortedMap.naturalOrder(); + ImmutableMap.Builder result = ImmutableMap.builder(); result.putAll(registry); taggedRegistries.forEach((tag, metrics) -> metrics.getMetrics() .forEach((metricName, metric) -> result.put( diff --git a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/MetricName.java b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/MetricName.java index aa2e6420a..d79dc5237 100644 --- a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/MetricName.java +++ b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/MetricName.java @@ -17,37 +17,15 @@ package com.palantir.tritium.metrics.registry; import com.palantir.logsafe.Safe; -import java.util.Comparator; -import java.util.Iterator; -import java.util.Map; +import java.util.SortedMap; import org.immutables.value.Value; @Value.Immutable(prehash = true) @Value.Style( - jdkOnly = true, get = {"get*", "is*"}, overshadowImplementation = true, visibility = Value.Style.ImplementationVisibility.PACKAGE) -public interface MetricName extends Comparable { - - Comparator> entryComparator = - Map.Entry.comparingByKey() - .thenComparing(Map.Entry.comparingByValue()); - Comparator metricNameComparator = - Comparator.comparing(MetricName::safeName) - .thenComparing(metricName -> metricName.safeTags().entrySet(), (set1, set2) -> { - Iterator> i1 = set1.iterator(); - Iterator> i2 = set2.iterator(); - while (i1.hasNext() && i2.hasNext()) { - Map.Entry e1 = i1.next(); - Map.Entry e2 = i2.next(); - int compare = entryComparator.compare(e1, e2); - if (compare != 0) { - return compare; - } - } - return i1.hasNext() ? -1 : i2.hasNext() ? 1 : 0; - }); +public interface MetricName { /** * General/abstract measure (e.g. server.response-time). @@ -61,12 +39,8 @@ public interface MetricName extends Comparable { *

* All tags and keys must be {@link Safe} to log. */ - Map safeTags(); - - @Override - default int compareTo(MetricName that) { - return metricNameComparator.compare(this, that); - } + @Value.NaturalOrder + SortedMap safeTags(); static Builder builder() { return new Builder(); diff --git a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/MetricNameTest.java b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/MetricNameTest.java index 5648e9462..2286568f3 100644 --- a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/MetricNameTest.java +++ b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/MetricNameTest.java @@ -27,45 +27,62 @@ public void compareSame() { MetricName one = MetricName.builder() .safeName("test") .putSafeTags("key", "value") + .putSafeTags("key1", "value1") + .putSafeTags("key2", "value2") .build(); MetricName two = MetricName.builder() .safeName("test") + .putSafeTags("key2", "value2") .putSafeTags("key", "value") + .putSafeTags("key1", "value1") .build(); assertThat(one).isEqualTo(two); assertThat(two).isEqualTo(one); - assertThat(one).isEqualByComparingTo(two); - assertThat(one).usingComparator(MetricName.metricNameComparator).isEqualByComparingTo(two); + + assertThat(one.toString()).isEqualTo(two.toString()); + } + + @Test + public void compareName() { + assertThat(MetricName.builder().safeName("a").build()) + .isEqualTo(MetricName.builder().safeName("a").build()); + + assertThat(MetricName.builder().safeName("a").build()) + .isNotEqualTo(MetricName.builder().safeName("b").build()); } @Test - public void compareNameOrder() { + public void compareTagNames() { MetricName one = MetricName.builder() .safeName("a") - .putSafeTags("key", "value") + .putSafeTags("key1", "value1") + .putSafeTags("key2", "value2") .build(); MetricName two = MetricName.builder() - .safeName("b") - .putSafeTags("key", "value") + .safeName("a") + .putSafeTags("key1", "value1") + .putSafeTags("key3", "value2") .build(); - assertThat(one).isLessThan(two); - assertThat(two).isGreaterThan(one); + assertThat(one).isNotEqualTo(two); + assertThat(two).isNotEqualTo(one); } @Test - public void compareSameName() { + public void compareTagValues() { MetricName one = MetricName.builder() - .safeName("test") - .putSafeTags("a", "value") + .safeName("a") + .putSafeTags("key1", "value1") + .putSafeTags("key2", "value2") .build(); MetricName two = MetricName.builder() - .safeName("test") - .putSafeTags("b", "value") + .safeName("a") + .putSafeTags("key1", "value1") + .putSafeTags("key2", "valueZ") .build(); - assertThat(one).isLessThan(two); - assertThat(two).isGreaterThan(one); + assertThat(one).isNotEqualTo(two); + assertThat(two).isNotEqualTo(one); } } diff --git a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TaggedMetricRegistryTest.java b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TaggedMetricRegistryTest.java index 2e5cd6e7d..79bf65dd9 100644 --- a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TaggedMetricRegistryTest.java +++ b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TaggedMetricRegistryTest.java @@ -215,6 +215,33 @@ public void testReplaceMetricRegistry() { assertThat(registry.getMetrics()).isEmpty(); } + @Test + public void testGetMetrics() { + MetricName metricName = MetricName.builder() + .safeName("counter1") + .putSafeTags("tagA", Long.toString(1)) + .putSafeTags("tagB", Long.toString(2)) + .build(); + Counter counter = registry.counter(metricName); + counter.inc(); + Metric metric = registry.getMetrics().get(metricName); + assertThat(metric) + .isInstanceOf(Counter.class) + .isSameAs(counter) + .isSameAs(registry.counter( + MetricName.builder() + .safeName("counter1") + .putSafeTags("tagB", "2") + .putSafeTags("tagA", Long.toString(1)) + .build())) + .isSameAs(registry.getMetrics().get(MetricName.builder() + .safeName("counter1") + .putSafeTags("tagA", Long.toString(1)) + .putSafeTags("tagB", Integer.toString(2)) + .build())); + assertThat(counter.getCount()).isEqualTo(1); + } + private void assertMetric(String name, String tagKey, String tagValue, Meter meter) { assertThat(registry.getMetrics()) .containsEntry(MetricName.builder().safeName(name).putSafeTags(tagKey, tagValue).build(), meter);