Merge pull request #901 from zalando/bugfix/prometheus-stable-tags

Made all tag generators produce fixed set of tags
zalando · Feb 18, 2020 · 5c8a077 · 5c8a077
2 parents 8edb1e6 + c45e914
commit 5c8a077
Show file tree

Hide file tree

Showing 10 changed files with 168 additions and 104 deletions.
diff --git a/riptide-micrometer/src/main/java/org/zalando/riptide/micrometer/CompletableFutures.java b/riptide-micrometer/src/main/java/org/zalando/riptide/micrometer/CompletableFutures.java
@@ -0,0 +1,34 @@
+package org.zalando.riptide.micrometer;
+
+import org.zalando.fauxpas.ThrowingBiConsumer;
+import org.zalando.fauxpas.ThrowingConsumer;
+
+import static java.util.Objects.nonNull;
+
+final class CompletableFutures {
+
+    private CompletableFutures() {
+        // nothing to do
+    }
+
+    static <T, X extends Exception> ThrowingBiConsumer<T, Throwable, X> onResult(
+            final ThrowingConsumer<T, X> consumer) {
+
+        return (result, throwable) -> {
+            if (nonNull(result)) {
+                consumer.tryAccept(result);
+            }
+        };
+    }
+
+    static <T, X extends Exception> ThrowingBiConsumer<T, Throwable, X> onError(
+            final ThrowingConsumer<Throwable, X> consumer) {
+
+        return (result, throwable) -> {
+            if (nonNull(throwable)) {
+                consumer.tryAccept(throwable);
+            }
+        };
+    }
+
+}
diff --git a/riptide-micrometer/src/main/java/org/zalando/riptide/micrometer/MicrometerPlugin.java b/riptide-micrometer/src/main/java/org/zalando/riptide/micrometer/MicrometerPlugin.java
@@ -10,7 +10,7 @@
 import lombok.AllArgsConstructor;
 import org.apiguardian.api.API;
 import org.springframework.http.client.ClientHttpResponse;
-import org.zalando.fauxpas.ThrowingUnaryOperator;
+import org.zalando.fauxpas.ThrowingSupplier;
 import org.zalando.riptide.Attribute;
 import org.zalando.riptide.AttributeStage;
 import org.zalando.riptide.Plugin;
@@ -30,10 +30,10 @@
 
 import static com.google.common.collect.ImmutableList.copyOf;
 import static io.micrometer.core.instrument.Timer.builder;
-import static java.util.Objects.nonNull;
 import static lombok.AccessLevel.PRIVATE;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
-import static org.zalando.fauxpas.FauxPas.throwingBiConsumer;
+import static org.zalando.riptide.micrometer.CompletableFutures.onError;
+import static org.zalando.riptide.micrometer.CompletableFutures.onResult;
 
 @API(status = EXPERIMENTAL)
 @AllArgsConstructor(access = PRIVATE)
@@ -106,16 +106,8 @@ public RequestExecution aroundNetwork(final RequestExecution execution) {
             final Measurement measurement = new Measurement(arguments);
 
             return execution.execute(arguments)
-                    .whenComplete(throwingBiConsumer((response, throwable) -> {
-                        if (nonNull(response)) {
-                            measurement.record(response);
-                        }
-                    }))
-                    .whenComplete((response, throwable) -> {
-                        if (nonNull(throwable)) {
-                            measurement.record(throwable);
-                        }
-                    });
+                    .whenComplete(onResult(measurement::record))
+                    .whenComplete(onError(measurement::record));
         };
     }
 
@@ -126,23 +118,22 @@ private final class Measurement {
         private final RequestArguments arguments;
 
         void record(final ClientHttpResponse response) throws IOException {
-            record(timer ->
-                    timer.tags(generator.onResponse(arguments, response)));
+            record(() -> generator.onResponse(arguments, response));
         }
 
         void record(final Throwable throwable) {
-            record(timer ->
-                    timer.tags(generator.onError(arguments, throwable)));
+            record(() -> generator.onError(arguments, throwable));
         }
 
         <X extends Exception> void record(
-                final ThrowingUnaryOperator<Timer.Builder, X> build) throws X {
+                final ThrowingSupplier<Iterable<Tag>, X> tags) throws X {
 
-            final Timer.Builder builder = build.tryApply(builder(metricName)
+            final Timer timer = builder(metricName)
                     .tags(defaultTags)
-                    .tags(generator.onRequest(arguments)));
+                    .tags(generator.onRequest(arguments))
+                    .tags(tags.tryGet())
+                    .register(registry);
 
-            final Timer timer = builder.register(registry);
             sample.stop(timer);
         }
 

diff --git a/...ide-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/CallSiteTagGenerator.java b/...ide-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/CallSiteTagGenerator.java
@@ -8,6 +8,13 @@
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 import static org.zalando.riptide.micrometer.MicrometerPlugin.TAGS;
 
+/**
+ * <strong>Beware</strong> that some meter registries, e.g. Prometheus, only
+ * allow a fixed set of tag keys for the same metric. That means if tags are
+ * being passed from a call site they should be passed for all calls. Either
+ * by adjusting all call sites or by defining a custom tag generator that
+ * provides default values.
+ */
 @API(status = EXPERIMENTAL)
 public final class CallSiteTagGenerator implements TagGenerator {
 

diff --git a/...de-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/CompositeTagGenerator.java b/...de-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/CompositeTagGenerator.java
@@ -1,13 +1,15 @@
 package org.zalando.riptide.micrometer.tag;
 
+import com.google.common.collect.Iterables;
 import io.micrometer.core.instrument.Tag;
-import io.micrometer.core.instrument.Tags;
 import lombok.AllArgsConstructor;
 import org.springframework.http.client.ClientHttpResponse;
 import org.zalando.riptide.RequestArguments;
 
-import java.util.Collections;
+import java.util.stream.Stream;
 
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.toList;
 import static java.util.stream.StreamSupport.stream;
 import static org.zalando.fauxpas.FauxPas.throwingFunction;
 
@@ -18,34 +20,34 @@ final class CompositeTagGenerator implements TagGenerator {
 
     @Override
     public Iterable<Tag> onRequest(final RequestArguments arguments) {
-
-        return stream(generators.spliterator(), false)
+        return generators()
                 .map(generator -> generator.onRequest(arguments))
-                .reduce(Tags::concat)
-                .orElse(Collections.emptyList());
+                .collect(collectingAndThen(toList(), Iterables::concat));
     }
 
     @Override
     public Iterable<Tag> onResponse(
             final RequestArguments arguments,
             final ClientHttpResponse response) {
 
-        return stream(generators.spliterator(), false)
+        return generators()
                 .map(throwingFunction(generator ->
                         generator.onResponse(arguments, response)))
-                .reduce(Tags::concat)
-                .orElse(Collections.emptyList());
+                .collect(collectingAndThen(toList(), Iterables::concat));
     }
 
     @Override
     public Iterable<Tag> onError(
             final RequestArguments arguments,
             final Throwable throwable) {
 
-        return stream(generators.spliterator(), false)
+        return generators()
                 .map(generator -> generator.onError(arguments, throwable))
-                .reduce(Tags::concat)
-                .orElse(Collections.emptyList());
+                .collect(collectingAndThen(toList(), Iterables::concat));
+    }
+
+    private Stream<TagGenerator> generators() {
+        return stream(generators.spliterator(), false);
     }
 
 }
diff --git a/...de-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/ErrorKindTagGenerator.java b/...de-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/ErrorKindTagGenerator.java
@@ -4,8 +4,10 @@
 import io.micrometer.core.instrument.Tag;
 import lombok.AllArgsConstructor;
 import org.apiguardian.api.API;
+import org.springframework.http.client.ClientHttpResponse;
 import org.zalando.riptide.RequestArguments;
 
+import java.util.Set;
 import java.util.function.UnaryOperator;
 
 import static java.util.Collections.singleton;
@@ -15,20 +17,31 @@
 @AllArgsConstructor
 public final class ErrorKindTagGenerator implements TagGenerator {
 
+    private static final String ERROR_KIND = "error.kind";
+    private static final Set<Tag> NONE = singleton(Tag.of(ERROR_KIND, "none"));
+
     private final UnaryOperator<Throwable> extractor;
 
     public ErrorKindTagGenerator() {
         this(Throwables::getRootCause);
     }
 
+    @Override
+    public Iterable<Tag> onResponse(
+            final RequestArguments arguments,
+            final ClientHttpResponse response) {
+
+        return NONE;
+    }
+
     @Override
     public Iterable<Tag> onError(
             final RequestArguments arguments,
             final Throwable throwable) {
 
         final Throwable error = extractor.apply(throwable);
         final String name = error.getClass().getSimpleName();
-        return singleton(Tag.of("error.kind", name));
+        return singleton(Tag.of(ERROR_KIND, name));
     }
 
 }
diff --git a/...ide-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/HttpPathTagGenerator.java b/...ide-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/HttpPathTagGenerator.java
@@ -6,9 +6,8 @@
 
 import javax.annotation.Nullable;
 
-import static java.util.Collections.emptyList;
+import static com.google.common.base.MoreObjects.firstNonNull;
 import static java.util.Collections.singleton;
-import static java.util.Objects.nonNull;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 @API(status = EXPERIMENTAL)
@@ -17,11 +16,7 @@ public final class HttpPathTagGenerator implements TagGenerator {
     @Override
     public Iterable<Tag> onRequest(final RequestArguments arguments) {
         @Nullable final String uriTemplate = arguments.getUriTemplate();
-
-        if (nonNull(uriTemplate)) {
-            return singleton(Tag.of("http.path", uriTemplate));
-        }
-
-        return emptyList();
+        return singleton(Tag.of("http.path", firstNonNull(uriTemplate, "")));
     }
+
 }
diff --git a/...e-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/HttpStatusTagGenerator.java b/...e-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/HttpStatusTagGenerator.java
@@ -6,20 +6,32 @@
 import org.zalando.riptide.RequestArguments;
 
 import java.io.IOException;
+import java.util.Set;
 
 import static java.util.Collections.singleton;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 @API(status = EXPERIMENTAL)
 public final class HttpStatusTagGenerator implements TagGenerator {
 
+    private static final String STATUS_CODE = "http.status_code";
+    private static final Set<Tag> NONE = singleton(Tag.of(STATUS_CODE, "0"));
+
     @Override
     public Iterable<Tag> onResponse(
             final RequestArguments arguments,
             final ClientHttpResponse response) throws IOException {
 
         final String status = String.valueOf(response.getRawStatusCode());
-        return singleton(Tag.of("http.status_code", status));
+        return singleton(Tag.of(STATUS_CODE, status));
+    }
+
+    @Override
+    public Iterable<Tag> onError(
+            final RequestArguments arguments,
+            final Throwable throwable) {
+
+        return NONE;
     }
 
 }
diff --git a/riptide-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/RetryTagGenerator.java b/riptide-micrometer/src/main/java/org/zalando/riptide/micrometer/tag/RetryTagGenerator.java
@@ -3,11 +3,9 @@
 import io.micrometer.core.instrument.Tag;
 import org.zalando.riptide.Attributes;
 import org.zalando.riptide.RequestArguments;
-import org.zalando.riptide.micrometer.tag.TagGenerator;
 
-import java.util.Collections;
-
-import static java.util.Collections.emptySet;
+import static java.util.Collections.singleton;
+import static org.zalando.riptide.Attributes.RETRIES;
 
 /**
  * @see Attributes#RETRIES
@@ -16,11 +14,8 @@ public final class RetryTagGenerator implements TagGenerator {
 
     @Override
     public Iterable<Tag> onRequest(final RequestArguments arguments) {
-        return arguments.getAttribute(Attributes.RETRIES)
-                .map(String::valueOf)
-                .map(attempts -> Tag.of("retry_number", attempts))
-                .map(Collections::singleton)
-                .orElse(emptySet());
+        final int retries = arguments.getAttribute(RETRIES).orElse(0);
+        return singleton(Tag.of("retry_number", String.valueOf(retries)));
     }
 
 }