Merge branch '1.13.x' into 1.14.x

Author: shakuzen

docs/modules/ROOT/pages/concepts/counters.adoc

@@ -153,3 +153,11 @@ FunctionCounter counter = FunctionCounter
     .tags("region", "test") // optional
     .register(registry);
 ----
+
+WARNING: Attempting to construct a function-tracking counter with a primitive number or one of its `java.lang` object forms is always incorrect. These numbers are immutable. Thus, the function-tracking counter cannot ever be changed. Attempting to "re-register" the function-tracking counter with a different number does not work, as the registry maintains only one meter for each unique combination of name and tags. "Re-registering" a function-tracking counter can happen indirectly for example as the result of a `MeterFilter` modifying the name and/or the tags of two different function-tracking counter so that they will be the same after the filter is applied.
+
+Attempting to "re-register" a function-tracking counter will result in a warning like this:
+[source]
+----
+WARNING: This FunctionCounter has been already registered (MeterId{name='my.fc', tags=[]}), the registration will be ignored. Note that subsequent logs will be logged at debug level.
+----

docs/modules/ROOT/pages/concepts/gauges.adoc

@@ -44,6 +44,12 @@ This pattern should be less common than the `DoubleFunction` form. Remember that
 
 WARNING: Attempting to construct a gauge with a primitive number or one of its `java.lang` object forms is always incorrect. These numbers are immutable. Thus, the gauge cannot ever be changed. Attempting to "re-register" the gauge with a different number does not work, as the registry maintains only one meter for each unique combination of name and tags. "Re-registering" a gauge can happen indirectly for example as the result of a `MeterFilter` modifying the name and/or the tags of two different gauges so that they will be the same after the filter is applied.
 
+Attempting to "re-register" a gauge will result in a warning like this:
+[source]
+----
+WARNING: This Gauge has been already registered (MeterId{name='my.gauge', tags=[]}), the registration will be ignored. Note that subsequent logs will be logged at debug level.
+----
+
 == Gauge Fluent Builder
 
 The interface contains a fluent builder for gauges:

docs/modules/ROOT/pages/concepts/timers.adoc

@@ -186,6 +186,14 @@ FunctionTimer.builder("cache.gets.latency", cache,
     .register(registry);
 ----
 
+WARNING: Attempting to construct a function-tracking timer with a primitive number or one of its `java.lang` object forms is always incorrect. These numbers are immutable. Thus, the function-tracking timer cannot ever be changed. Attempting to "re-register" the function-tracking timer with a different number does not work, as the registry maintains only one meter for each unique combination of name and tags. "Re-registering" a function-tracking timer can happen indirectly for example as the result of a `MeterFilter` modifying the name and/or the tags of two different function-tracking timer so that they will be the same after the filter is applied.
+
+Attempting to "re-register" a function-tracking timer will result in a warning like this:
+[source]
+----
+WARNING: This FunctionTimer has been already registered (MeterId{name='my.ft', tags=[]}), the registration will be ignored. Note that subsequent logs will be logged at debug level.
+----
+
 == Pause Detection
 
 Micrometer uses the `LatencyUtils` package to compensate for https://highscalability.com/blog/2015/10/5/your-load-generator-is-probably-lying-to-you-take-the-red-pi.html[coordinated omission] -- extra latency arising from system and VM pauses that skew your latency statistics downward. Distribution statistics, such as percentiles and SLO counts, are influenced by a pause detector implementation that adds additional latency here and there to compensate for pauses.

micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java

@@ -67,8 +67,7 @@
  */
 public abstract class MeterRegistry {
 
-    private static final WarnThenDebugLogger gaugeDoubleRegistrationLogger = new WarnThenDebugLogger(
-            MeterRegistry.class);
+    private static final WarnThenDebugLogger doubleRegistrationLogger = new WarnThenDebugLogger(MeterRegistry.class);
 
     // @formatter:off
     private static final EnumMap<TimeUnit, String> BASE_TIME_UNIT_STRING_CACHE = Arrays.stream(TimeUnit.values())
@@ -640,7 +639,7 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
 
         Meter m = preFilterIdToMeterMap.get(originalId);
         if (m != null && !isStaleId(originalId)) {
-            checkAndWarnAboutGaugeDoubleRegistration(m);
+            checkAndWarnAboutDoubleRegistration(m);
             return m;
         }
 
@@ -653,7 +652,7 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
             if (isStaleId(originalId)) {
                 unmarkStaleId(originalId);
             }
-            checkAndWarnAboutGaugeDoubleRegistration(m);
+            checkAndWarnAboutDoubleRegistration(m);
         }
         else {
             if (isClosed()) {
@@ -712,12 +711,21 @@ private boolean unmarkStaleId(Id originalId) {
         return !stalePreFilterIds.isEmpty() && stalePreFilterIds.remove(originalId);
     }
 
-    private void checkAndWarnAboutGaugeDoubleRegistration(Meter meter) {
-        if (meter instanceof Gauge) {
-            gaugeDoubleRegistrationLogger.log(() -> String.format(
-                    "This Gauge has been already registered (%s), the Gauge registration will be ignored.",
-                    meter.getId()));
+    private void checkAndWarnAboutDoubleRegistration(Meter meter) {
+        if (meter instanceof Gauge) { // also TimeGauge
+            warnAboutDoubleRegistration("Gauge", meter.getId());
         }
+        else if (meter instanceof FunctionCounter) {
+            warnAboutDoubleRegistration("FunctionCounter", meter.getId());
+        }
+        else if (meter instanceof FunctionTimer) {
+            warnAboutDoubleRegistration("FunctionTimer", meter.getId());
+        }
+    }
+
+    private void warnAboutDoubleRegistration(String type, Meter.Id id) {
+        doubleRegistrationLogger.log(() -> String
+            .format("This %s has been already registered (%s), the registration will be ignored.", type, id));
     }
 
     private boolean accept(Meter.Id id) {

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/kafka/KafkaMetrics.java

@@ -150,7 +150,7 @@ private Iterable<Tag> getCommonTags(MeterRegistry registry) {
         // FIXME hack until we have proper API to retrieve common tags
         Meter.Id dummyId = Meter.builder("delete.this", OTHER, Collections.emptyList()).register(registry).getId();
         registry.remove(dummyId);
-        return dummyId.getTags();
+        return dummyId.getTagsAsIterable();
     }
 
     /**
@@ -191,74 +191,77 @@ void checkAndBindMetrics(MeterRegistry registry) {
             Map<MetricName, ? extends Metric> currentMetrics = this.metricsSupplier.get();
             this.metrics.set(currentMetrics);
 
-            if (!currentMeters.equals(currentMetrics.keySet())) {
-                Set<MetricName> metricsToRemove = currentMeters.stream()
-                    .filter(metricName -> !currentMetrics.containsKey(metricName))
-                    .collect(Collectors.toSet());
+            if (currentMeters.equals(currentMetrics.keySet())) {
+                return;
+            }
 
-                for (MetricName metricName : metricsToRemove) {
-                    Meter.Id id = meterIdForComparison(metricName);
-                    registry.remove(id);
-                    registeredMeterIds.remove(id);
-                }
+            Set<MetricName> metricsToRemove = currentMeters.stream()
+                .filter(metricName -> !currentMetrics.containsKey(metricName))
+                .collect(Collectors.toSet());
+
+            for (MetricName metricName : metricsToRemove) {
+                Meter.Id id = meterIdForComparison(metricName);
+                registry.remove(id);
+                registeredMeterIds.remove(id);
+            }
 
-                currentMeters = new HashSet<>(currentMetrics.keySet());
+            currentMeters = new HashSet<>(currentMetrics.keySet());
 
-                Map<String, List<Meter>> registryMetersByNames = registry.getMeters()
-                    .stream()
-                    .collect(Collectors.groupingBy(meter -> meter.getId().getName()));
+            Map<String, List<Meter>> registryMetersByNames = registry.getMeters()
+                .stream()
+                .collect(Collectors.groupingBy(meter -> meter.getId().getName()));
 
-                currentMetrics.forEach((name, metric) -> {
-                    // Filter out non-numeric values
-                    // Filter out metrics from groups that include metadata
-                    if (!(metric.metricValue() instanceof Number) || METRIC_GROUP_APP_INFO.equals(name.group())
-                            || METRIC_GROUP_METRICS_COUNT.equals(name.group())) {
-                        return;
-                    }
+            currentMetrics.forEach((name, metric) -> {
+                // Filter out non-numeric values
+                // Filter out metrics from groups that include metadata
+                if (!(metric.metricValue() instanceof Number) || METRIC_GROUP_APP_INFO.equals(name.group())
+                        || METRIC_GROUP_METRICS_COUNT.equals(name.group())) {
+                    return;
+                }
 
-                    String meterName = meterName(name);
-
-                    // Kafka has metrics with lower number of tags (e.g. with/without
-                    // topic or partition tag)
-                    // Remove meters with lower number of tags
-                    boolean hasLessTags = false;
-                    for (Meter other : registryMetersByNames.getOrDefault(meterName, emptyList())) {
-                        Meter.Id otherId = other.getId();
-                        List<Tag> tags = otherId.getTags();
-                        List<Tag> meterTagsWithCommonTags = meterTags(name, true);
-                        if (tags.size() < meterTagsWithCommonTags.size()) {
-                            registry.remove(otherId);
-                            registeredMeterIds.remove(otherId);
-                        }
-                        // Check if already exists
-                        else if (tags.size() == meterTagsWithCommonTags.size()) {
-                            if (tags.containsAll(meterTagsWithCommonTags))
-                                return;
-                        }
-                        else
-                            hasLessTags = true;
+                String meterName = meterName(name);
+
+                // Kafka has metrics with lower number of tags (e.g. with/without
+                // topic or partition tag)
+                // Remove meters with lower number of tags
+                boolean hasLessTags = false;
+                for (Meter other : registryMetersByNames.getOrDefault(meterName, emptyList())) {
+                    Meter.Id otherId = other.getId();
+                    List<Tag> otherTags = otherId.getTags();
+                    List<Tag> meterTagsWithCommonTags = meterTags(name, true);
+                    if (otherTags.size() < meterTagsWithCommonTags.size()) {
+                        registry.remove(otherId);
+                        registeredMeterIds.remove(otherId);
                     }
-                    if (hasLessTags)
-                        return;
-
-                    List<Tag> tags = meterTags(name);
-                    try {
-                        Meter meter = bindMeter(registry, metric, meterName, tags);
-                        List<Meter> meters = registryMetersByNames.computeIfAbsent(meterName, k -> new ArrayList<>());
-                        meters.add(meter);
+                    // Check if already exists
+                    else if (otherTags.size() == meterTagsWithCommonTags.size()) {
+                        // https://www.jetbrains.com/help/inspectopedia/SlowListContainsAll.html
+                        if (new HashSet<>(otherTags).containsAll(meterTagsWithCommonTags))
+                            return;
                     }
-                    catch (Exception ex) {
-                        String message = ex.getMessage();
-                        if (message != null && message.contains("Prometheus requires")) {
-                            warnThenDebugLogger.log(() -> "Failed to bind meter: " + meterName + " " + tags
-                                    + ". However, this could happen and might be restored in the next refresh.");
-                        }
-                        else {
-                            log.warn("Failed to bind meter: " + meterName + " " + tags + ".", ex);
-                        }
+                    else
+                        hasLessTags = true;
+                }
+                if (hasLessTags)
+                    return;
+
+                List<Tag> tags = meterTags(name);
+                try {
+                    Meter meter = bindMeter(registry, metric, meterName, tags);
+                    List<Meter> meters = registryMetersByNames.computeIfAbsent(meterName, k -> new ArrayList<>());
+                    meters.add(meter);
+                }
+                catch (Exception ex) {
+                    String message = ex.getMessage();
+                    if (message != null && message.contains("Prometheus requires")) {
+                        warnThenDebugLogger.log(() -> "Failed to bind meter: " + meterName + " " + tags
+                                + ". However, this could happen and might be restored in the next refresh.");
                     }
-                });
-            }
+                    else {
+                        log.warn("Failed to bind meter: " + meterName + " " + tags + ".", ex);
+                    }
+                }
+            });
         }
         catch (Exception e) {
             log.warn("Failed to bind KafkaMetric", e);