Add meter filter to limit tag cardinality on RestTemplate instrumenta…

…tion (fixes #363) * Tags.concat now prevents duplicate tag keys. * Tags.zip now prevents duplicate tag keys * New Meter.Id#getTag(name) * New MeterFilter#maximumAllowableTagKeys(..)
micrometer-metrics · Jan 24, 2018 · 1341ad4 · 1341ad4
1 parent c0f8243
commit 1341ad4
Show file tree

Hide file tree

Showing 13 changed files with 269 additions and 87 deletions.
diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/Meter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/Meter.java
@@ -100,6 +100,15 @@ public Iterable<Tag> getTags() {
             return tags;
         }
 
+        @Nullable
+        public String getTag(String name) {
+            for (Tag tag : tags) {
+                if(tag.getKey().equals(name))
+                    return tag.getValue();
+            }
+            return null;
+        }
+
         @Nullable
         public String getBaseUnit() {
             return baseUnit;

diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java
@@ -16,12 +16,11 @@
 package io.micrometer.core.instrument;
 
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
+import java.util.function.Function;
 import java.util.stream.Stream;
 
-import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
 import static java.util.stream.StreamSupport.stream;
 
 /**
@@ -31,21 +30,26 @@
 public final class Tags {
     private Tags() {}
 
-    public static List<Tag> zip(String... keyValues) {
+    public static Iterable<Tag> zip(String... keyValues) {
         if (keyValues.length % 2 == 1) {
             throw new IllegalArgumentException("size must be even, it is a set of key=value pairs");
         }
-        List<Tag> ts = new ArrayList<>(keyValues.length / 2);
+
+        Map<String, Tag> ts = new HashMap<>(keyValues.length / 2);
         for (int i = 0; i < keyValues.length; i += 2) {
-            ts.add(Tag.of(keyValues[i], keyValues[i + 1]));
+            ts.put(keyValues[i], Tag.of(keyValues[i], keyValues[i + 1]));
         }
-        return ts;
+
+        return ts.values();
     }
 
     public static Iterable<Tag> concat(Iterable<Tag> tags, Iterable<Tag> otherTags) {
         if(!otherTags.iterator().hasNext())
             return tags;
-        return Stream.concat(stream(tags.spliterator(), false), stream(otherTags.spliterator(), false)).collect(toList());
+
+        return Stream.concat(stream(tags.spliterator(), false), stream(otherTags.spliterator(), false))
+            .collect(toMap(Tag::getKey, Function.identity(), (tag, otherTag) -> otherTag))
+            .values();
     }
 
     public static Iterable<Tag> concat(Iterable<Tag> tags, String... keyValues) {

diff --git a/.../java/io/micrometer/core/instrument/binder/hystrix/MicrometerMetricsPublisherCommand.java b/.../java/io/micrometer/core/instrument/binder/hystrix/MicrometerMetricsPublisherCommand.java
@@ -69,7 +69,7 @@ public class MicrometerMetricsPublisherCommand implements HystrixMetricsPublishe
     private final MeterRegistry meterRegistry;
     private final HystrixCommandMetrics metrics;
     private final HystrixCircuitBreaker circuitBreaker;
-    private final List<Tag> tags;
+    private final Iterable<Tag> tags;
     private final HystrixCommandKey commandKey;
 
     public MicrometerMetricsPublisherCommand(MeterRegistry meterRegistry, HystrixCommandKey commandKey, HystrixCommandGroupKey commandGroupKey, HystrixCommandMetrics metrics, HystrixCircuitBreaker circuitBreaker, HystrixCommandProperties properties) {

diff --git a/...rc/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java b/...rc/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java
@@ -29,7 +29,6 @@
 
 import java.io.IOException;
 import java.util.Collections;
-import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -138,14 +137,14 @@ public static class Builder {
         private MeterRegistry registry;
         private String name;
         private Function<Request, String> uriMapper = (request) -> Optional.ofNullable(request.header(URI_PATTERN)).orElse("none");
-        private List<Tag> tags = Collections.emptyList();
+        private Iterable<Tag> tags = Collections.emptyList();
 
         Builder(MeterRegistry registry, String name) {
             this.registry = registry;
             this.name = name;
         }
 
-        public Builder tags(List<Tag> tags) {
+        public Builder tags(Iterable<Tag> tags) {
             this.tags = tags;
             return this;
         }

diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/config/MeterFilter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/config/MeterFilter.java
@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -165,6 +166,14 @@ public MeterFilterReply accept(Meter.Id id) {
         };
     }
 
+    static MeterFilter accept() {
+        return MeterFilter.accept(id -> true);
+    }
+
+    static MeterFilter deny() {
+        return MeterFilter.deny(id -> true);
+    }
+
     /**
      * Useful for cost-control in monitoring systems which charge directly or indirectly by the
      * total number of time series you generate.
@@ -190,6 +199,44 @@ public MeterFilterReply accept(Meter.Id id) {
         };
     }
 
+    /**
+     * Places an upper bound on
+     *
+     * @param meterName
+     * @param tagKey
+     * @param maximumTagValues
+     * @param onMaxReached
+     * @return
+     */
+    static MeterFilter maximumAllowableTags(String meterName, String tagKey, int maximumTagValues,
+                                            MeterFilter onMaxReached) {
+        return new MeterFilter() {
+            private final Set<String> observedTagValues = new ConcurrentSkipListSet<>();
+
+            @Override
+            public MeterFilterReply accept(Meter.Id id) {
+                if(id.getName().equals(meterName)) {
+                    String value = id.getTag(tagKey);
+                    if(value != null)
+                        observedTagValues.add(value);
+                }
+
+                if(observedTagValues.size() > maximumTagValues) {
+                    return onMaxReached.accept(id);
+                }
+                return MeterFilterReply.NEUTRAL;
+            }
+
+            @Override
+            public HistogramConfig configure(Meter.Id id, HistogramConfig config) {
+                if(observedTagValues.size() > maximumTagValues) {
+                    return onMaxReached.configure(id, config);
+                }
+                return config;
+            }
+        };
+    }
+
     static MeterFilter denyNameStartsWith(String prefix) {
         return deny(id -> id.getName().startsWith(prefix));
     }

diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/MeterFilterTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/MeterFilterTest.java
@@ -22,6 +22,8 @@
 import org.assertj.core.api.Condition;
 import org.junit.jupiter.api.Test;
 
+import java.util.concurrent.atomic.AtomicInteger;
+
 import static java.util.Collections.emptyList;
 import static java.util.stream.StreamSupport.stream;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -87,6 +89,31 @@ void maximumAllowableMetrics() {
         assertThat(filter.accept(id2)).isEqualTo(MeterFilterReply.DENY);
     }
 
+    @Test
+    void maximumAllowableTags() {
+        AtomicInteger n = new AtomicInteger(0);
+
+        MeterFilter filter = MeterFilter.maximumAllowableTags("name", "k", 2, new MeterFilter() {
+            @Override
+            public MeterFilterReply accept(Meter.Id id) {
+                n.incrementAndGet();
+                return MeterFilterReply.NEUTRAL;
+            }
+        });
+
+        Meter.Id id = new Meter.Id("name", Tags.of("k", "1"), null, null, Meter.Type.Counter);
+        Meter.Id id2 = new Meter.Id("name", Tags.of("k", "2"), null, null, Meter.Type.Counter);
+        Meter.Id id3 = new Meter.Id("name", Tags.of("k", "3"), null, null, Meter.Type.Counter);
+
+        filter.accept(id);
+        filter.accept(id);
+        filter.accept(id2);
+        filter.accept(id);
+        filter.accept(id3);
+
+        assertThat(n.get()).isEqualTo(1);
+    }
+
     private static Condition<Meter.Id> tag(String tagKey) {
         return tag(tagKey, null);
     }

diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/TagsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/TagsTest.java
@@ -27,7 +27,18 @@ class TagsTest {
     @Test
     void createsListWithSingleTag() {
         Iterable<Tag> tags = Tags.of("k1", "v1");
-
         assertThat(tags).containsExactly(Tag.of("k1", "v1"));
     }
+
+    @Test
+    void concatOnTwoTagsWithSameKeyAreMergedIntoOneTag() {
+        Iterable<Tag> tags = Tags.concat(Tags.of("k", "v1"), "k", "v2");
+        assertThat(tags).containsExactly(Tag.of("k", "v2"));
+    }
+
+    @Test
+    void zipOnTwoTagsWithSameKeyAreMergedIntoOneTag() {
+        Iterable<Tag> tags = Tags.zip("k", "v1", "k", "v2");
+        assertThat(tags).containsExactly(Tag.of("k", "v2"));
+    }
 }
diff --git a/...a/io/micrometer/core/instrument/binder/hystrix/MicrometerMetricsPublisherCommandTest.java b/...a/io/micrometer/core/instrument/binder/hystrix/MicrometerMetricsPublisherCommandTest.java
@@ -25,7 +25,6 @@
 import reactor.core.publisher.Flux;
 
 import java.time.Duration;
-import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -67,7 +66,7 @@ void testCumulativeCounters() throws Exception {
             new SuccessCommand(key).execute();
         }
 
-        List<Tag> tags = Tags.zip("group", "MicrometerGROUP", "key", "MicrometerCOMMAND-A");
+        Iterable<Tag> tags = Tags.zip("group", "MicrometerGROUP", "key", "MicrometerCOMMAND-A");
 
         assertExecutionMetric(registry, "success", 24.0);
         assertThat(registry.mustFind("hystrix.execution").tags(tags).tags("event", "timeout").functionCounter().count()).isEqualTo(3.0);
@@ -108,7 +107,7 @@ void testOpenCircuit() throws Exception {
         new FailureCommand(key).execute();
         new SuccessCommand(key).execute();
 
-        List<Tag> tags = Tags.zip("group", groupKey.name(), "key", key.name());
+        Iterable<Tag> tags = Tags.zip("group", groupKey.name(), "key", key.name());
 
         assertExecutionMetric(registry, "short_circuited", 6.0);
         assertThat(registry.mustFind("hystrix.execution").tags(tags).tags("event", "success").functionCounter().count()).isEqualTo(0.0);

diff --git a/...ter-core/src/test/java/io/micrometer/core/instrument/binder/tomcat/TomcatMetricsTest.java b/...ter-core/src/test/java/io/micrometer/core/instrument/binder/tomcat/TomcatMetricsTest.java
@@ -31,7 +31,6 @@
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
-import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
@@ -82,7 +81,7 @@ public Context getContext() {
         expiredSession.setCreationTime(System.currentTimeMillis() - 10_000);
         manager.remove(expiredSession, true);
 
-        List<Tag> tags = Tags.zip("metricTag", "val1");
+        Iterable<Tag> tags = Tags.zip("metricTag", "val1");
         TomcatMetrics.monitor(registry, manager, tags);
 
         assertThat(registry.mustFind("tomcat.sessions.active.max").tags(tags).gauge().value()).isEqualTo(3.0);