Polish JettyClientMetrics changes (#2036)

Author: izeye

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyClientMetrics.java

@@ -18,8 +18,10 @@
 import io.micrometer.core.annotation.Incubating;
 import io.micrometer.core.instrument.DistributionSummary;
 import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Timer;
 import io.micrometer.core.instrument.config.MeterFilter;
+import io.micrometer.core.instrument.internal.OnlyOnceLoggingDenyMeterFilter;
 import org.eclipse.jetty.client.api.Request;
 
 /**
@@ -43,7 +45,15 @@ protected JettyClientMetrics(MeterRegistry registry, JettyClientTagsProvider tag
         this.timingMetricName = timingMetricName;
         this.contentSizeMetricName = contentSizeMetricName;
 
-        registry.config().meterFilter(MeterFilter.maximumAllowableTags(this.timingMetricName, "uri", maxUriTags, MeterFilter.deny()));
+        MeterFilter timingMetricDenyFilter = new OnlyOnceLoggingDenyMeterFilter(
+                () -> String.format("Reached the maximum number of URI tags for '%s'.", timingMetricName));
+        MeterFilter contentSizeMetricDenyFilter = new OnlyOnceLoggingDenyMeterFilter(
+                () -> String.format("Reached the maximum number of URI tags for '%s'.", contentSizeMetricName));
+        registry.config()
+                .meterFilter(MeterFilter.maximumAllowableTags(
+                        this.timingMetricName, "uri", maxUriTags, timingMetricDenyFilter))
+                .meterFilter(MeterFilter.maximumAllowableTags(
+                        this.contentSizeMetricName, "uri", maxUriTags, contentSizeMetricDenyFilter));
     }
 
     @Override
@@ -52,17 +62,18 @@ public void onQueued(Request request) {
 
         request.onComplete(result -> {
             long requestLength = result.getRequest().getContent().getLength();
+            Iterable<Tag> httpRequestTags = tagsProvider.httpRequestTags(result);
             if (requestLength >= 0) {
                 DistributionSummary.builder(contentSizeMetricName)
                         .description("Content sizes for Jetty HTTP client requests")
-                        .tags(tagsProvider.httpRequestTags(result))
+                        .tags(httpRequestTags)
                         .register(registry)
                         .record(requestLength);
             }
 
             sample.stop(Timer.builder(timingMetricName)
                     .description("Jetty HTTP client request timing")
-                    .tags(tagsProvider.httpRequestTags(result))
+                    .tags(httpRequestTags)
                     .register(registry));
         });
     }

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyClientTags.java

@@ -16,6 +16,7 @@
 package io.micrometer.core.instrument.binder.jetty;
 
 import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.binder.http.Outcome;
 import io.micrometer.core.instrument.util.StringUtils;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
@@ -42,20 +43,6 @@ public final class JettyClientTags {
 
     private static final Tag EXCEPTION_NONE = Tag.of("exception", "None");
 
-    private static final Tag STATUS_SERVER_ERROR = Tag.of("status", String.valueOf(HttpStatus.INTERNAL_SERVER_ERROR_500));
-
-    private static final Tag OUTCOME_UNKNOWN = Tag.of("outcome", "UNKNOWN");
-
-    private static final Tag OUTCOME_INFORMATIONAL = Tag.of("outcome", "INFORMATIONAL");
-
-    private static final Tag OUTCOME_SUCCESS = Tag.of("outcome", "SUCCESS");
-
-    private static final Tag OUTCOME_REDIRECTION = Tag.of("outcome", "REDIRECTION");
-
-    private static final Tag OUTCOME_CLIENT_ERROR = Tag.of("outcome", "CLIENT_ERROR");
-
-    private static final Tag OUTCOME_SERVER_ERROR = Tag.of("outcome", "SERVER_ERROR");
-
     private static final Tag METHOD_UNKNOWN = Tag.of("method", "UNKNOWN");
 
     private static final Pattern TRAILING_SLASH_PATTERN = Pattern.compile("/$");
@@ -69,7 +56,7 @@ private JettyClientTags() {
      * Creates a {@code method} tag based on the {@link Request#getMethod()
      * method} of the given {@code request}.
      *
-     * @param request the container request
+     * @param request the request
      * @return the method tag whose value is a capitalized method (e.g. GET).
      */
     public static Tag method(Request request) {
@@ -107,20 +94,16 @@ public static Tag uri(Result result, Function<Result, String> successfulUriPatte
         }
 
         String matchingPattern = successfulUriPattern.apply(result);
+        matchingPattern = MULTIPLE_SLASH_PATTERN.matcher(matchingPattern).replaceAll("/");
         if (matchingPattern.equals("/")) {
             return URI_ROOT;
         }
-
-        matchingPattern = MULTIPLE_SLASH_PATTERN.matcher(matchingPattern).replaceAll("/");
-        if (!matchingPattern.equals("/")) {
-            matchingPattern = TRAILING_SLASH_PATTERN.matcher(matchingPattern).replaceAll("");
-        }
-
+        matchingPattern = TRAILING_SLASH_PATTERN.matcher(matchingPattern).replaceAll("");
         return Tag.of("uri", matchingPattern);
     }
 
     /**
-     * Creates a {@code exception} tag based on the {@link Class#getSimpleName() simple
+     * Creates an {@code exception} tag based on the {@link Class#getSimpleName() simple
      * name} of the class of the given {@code exception}.
      *
      * @param result the request result
@@ -152,19 +135,7 @@ public static Tag exception(Result result) {
      * @return the outcome tag derived from the status of the response
      */
     public static Tag outcome(Result result) {
-        int status = result.getResponse().getStatus();
-        if (HttpStatus.isInformational(status)) {
-            return OUTCOME_INFORMATIONAL;
-        } else if (HttpStatus.isSuccess(status)) {
-            return OUTCOME_SUCCESS;
-        } else if (HttpStatus.isRedirection(status)) {
-            return OUTCOME_REDIRECTION;
-        } else if (HttpStatus.isClientError(status)) {
-            return OUTCOME_CLIENT_ERROR;
-        } else if (HttpStatus.isServerError(status)) {
-            return OUTCOME_SERVER_ERROR;
-        }
-        return OUTCOME_UNKNOWN;
+        return Outcome.forStatus(result.getResponse().getStatus()).asTag();
     }
 
 }

micrometer-core/src/main/java/io/micrometer/core/instrument/internal/OnlyOnceLoggingDenyMeterFilter.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2012-2019 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.micrometer.core.instrument.internal;
+
+import io.micrometer.core.instrument.Meter.Id;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.micrometer.core.instrument.config.MeterFilterReply;
+import io.micrometer.core.util.internal.logging.InternalLogger;
+import io.micrometer.core.util.internal.logging.InternalLoggerFactory;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
+
+/**
+ * {@link MeterFilter} to log only once a warning message and deny a {@link Id Meter.Id}.
+ *
+ * @author Jon Schneider
+ * @author Dmytro Nosan
+ * @since 1.5.0
+ */
+public final class OnlyOnceLoggingDenyMeterFilter implements MeterFilter {
+
+    private static final InternalLogger logger = InternalLoggerFactory.getInstance(OnlyOnceLoggingDenyMeterFilter.class);
+
+    private final AtomicBoolean alreadyWarned = new AtomicBoolean(false);
+
+    private final Supplier<String> message;
+
+    public OnlyOnceLoggingDenyMeterFilter(Supplier<String> message) {
+        this.message = message;
+    }
+
+    @Override
+    public MeterFilterReply accept(Id id) {
+        if (logger.isWarnEnabled() && this.alreadyWarned.compareAndSet(false, true)) {
+            logger.warn(this.message.get());
+        }
+        return MeterFilterReply.DENY;
+    }
+
+}