Merge pull request #177 from pstout/pds-change-rollup-behavior

Change the behavior of rollup method to use first/newest value for du…

spectator-api/src/main/java/com/netflix/spectator/api/DefaultId.java

@@ -65,17 +65,18 @@ public DefaultId(String name) {
   }
 
   /**
-   * Returns a new id with the tag list sorted by key and with no duplicate keys. If a duplicate
-   * is found the last entry in the list with a given key will be used.
+   * Returns a new id with the tag list sorted by key and with no duplicate keys.  This is equivalent to
+   * {@code rollup(Collections.<String>emptySet(), false)}.
    */
   public DefaultId normalize() {
     return rollup(Collections.<String>emptySet(), false);
   }
 
   /**
-   * Create a new id by removing tags from the list. This operation will 1) sort the list by the
-   * tag keys, 2) dedup entries if multiple have the same key, and 3) remove keys specified by
-   * the parameters.
+   * Create a new id by possibly removing tags from the list.  This operation will:<br/>
+   *     1) remove keys as specified by the parameters<br/>
+   *     2) dedup entries that have the same key, the first value associated with the key will be the one kept,<br/>
+   *     3) sort the list by the tag keys.
    *
    * @param keys
    *     Set of keys to either keep or remove.
@@ -88,7 +89,7 @@ public DefaultId normalize() {
   public DefaultId rollup(Set<String> keys, boolean keep) {
     Map<String, String> ts = new TreeMap<>();
     for (Tag t : tags) {
-      if (keys.contains(t.key()) == keep) {
+      if (keys.contains(t.key()) == keep && !ts.containsKey(t.key())) {
         ts.put(t.key(), t.value());
       }
     }

spectator-api/src/test/java/com/netflix/spectator/api/DefaultIdTest.java

@@ -86,6 +86,24 @@ public void testRollup() {
     Assert.assertEquals(dropId, id.rollup(keys, false));
   }
 
+  @Test
+  public void testRollupDeduping() {
+    Set<String> keys = new HashSet<>();
+    keys.add("k1");
+    DefaultId idWithDupes = (new DefaultId("foo")).withTag("k1", "v1").withTag("k1", "v2");
+    DefaultId expectedId = (new DefaultId("foo")).withTag("k1", "v2");
+    Assert.assertEquals(expectedId, idWithDupes.rollup(keys, true));
+  }
+
+  @Test
+  public void testRollupDedupingOfExcludedKey() {
+    Set<String> keys = new HashSet<>();
+    keys.add("k1");
+    DefaultId idWithDupes = (new DefaultId("foo")).withTag("k1", "v1").withTag("k1", "v2");
+    DefaultId expectedId = new DefaultId("foo");
+    Assert.assertEquals(expectedId, idWithDupes.rollup(keys, false));
+  }
+
   @Test
   public void testToString() {
     DefaultId id = (new DefaultId("foo")).withTag("k1", "v1").withTag("k2", "v2");