From 9587003d44d30660bc7ea073cd753e22c41463f2 Mon Sep 17 00:00:00 2001
From: Jon Schneider <jkschneider@gmail.com>
Date: Thu, 1 Sep 2016 14:32:30 -0700
Subject: [PATCH] Sanitize Servo metric tag key/value pairs before publishing
 to Atlas (#1315)

---
 .../metrics/atlas/AtlasMetricObserver.java    | 26 ++++++++++++++++---
 .../atlas/AtlasMetricObserverTests.java       | 18 +++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserver.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserver.java
index 42bc9225..540785d5 100644
--- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserver.java
+++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserver.java
@@ -38,6 +38,7 @@ import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.dataformat.smile.SmileFactory;
 import com.netflix.servo.Metric;
 import com.netflix.servo.annotations.DataSourceType;
+import com.netflix.servo.monitor.MonitorConfig;
 import com.netflix.servo.publish.MetricObserver;
 import com.netflix.servo.tag.BasicTag;
 import com.netflix.servo.tag.Tag;
@@ -96,7 +97,7 @@ public class AtlasMetricObserver implements MetricObserver {
 		return true;
 	}
 
-	protected static String normalizeAtlasUri(String uri) {
+	static String normalizeAtlasUri(String uri) {
 		if (uri != null) {
 			Matcher matcher = Pattern.compile("(.+?)(/api/v1/publish)?/?").matcher(uri);
 			if (matcher.matches())
@@ -119,7 +120,7 @@ public class AtlasMetricObserver implements MetricObserver {
 			return;
 		}
 
-		List<Metric> metrics = addTypeTagsAsNecessary(rawMetrics);
+		List<Metric> metrics = sanitizeTags(addTypeTagsAsNecessary(rawMetrics));
 
 		for (int i = 0; i < metrics.size(); i += config.getBatchSize()) {
 			List<Metric> batch = metrics.subList(i,
@@ -217,8 +218,25 @@ public class AtlasMetricObserver implements MetricObserver {
 		return totalMetricsInBatch;
 	}
 
-	protected static List<Metric> addTypeTagsAsNecessary(List<Metric> metrics) {
-		List<Metric> typedMetrics = new ArrayList<>();
+	static List<Metric> sanitizeTags(List<Metric> metrics) {
+		List<Metric> sanitized = new ArrayList<>(metrics.size());
+		for (Metric m : metrics) {
+			MonitorConfig.Builder config = MonitorConfig.builder(toValidCharset(m.getConfig().getName()));
+			for (Tag tag : m.getConfig().getTags()) {
+				config.withTag(toValidCharset(tag.getKey()), toValidCharset(tag.getValue()));
+			}
+			config.withPublishingPolicy(m.getConfig().getPublishingPolicy());
+			sanitized.add(new Metric(config.build(), m.getTimestamp(), m.getValue()));
+		}
+		return sanitized;
+	}
+	
+	private static String toValidCharset(String name) {
+		return name.replaceAll("[^\\.\\-\\w]", "_");
+	}
+
+	static List<Metric> addTypeTagsAsNecessary(List<Metric> metrics) {
+		List<Metric> typedMetrics = new ArrayList<>(metrics.size());
 		for (Metric m : metrics) {
 			String value = m.getConfig().getTags().getValue(DataSourceType.KEY);
 			Metric transformed;
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserverTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserverTests.java
index 5c1a4464..13542fbc 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserverTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/atlas/AtlasMetricObserverTests.java
@@ -17,6 +17,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
+import com.netflix.servo.tag.Tag;
 import org.junit.Test;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
@@ -191,6 +192,23 @@ public class AtlasMetricObserverTests {
 		assertThat(obs.sendMetricsBatch(generateMetrics(2)), 
 				is(equalTo(AtlasMetricObserver.PublishMetricsBatchStatus.PartialSuccess)));
 	}
+	
+	@Test
+	public void sanitizeMetrics() {
+		String mixtureOfValidAndInvalidChars = "a_1.2-Z/ A";
+		
+		Metric m = new Metric(new MonitorConfig.Builder(mixtureOfValidAndInvalidChars)
+				.withTag(mixtureOfValidAndInvalidChars, mixtureOfValidAndInvalidChars).build(), 0, 1);
+		
+		Metric sanitizedMetric = AtlasMetricObserver.sanitizeTags(Collections.singletonList(m)).get(0);
+
+		String valid = "a_1.2-Z__A";
+		assertThat(sanitizedMetric.getConfig().getName(), is(equalTo(valid)));
+		
+		Tag tag = sanitizedMetric.getConfig().getTags().iterator().next();
+		assertThat(tag.getKey(), is(equalTo(valid)));
+		assertThat(tag.getValue(), is(equalTo(valid)));
+	}
 
 	private List<Metric> generateMetrics(int numberOfMetrics) {
 		List<Metric> metrics = new ArrayList<>();