Read nf.container from NETFLIX_CONTAINER_NAME

spectator-nflx-tagging/src/main/java/com/netflix/spectator/nflx/tagging/NetflixTagging.java

@@ -110,7 +110,9 @@ public static Map<String, String> commonTags(Function<String, String> getenv) {
     putIfNotEmptyOrNull(getenv, tags, "nf.app", "NETFLIX_APP");
     putIfNotEmptyOrNull(getenv, tags, "nf.asg", "NETFLIX_AUTO_SCALE_GROUP");
     putIfNotEmptyOrNull(getenv, tags, "nf.cluster", "NETFLIX_CLUSTER");
-    putIfNotEmptyOrNull(getenv, tags, "nf.container", "TITUS_CONTAINER_NAME");
+    putIfNotEmptyOrNull(getenv, tags, "nf.container",
+        "NETFLIX_CONTAINER_NAME",
+        "TITUS_CONTAINER_NAME");
     putIfNotEmptyOrNull(getenv, tags, "nf.node",
         "NETFLIX_INSTANCE_ID",
         "TITUS_TASK_INSTANCE_ID",

spectator-nflx-tagging/src/test/java/com/netflix/spectator/nflx/tagging/NetflixTaggingTest.java

@@ -37,7 +37,7 @@ private static Map<String, String> sampleEnvironmentVars() {
     vars.put("NETFLIX_STACK", "bar");
     vars.put("EC2_INSTANCE_TYPE", "m5.large");
     vars.put("EC2_AVAILABILITY_ZONE", "us-east-1e");
-    vars.put("TITUS_CONTAINER_NAME", "main");
+    vars.put("NETFLIX_CONTAINER_NAME", "main");
 
     vars.put("NETFLIX_ACCOUNT_TYPE", "example");
     vars.put("NETFLIX_BUILD_JOB", "http://build/foo/42");
@@ -225,4 +225,63 @@ public void commonTagsTitusOverridesEc2InstanceId() {
     Map<String, String> actual = NetflixTagging.commonTags(vars::get);
     Assertions.assertEquals(expected, actual);
   }
+
+  @Test
+  public void commonTagsFallbackToTitusContainerName() {
+    Map<String, String> vars = sampleEnvironmentVars();
+    vars.put("TITUS_CONTAINER_NAME", vars.get("NETFLIX_CONTAINER_NAME"));
+    vars.remove("NETFLIX_CONTAINER_NAME");
+    Map<String, String> expected = sampleExpectedTags();
+    Map<String, String> actual = NetflixTagging.commonTags(vars::get);
+    Assertions.assertEquals(expected, actual);
+  }
+
+  @Test
+  public void commonTagsNetflixContainerNameOverridesTitus() {
+    Map<String, String> vars = sampleEnvironmentVars();
+    vars.put("TITUS_CONTAINER_NAME", "titus-main");
+    Map<String, String> expected = sampleExpectedTags();
+    Map<String, String> actual = NetflixTagging.commonTags(vars::get);
+    Assertions.assertEquals(expected, actual);
+  }
+
+  @Test
+  public void commonTagsEmptyNetflixContainerNameFallsBackToTitus() {
+    Map<String, String> vars = sampleEnvironmentVars();
+    vars.put("NETFLIX_CONTAINER_NAME", "");
+    vars.put("TITUS_CONTAINER_NAME", "main");
+    Map<String, String> expected = sampleExpectedTags();
+    Map<String, String> actual = NetflixTagging.commonTags(vars::get);
+    Assertions.assertEquals(expected, actual);
+  }
+
+  @Test
+  public void commonTagsBlankNetflixContainerNameFallsBackToTitus() {
+    Map<String, String> vars = sampleEnvironmentVars();
+    vars.put("NETFLIX_CONTAINER_NAME", " \t");
+    vars.put("TITUS_CONTAINER_NAME", "main");
+    Map<String, String> expected = sampleExpectedTags();
+    Map<String, String> actual = NetflixTagging.commonTags(vars::get);
+    Assertions.assertEquals(expected, actual);
+  }
+
+  @Test
+  public void commonTagsNoContainerName() {
+    Map<String, String> vars = sampleEnvironmentVars();
+    vars.remove("NETFLIX_CONTAINER_NAME");
+    Map<String, String> expected = sampleExpectedTags();
+    expected.remove("nf.container");
+    Map<String, String> actual = NetflixTagging.commonTags(vars::get);
+    Assertions.assertEquals(expected, actual);
+  }
+
+  @Test
+  public void commonTagsEmptyProcessNameIgnored() {
+    Map<String, String> vars = sampleEnvironmentVars();
+    vars.put("NETFLIX_PROCESS_NAME", "");
+    Map<String, String> expected = sampleExpectedTags();
+    expected.remove("nf.process");
+    Map<String, String> actual = NetflixTagging.commonTags(vars::get);
+    Assertions.assertEquals(expected, actual);
+  }
 }

spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/CommonTags.java

@@ -20,8 +20,19 @@
 import java.util.function.Function;
 
 /**
- * Helper for accessing common tags that are specific to a process and thus cannot be
- * managed by a shared spectatord instance.
+ * Helper for accessing common tags that are specific to a container or process and thus
+ * cannot be managed by a shared spectatord instance, which only adds pod- or host-level tags.
+ *
+ * <ul>
+ *   <li><code>nf.container</code>: from <code>NETFLIX_CONTAINER_NAME</code>, which the
+ *       platform sets to the Kubernetes container name, falling back to
+ *       <code>TITUS_CONTAINER_NAME</code>.</li>
+ *   <li><code>nf.process</code>: from <code>NETFLIX_PROCESS_NAME</code>, which the user sets
+ *       for each process in the container entrypoint.</li>
+ * </ul>
+ *
+ * <p>Empty or whitespace-only values are skipped. If none of the variables for a tag has a
+ * value, the tag is not added.
  */
 final class CommonTags {
 
@@ -38,7 +49,9 @@ private CommonTags() {
    */
   static Map<String, String> commonTags(Function<String, String> getenv) {
     Map<String, String> tags = new HashMap<>();
-    putIfNotEmptyOrNull(getenv, tags, "nf.container", "TITUS_CONTAINER_NAME");
+    putIfNotEmptyOrNull(getenv, tags, "nf.container",
+        "NETFLIX_CONTAINER_NAME",
+        "TITUS_CONTAINER_NAME");
     putIfNotEmptyOrNull(getenv, tags, "nf.process", "NETFLIX_PROCESS_NAME");
     return tags;
   }

spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/SidecarConfig.java

@@ -43,7 +43,12 @@ default String outputLocation() {
   }
 
   /**
-   * Returns the common tags to apply to all metrics.
+   * Returns the common tags to apply to all metrics. By default these are
+   * <code>nf.container</code> from <code>NETFLIX_CONTAINER_NAME</code> (falling back to
+   * <code>TITUS_CONTAINER_NAME</code>) and <code>nf.process</code> from
+   * <code>NETFLIX_PROCESS_NAME</code>; blank values are skipped. They are merged last onto every
+   * meter id, so they override meter tags with the same key. Implementations that override this
+   * method should start from <code>SidecarConfig.super.commonTags()</code> to keep them.
    */
   default Map<String, String> commonTags() {
     return CommonTags.commonTags(System::getenv);

spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarConfigTest.java

@@ -16,9 +16,12 @@
 package com.netflix.spectator.sidecar;
 
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
 
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.function.Function;
 
 
@@ -38,10 +41,22 @@ public void outputLocationSet() {
 
   @Test
   public void commonTagsEmpty() {
+    Assumptions.assumeTrue(
+        System.getenv("NETFLIX_CONTAINER_NAME") == null
+            && System.getenv("TITUS_CONTAINER_NAME") == null
+            && System.getenv("NETFLIX_PROCESS_NAME") == null,
+        "container/process env vars are set on this machine");
     SidecarConfig config = s -> null;
     Assertions.assertEquals(Collections.emptyMap(), config.commonTags());
   }
 
+  @Test
+  public void commonTagsDefaultReadsEnvironment() {
+    // Checks the default wiring on any host, including ones where the variables are set.
+    SidecarConfig config = s -> null;
+    Assertions.assertEquals(CommonTags.commonTags(System::getenv), config.commonTags());
+  }
+
   @Test
   public void commonTagsProcess() {
     Function<String, String> getenv = s -> "NETFLIX_PROCESS_NAME".equals(s) ? "test" : null;
@@ -57,4 +72,64 @@ public void commonTagsContainer() {
         Collections.singletonMap("nf.container", "test"),
         CommonTags.commonTags(getenv));
   }
+
+  @Test
+  public void commonTagsContainerNetflix() {
+    Function<String, String> getenv = s -> "NETFLIX_CONTAINER_NAME".equals(s) ? " test " : null;
+    Assertions.assertEquals(
+        Collections.singletonMap("nf.container", "test"),
+        CommonTags.commonTags(getenv));
+  }
+
+  @Test
+  public void commonTagsContainerNetflixWinsOverTitus() {
+    Map<String, String> vars = new HashMap<>();
+    vars.put("NETFLIX_CONTAINER_NAME", "app");
+    vars.put("TITUS_CONTAINER_NAME", "main");
+    Assertions.assertEquals(
+        Collections.singletonMap("nf.container", "app"),
+        CommonTags.commonTags(vars::get));
+  }
+
+  @Test
+  public void commonTagsContainerEmptyNetflixFallsBackToTitus() {
+    Map<String, String> vars = new HashMap<>();
+    vars.put("NETFLIX_CONTAINER_NAME", "");
+    vars.put("TITUS_CONTAINER_NAME", "main");
+    Assertions.assertEquals(
+        Collections.singletonMap("nf.container", "main"),
+        CommonTags.commonTags(vars::get));
+  }
+
+  @Test
+  public void commonTagsContainerBlankNetflixFallsBackToTitus() {
+    Map<String, String> vars = new HashMap<>();
+    vars.put("NETFLIX_CONTAINER_NAME", " \t");
+    vars.put("TITUS_CONTAINER_NAME", "main");
+    Assertions.assertEquals(
+        Collections.singletonMap("nf.container", "main"),
+        CommonTags.commonTags(vars::get));
+  }
+
+  @Test
+  public void commonTagsProcessEmptyIgnored() {
+    Function<String, String> getenv = s -> "NETFLIX_PROCESS_NAME".equals(s) ? "" : null;
+    Assertions.assertEquals(Collections.emptyMap(), CommonTags.commonTags(getenv));
+  }
+
+  @Test
+  public void commonTagsNoneSet() {
+    Assertions.assertEquals(Collections.emptyMap(), CommonTags.commonTags(s -> null));
+  }
+
+  @Test
+  public void commonTagsContainerAndProcess() {
+    Map<String, String> vars = new HashMap<>();
+    vars.put("NETFLIX_CONTAINER_NAME", "app");
+    vars.put("NETFLIX_PROCESS_NAME", "processA");
+    Map<String, String> expected = new HashMap<>();
+    expected.put("nf.container", "app");
+    expected.put("nf.process", "processA");
+    Assertions.assertEquals(expected, CommonTags.commonTags(vars::get));
+  }
 }

spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarRegistryTest.java

@@ -25,7 +25,9 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 
@@ -209,6 +211,26 @@ public void iterator() {
     Assertions.assertFalse(registry.iterator().hasNext());
   }
 
+  @Test
+  public void commonTagsOverrideMeterTags() {
+    MemoryWriter w = new MemoryWriter();
+    SidecarConfig cfg = new SidecarConfig() {
+      @Override public String get(String k) {
+        return null;
+      }
+
+      @Override public Map<String, String> commonTags() {
+        return Collections.singletonMap("nf.container", "main");
+      }
+    };
+    try (SidecarRegistry r = new SidecarRegistry(clock, cfg, w)) {
+      r.counter("test", "nf.container", "user").increment();
+      Assertions.assertEquals(
+          Collections.singletonList("c:test,nf.container=main:1"),
+          w.messages());
+    }
+  }
+
   private static class TestConfig implements SidecarConfig {
 
     private final MemoryWriter writer = new MemoryWriter();
@@ -221,6 +243,10 @@ private static class TestConfig implements SidecarConfig {
       return "none";
     }
 
+    @Override public Map<String, String> commonTags() {
+      return Collections.emptyMap();
+    }
+
     MemoryWriter writer() {
       return writer;
     }