Merge pull request #287 from brharrington/aws-cleanup

fix service{Name,Endpoint} for aws collector

spectator-ext-aws/build.gradle

@@ -1,4 +1,5 @@
 dependencies {
   compile project(":spectator-api")
   compile "com.amazonaws:aws-java-sdk-core"
+  testCompile "com.amazonaws:aws-java-sdk-cloudwatch"
 }

spectator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java

@@ -61,8 +61,8 @@ public class SpectatorRequestMetricCollector extends RequestMetricCollector {
   };
 
   private static final TagField[] TAGS = {
-      new TagField(Field.ServiceEndpoint),
-      new TagField(Field.ServiceName, SpectatorRequestMetricCollector::getHost),
+      new TagField(Field.ServiceEndpoint, SpectatorRequestMetricCollector::getHost),
+      new TagField(Field.ServiceName),
       new TagField(Field.StatusCode)
   };
 

spectator-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java

@@ -19,6 +19,7 @@
 import com.amazonaws.Request;
 import com.amazonaws.Response;
 import com.amazonaws.http.HttpResponse;
+import com.amazonaws.services.cloudwatch.model.ListMetricsRequest;
 import com.amazonaws.util.AWSRequestMetrics;
 import com.amazonaws.util.AWSRequestMetricsFullSupport;
 import com.amazonaws.util.TimingInfo;
@@ -29,9 +30,13 @@
 
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import static org.junit.Assert.*;
 
@@ -46,46 +51,96 @@ public void setUp() {
     collector = new SpectatorRequestMetricCollector(registry);
   }
 
-  @Test
-  public void testMetricCollection() {
-    //setup
+  private void execRequest(String endpoint, int status) {
     AWSRequestMetrics metrics = new AWSRequestMetricsFullSupport();
+    metrics.addProperty(AWSRequestMetrics.Field.ServiceName, "AmazonCloudWatch");
+    metrics.addProperty(AWSRequestMetrics.Field.ServiceEndpoint, endpoint);
+    metrics.addProperty(AWSRequestMetrics.Field.StatusCode, "" + status);
+    if (status == 503) {
+      metrics.addProperty(AWSRequestMetrics.Field.AWSErrorCode, "Throttled");
+    }
     String counterName = "BytesProcessed";
     String timerName = "ClientExecuteTime";
     metrics.setCounter(counterName, 12345);
     metrics.getTimingInfo().addSubMeasurement(timerName, TimingInfo.unmodifiableTimingInfo(100000L, 200000L));
 
-    Request<?> req = new DefaultRequest("foo");
+    Request<?> req = new DefaultRequest(new ListMetricsRequest(), "AmazonCloudWatch");
     req.setAWSRequestMetrics(metrics);
-    req.setEndpoint(URI.create("http://foo"));
+    req.setEndpoint(URI.create(endpoint));
 
-    HttpResponse hr = new HttpResponse(req, new HttpPost("http://foo"));
-    hr.setStatusCode(200);
-    Response<?> resp = new Response<>(null, new HttpResponse(req, new HttpPost("http://foo")));
+    HttpResponse hr = new HttpResponse(req, new HttpPost(endpoint));
+    hr.setStatusCode(status);
+    Response<?> resp = new Response<>(null, new HttpResponse(req, new HttpPost(endpoint)));
 
-    //when
     collector.collectMetrics(req, resp);
+  }
+
+  /** Returns a set of all values for a given tag key. */
+  private Set<String> valueSet(String k) {
+    return registry.stream()
+        .map(m -> Utils.getTagValue(m.id(), k))
+        .filter(v -> v != null)
+        .collect(Collectors.toSet());
+  }
+
+  private Set<String> set(String... vs) {
+    return new HashSet<>(Arrays.asList(vs));
+  }
+
+  @Test
+  public void extractServiceEndpoint() {
+    execRequest("http://monitoring", 200);
+    assertEquals(set("monitoring"), valueSet("serviceEndpoint"));
+
+    execRequest("http://s3", 200);
+    assertEquals(set("monitoring", "s3"), valueSet("serviceEndpoint"));
+  }
+
+  @Test
+  public void extractServiceName() {
+    execRequest("http://monitoring", 200);
+    assertEquals(set("AmazonCloudWatch"), valueSet("serviceName"));
+  }
+
+  @Test
+  public void extractRequestType() {
+    execRequest("http://monitoring", 200);
+    assertEquals(set("ListMetricsRequest"), valueSet("requestType"));
+  }
+
+  @Test
+  public void extractStatusCode() {
+    execRequest("http://monitoring", 200);
+    execRequest("http://monitoring", 429);
+    execRequest("http://monitoring", 503);
+    assertEquals(set("200", "429", "503"), valueSet("statusCode"));
+  }
+
+  @Test
+  public void extractErrorCode() {
+    execRequest("http://monitoring", 200);
+    assertEquals(set(), valueSet("AWSErrorCode"));
+
+    execRequest("http://monitoring", 503);
+    assertEquals(set("Throttled"), valueSet("AWSErrorCode"));
+  }
+
+  @Test
+  public void testMetricCollection() {
+    execRequest("http://foo", 200);
 
     //then
     List<Meter> allMetrics = new ArrayList<>();
     registry.iterator().forEachRemaining(allMetrics::add);
 
     assertEquals(2, allMetrics.size());
-    Optional<Timer> expectedTimer = allMetrics
-        .stream()
-        .filter(m -> m instanceof Timer && m.id().name().equals(SpectatorRequestMetricCollector.idName(timerName)))
-        .map(m -> (Timer) m)
-        .findFirst();
+    Optional<Timer> expectedTimer = registry.timers().findFirst();
     assertTrue(expectedTimer.isPresent());
     Timer timer = expectedTimer.get();
     assertEquals(1, timer.count());
     assertEquals(100000, timer.totalTime());
 
-    Optional<Counter> expectedCounter = allMetrics
-        .stream()
-        .filter(m -> m.id().name().equals(SpectatorRequestMetricCollector.idName(counterName)))
-        .map(m -> (Counter) m)
-        .findFirst();
+    Optional<Counter> expectedCounter = registry.counters().findFirst();
     assertTrue(expectedCounter.isPresent());
     assertEquals(12345L, expectedCounter.get().count());
   }