diff --git a/spectator-nflx-tagging/src/main/java/com/netflix/spectator/nflx/tagging/NetflixTagging.java b/spectator-nflx-tagging/src/main/java/com/netflix/spectator/nflx/tagging/NetflixTagging.java index 94a6289aa..73fd107c3 100644 --- a/spectator-nflx-tagging/src/main/java/com/netflix/spectator/nflx/tagging/NetflixTagging.java +++ b/spectator-nflx-tagging/src/main/java/com/netflix/spectator/nflx/tagging/NetflixTagging.java @@ -110,7 +110,9 @@ public static Map commonTags(Function 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", diff --git a/spectator-nflx-tagging/src/test/java/com/netflix/spectator/nflx/tagging/NetflixTaggingTest.java b/spectator-nflx-tagging/src/test/java/com/netflix/spectator/nflx/tagging/NetflixTaggingTest.java index 14daa13b1..ed528ec1a 100644 --- a/spectator-nflx-tagging/src/test/java/com/netflix/spectator/nflx/tagging/NetflixTaggingTest.java +++ b/spectator-nflx-tagging/src/test/java/com/netflix/spectator/nflx/tagging/NetflixTaggingTest.java @@ -37,7 +37,7 @@ private static Map 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 actual = NetflixTagging.commonTags(vars::get); Assertions.assertEquals(expected, actual); } + + @Test + public void commonTagsFallbackToTitusContainerName() { + Map vars = sampleEnvironmentVars(); + vars.put("TITUS_CONTAINER_NAME", vars.get("NETFLIX_CONTAINER_NAME")); + vars.remove("NETFLIX_CONTAINER_NAME"); + Map expected = sampleExpectedTags(); + Map actual = NetflixTagging.commonTags(vars::get); + Assertions.assertEquals(expected, actual); + } + + @Test + public void commonTagsNetflixContainerNameOverridesTitus() { + Map vars = sampleEnvironmentVars(); + vars.put("TITUS_CONTAINER_NAME", "titus-main"); + Map expected = sampleExpectedTags(); + Map actual = NetflixTagging.commonTags(vars::get); + Assertions.assertEquals(expected, actual); + } + + @Test + public void commonTagsEmptyNetflixContainerNameFallsBackToTitus() { + Map vars = sampleEnvironmentVars(); + vars.put("NETFLIX_CONTAINER_NAME", ""); + vars.put("TITUS_CONTAINER_NAME", "main"); + Map expected = sampleExpectedTags(); + Map actual = NetflixTagging.commonTags(vars::get); + Assertions.assertEquals(expected, actual); + } + + @Test + public void commonTagsBlankNetflixContainerNameFallsBackToTitus() { + Map vars = sampleEnvironmentVars(); + vars.put("NETFLIX_CONTAINER_NAME", " \t"); + vars.put("TITUS_CONTAINER_NAME", "main"); + Map expected = sampleExpectedTags(); + Map actual = NetflixTagging.commonTags(vars::get); + Assertions.assertEquals(expected, actual); + } + + @Test + public void commonTagsNoContainerName() { + Map vars = sampleEnvironmentVars(); + vars.remove("NETFLIX_CONTAINER_NAME"); + Map expected = sampleExpectedTags(); + expected.remove("nf.container"); + Map actual = NetflixTagging.commonTags(vars::get); + Assertions.assertEquals(expected, actual); + } + + @Test + public void commonTagsEmptyProcessNameIgnored() { + Map vars = sampleEnvironmentVars(); + vars.put("NETFLIX_PROCESS_NAME", ""); + Map expected = sampleExpectedTags(); + expected.remove("nf.process"); + Map actual = NetflixTagging.commonTags(vars::get); + Assertions.assertEquals(expected, actual); + } } diff --git a/spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/CommonTags.java b/spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/CommonTags.java index 6f17711cb..6af0b51e3 100644 --- a/spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/CommonTags.java +++ b/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. + * + *
    + *
  • nf.container: from NETFLIX_CONTAINER_NAME, which the + * platform sets to the Kubernetes container name, falling back to + * TITUS_CONTAINER_NAME.
    • + *
  • nf.process: from NETFLIX_PROCESS_NAME, which the user sets + * for each process in the container entrypoint.
    • + *
+ * + *

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 commonTags(Function getenv) { Map 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; } diff --git a/spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/SidecarConfig.java b/spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/SidecarConfig.java index 4c24a1247..0be685879 100644 --- a/spectator-reg-sidecar/src/main/java/com/netflix/spectator/sidecar/SidecarConfig.java +++ b/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 + * nf.container from NETFLIX_CONTAINER_NAME (falling back to + * TITUS_CONTAINER_NAME) and nf.process from + * NETFLIX_PROCESS_NAME; 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 SidecarConfig.super.commonTags() to keep them. */ default Map commonTags() { return CommonTags.commonTags(System::getenv); diff --git a/spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarConfigTest.java b/spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarConfigTest.java index 14618cfe6..1f516fc1e 100644 --- a/spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarConfigTest.java +++ b/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 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 getenv = s -> "NETFLIX_CONTAINER_NAME".equals(s) ? " test " : null; + Assertions.assertEquals( + Collections.singletonMap("nf.container", "test"), + CommonTags.commonTags(getenv)); + } + + @Test + public void commonTagsContainerNetflixWinsOverTitus() { + Map 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 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 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 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 vars = new HashMap<>(); + vars.put("NETFLIX_CONTAINER_NAME", "app"); + vars.put("NETFLIX_PROCESS_NAME", "processA"); + Map expected = new HashMap<>(); + expected.put("nf.container", "app"); + expected.put("nf.process", "processA"); + Assertions.assertEquals(expected, CommonTags.commonTags(vars::get)); + } } diff --git a/spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarRegistryTest.java b/spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarRegistryTest.java index 7191d719f..f545b07eb 100644 --- a/spectator-reg-sidecar/src/test/java/com/netflix/spectator/sidecar/SidecarRegistryTest.java +++ b/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 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 commonTags() { + return Collections.emptyMap(); + } + MemoryWriter writer() { return writer; }