From 2f148cad99c098d32d88b24017b96a552eda3e02 Mon Sep 17 00:00:00 2001 From: ramondebeijer Date: Fri, 13 Dec 2019 09:05:16 +0100 Subject: [PATCH 1/3] Truncate tag values > 1024 for Stackdriver (#1750) Limit the length of the TagValue of a metric to 1024 according to the Stackdriver reference documentation. --- .../stackdriver/StackdriverMeterRegistry.java | 50 +++++++++++++------ .../StackdriverNamingConvention.java | 18 +++++-- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java index 54a8e3296c..65452c586b 100644 --- a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java +++ b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java @@ -15,6 +15,21 @@ */ package io.micrometer.stackdriver; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import com.google.api.Distribution; import com.google.api.Metric; import com.google.api.MetricDescriptor; @@ -22,10 +37,25 @@ import com.google.api.gax.rpc.ApiException; import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.monitoring.v3.MetricServiceSettings; -import com.google.monitoring.v3.*; +import com.google.monitoring.v3.CreateMetricDescriptorRequest; +import com.google.monitoring.v3.CreateTimeSeriesRequest; +import com.google.monitoring.v3.Point; +import com.google.monitoring.v3.ProjectName; +import com.google.monitoring.v3.TimeInterval; +import com.google.monitoring.v3.TimeSeries; +import com.google.monitoring.v3.TypedValue; import com.google.protobuf.Timestamp; import io.micrometer.core.annotation.Incubating; -import io.micrometer.core.instrument.*; +import io.micrometer.core.instrument.Clock; +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; +import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.FunctionTimer; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.LongTaskTimer; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.TimeGauge; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.distribution.CountAtBucket; @@ -42,17 +72,6 @@ import io.micrometer.core.lang.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - -import java.util.*; -import java.util.concurrent.Callable; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Collectors; -import java.util.stream.Stream; - import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toCollection; import static java.util.stream.StreamSupport.stream; @@ -65,6 +84,7 @@ */ @Incubating(since = "1.1.0") public class StackdriverMeterRegistry extends StepMeterRegistry { + private static final ThreadFactory DEFAULT_THREAD_FACTORY = new NamedThreadFactory("stackdriver-metrics-publisher"); /** @@ -128,7 +148,7 @@ public void start(ThreadFactory threadFactory) { } else { try { this.client = MetricServiceClient.create(metricServiceSettings); - logger.info("publishing metrics to stackdriver every " + TimeUtils.format(config.step())); + logger.info("publishing metrics to stackdriver every {}", TimeUtils.format(config.step())); super.start(threadFactory); } catch (Exception e) { logger.error("unable to create stackdriver client", e); @@ -422,7 +442,7 @@ private Distribution distribution(HistogramSnapshot snapshot, boolean timeDomain // add the "+infinity" bucket, which does NOT have a corresponding bucket boundary bucketCounts.add(snapshot.count() - truncatedSum.get()); - + List bucketBoundaries = Arrays.stream(histogram) .map(countAtBucket -> timeDomain ? countAtBucket.bucket(getBaseTimeUnit()) : countAtBucket.bucket()) .collect(toCollection(ArrayList::new)); diff --git a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java index 108b690ee4..459c6d07f6 100644 --- a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java +++ b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java @@ -15,26 +15,31 @@ */ package io.micrometer.stackdriver; +import java.util.regex.Pattern; + import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.config.NamingConvention; import io.micrometer.core.instrument.util.StringUtils; import io.micrometer.core.lang.Nullable; -import java.util.regex.Pattern; - /** * {@link NamingConvention} for Stackdriver. - * + * * Names are mapped to Stackdriver's metric type names and tag keys are mapped to its metric label names. * - * @see "Naming rules" section on Stackdriver's reference documentation * + * @see "Naming rules" section on Stackdriver's reference documentation + * and + * @see "Custom Metrics" on the Stackdriver's Quotas and limits reference documentation + * * @author Jon Schneider * @since 1.1.0 */ public class StackdriverNamingConvention implements NamingConvention { + private static final int MAX_NAME_LENGTH = 200; private static final int MAX_TAG_KEY_LENGTH = 100; + private static final int MAX_TAG_VALUE_LENGTH = 1024; private static final Pattern NAME_BLACKLIST = Pattern.compile("[^\\w./_]"); private static final Pattern TAG_KEY_BLACKLIST = Pattern.compile("[^\\w_]"); private final NamingConvention nameDelegate; @@ -62,4 +67,9 @@ private String sanitize(String value, Pattern blacklist, int maxLength) { public String tagKey(String key) { return sanitize(tagKeyDelegate.tagKey(key), TAG_KEY_BLACKLIST, MAX_TAG_KEY_LENGTH); } + + @Override + public String tagValue(final String value) { + return StringUtils.truncate(value, MAX_TAG_VALUE_LENGTH); + } } From 1b825ed691e82e10822e1b9b673588d35cdbb5c5 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Fri, 13 Dec 2019 17:11:30 +0900 Subject: [PATCH 2/3] Polish Restore project conventions for previous contribution. --- .../stackdriver/StackdriverMeterRegistry.java | 49 ++++++------------- .../StackdriverNamingConvention.java | 2 +- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java index 65452c586b..f7048b0aa5 100644 --- a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java +++ b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverMeterRegistry.java @@ -15,21 +15,6 @@ */ package io.micrometer.stackdriver; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.Callable; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Collectors; -import java.util.stream.Stream; - import com.google.api.Distribution; import com.google.api.Metric; import com.google.api.MetricDescriptor; @@ -37,26 +22,11 @@ import com.google.api.gax.rpc.ApiException; import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.monitoring.v3.MetricServiceSettings; -import com.google.monitoring.v3.CreateMetricDescriptorRequest; -import com.google.monitoring.v3.CreateTimeSeriesRequest; -import com.google.monitoring.v3.Point; -import com.google.monitoring.v3.ProjectName; -import com.google.monitoring.v3.TimeInterval; -import com.google.monitoring.v3.TimeSeries; -import com.google.monitoring.v3.TypedValue; +import com.google.monitoring.v3.*; import com.google.protobuf.Timestamp; import io.micrometer.core.annotation.Incubating; -import io.micrometer.core.instrument.Clock; -import io.micrometer.core.instrument.Counter; -import io.micrometer.core.instrument.DistributionSummary; -import io.micrometer.core.instrument.FunctionCounter; -import io.micrometer.core.instrument.FunctionTimer; -import io.micrometer.core.instrument.Gauge; -import io.micrometer.core.instrument.LongTaskTimer; -import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.Tag; -import io.micrometer.core.instrument.TimeGauge; import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.distribution.CountAtBucket; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; @@ -72,6 +42,17 @@ import io.micrometer.core.lang.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + +import java.util.*; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toCollection; import static java.util.stream.StreamSupport.stream; @@ -84,7 +65,7 @@ */ @Incubating(since = "1.1.0") public class StackdriverMeterRegistry extends StepMeterRegistry { - + private static final ThreadFactory DEFAULT_THREAD_FACTORY = new NamedThreadFactory("stackdriver-metrics-publisher"); /** @@ -442,7 +423,7 @@ private Distribution distribution(HistogramSnapshot snapshot, boolean timeDomain // add the "+infinity" bucket, which does NOT have a corresponding bucket boundary bucketCounts.add(snapshot.count() - truncatedSum.get()); - + List bucketBoundaries = Arrays.stream(histogram) .map(countAtBucket -> timeDomain ? countAtBucket.bucket(getBaseTimeUnit()) : countAtBucket.bucket()) .collect(toCollection(ArrayList::new)); diff --git a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java index 459c6d07f6..58ef99ce20 100644 --- a/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java +++ b/implementations/micrometer-registry-stackdriver/src/main/java/io/micrometer/stackdriver/StackdriverNamingConvention.java @@ -69,7 +69,7 @@ public String tagKey(String key) { } @Override - public String tagValue(final String value) { + public String tagValue(String value) { return StringUtils.truncate(value, MAX_TAG_VALUE_LENGTH); } } From 099fa5acb53078e1a9c4f87cd014571008c0fb03 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Fri, 13 Dec 2019 17:15:02 +0900 Subject: [PATCH 3/3] Add test for Stackdriver tag value length limit --- .../stackdriver/StackdriverNamingConventionTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/implementations/micrometer-registry-stackdriver/src/test/java/io/micrometer/stackdriver/StackdriverNamingConventionTest.java b/implementations/micrometer-registry-stackdriver/src/test/java/io/micrometer/stackdriver/StackdriverNamingConventionTest.java index 882dbc3a75..9c7054d835 100644 --- a/implementations/micrometer-registry-stackdriver/src/test/java/io/micrometer/stackdriver/StackdriverNamingConventionTest.java +++ b/implementations/micrometer-registry-stackdriver/src/test/java/io/micrometer/stackdriver/StackdriverNamingConventionTest.java @@ -44,4 +44,10 @@ void tagValue() { assertThat(namingConvention.tagValue("my.tag.value")).isEqualTo("my.tag.value"); } + @Test + void tooLongTagValue() { + assertThat(namingConvention.tagValue("thisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolong")) + .isEqualTo("thisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistoolongthisistool"); + } + }