Read nf.container from NETFLIX_CONTAINER_NAME

spectator/common_tags.go

@@ -21,11 +21,15 @@ func addNonEmpty(tags map[string]string, tag string, envVars ...string) {
 
 // tagsFromEnvVars Extract common infrastructure tags from the Netflix environment variables.
 //
-// The extracted variables are specific to a process and thus cannot be managed by a shared
-// SpectatorD instance.
+//   - nf.container: NETFLIX_CONTAINER_NAME (injected by the platform with the container name),
+//     falling back to TITUS_CONTAINER_NAME.
+//   - nf.process: NETFLIX_PROCESS_NAME, set by the user for each process.
+//
+// The extracted variables are specific to a container or process and thus cannot be managed by a
+// shared SpectatorD instance. Unset, empty and whitespace-only values are ignored.
 func tagsFromEnvVars() map[string]string {
 	tags := make(map[string]string)
-	addNonEmpty(tags, "nf.container", "TITUS_CONTAINER_NAME")
+	addNonEmpty(tags, "nf.container", "NETFLIX_CONTAINER_NAME", "TITUS_CONTAINER_NAME")
 	addNonEmpty(tags, "nf.process", "NETFLIX_PROCESS_NAME")
 	return tags
 }

spectator/common_tags_test.go

@@ -43,6 +43,10 @@ func TestTagsFromEnvVars(t *testing.T) {
 	_ = os.Setenv("NETFLIX_PROCESS_NAME", "process_name")
 	defer os.Unsetenv("TITUS_CONTAINER_NAME")
 	defer os.Unsetenv("NETFLIX_PROCESS_NAME")
+	// An injected value would otherwise beat TITUS_CONTAINER_NAME. t.Setenv restores it after the test, and
+	// Unsetenv makes it absent rather than empty.
+	t.Setenv("NETFLIX_CONTAINER_NAME", "")
+	_ = os.Unsetenv("NETFLIX_CONTAINER_NAME")
 
 	tags := tagsFromEnvVars()
 
@@ -54,3 +58,79 @@ func TestTagsFromEnvVars(t *testing.T) {
 		t.Errorf("Expected 'process_name', got '%s'", tags["nf.process"])
 	}
 }
+
+func TestAddNonEmptyFirstNonEmptyWins(t *testing.T) {
+	t.Setenv("FIRST_ENV_VAR", " \t ")
+	t.Setenv("SECOND_ENV_VAR", " second ")
+	t.Setenv("THIRD_ENV_VAR", "third")
+
+	tags := make(map[string]string)
+	addNonEmpty(tags, "test_tag", "FIRST_ENV_VAR", "SECOND_ENV_VAR", "THIRD_ENV_VAR")
+
+	if tags["test_tag"] != "second" {
+		t.Errorf("Expected 'second', got '%s'", tags["test_tag"])
+	}
+}
+
+func TestTagsFromEnvVarsPrefersNetflixContainerName(t *testing.T) {
+	t.Setenv("NETFLIX_CONTAINER_NAME", "app")
+	t.Setenv("TITUS_CONTAINER_NAME", "main")
+
+	if got := tagsFromEnvVars()["nf.container"]; got != "app" {
+		t.Errorf("Expected 'app', got '%s'", got)
+	}
+}
+
+func TestTagsFromEnvVarsFallsBackToTitusContainerName(t *testing.T) {
+	for _, value := range []string{"", " \t "} {
+		t.Setenv("NETFLIX_CONTAINER_NAME", value)
+		t.Setenv("TITUS_CONTAINER_NAME", "main")
+
+		if got := tagsFromEnvVars()["nf.container"]; got != "main" {
+			t.Errorf("NETFLIX_CONTAINER_NAME=%q: expected 'main', got '%s'", value, got)
+		}
+	}
+}
+
+func TestTagsFromEnvVarsNetflixContainerNameOnly(t *testing.T) {
+	t.Setenv("NETFLIX_CONTAINER_NAME", " app ")
+	t.Setenv("TITUS_CONTAINER_NAME", "")
+
+	if got := tagsFromEnvVars()["nf.container"]; got != "app" {
+		t.Errorf("Expected 'app', got '%s'", got)
+	}
+}
+
+func TestTagsFromEnvVarsSkipsEmptyValues(t *testing.T) {
+	t.Setenv("NETFLIX_CONTAINER_NAME", "")
+	t.Setenv("TITUS_CONTAINER_NAME", "")
+	t.Setenv("NETFLIX_PROCESS_NAME", "")
+
+	tags := tagsFromEnvVars()
+	if _, ok := tags["nf.container"]; ok {
+		t.Errorf("Expected nf.container not to be set")
+	}
+	if _, ok := tags["nf.process"]; ok {
+		t.Errorf("Expected nf.process not to be set")
+	}
+}
+
+func TestTagsFromEnvVarsNoneSet(t *testing.T) {
+	for _, envVar := range []string{"NETFLIX_CONTAINER_NAME", "TITUS_CONTAINER_NAME", "NETFLIX_PROCESS_NAME"} {
+		t.Setenv(envVar, "") // restores the host value after the test
+		_ = os.Unsetenv(envVar)
+	}
+
+	if tags := tagsFromEnvVars(); len(tags) != 0 {
+		t.Errorf("Expected no tags, got %v", tags)
+	}
+}
+
+// clearEnvTags clears the environment variables read by tagsFromEnvVars until the end of the test, so values
+// injected into the host environment do not change the expected tags.
+func clearEnvTags(t *testing.T) {
+	t.Helper()
+	t.Setenv("NETFLIX_CONTAINER_NAME", "")
+	t.Setenv("TITUS_CONTAINER_NAME", "")
+	t.Setenv("NETFLIX_PROCESS_NAME", "")
+}

spectator/config.go

@@ -19,6 +19,9 @@ type Config struct {
 
 // NewConfig creates a new configuration with the provided location, extra common tags, and logger. All fields are
 // optional. The extra common tags are added to every metric, on top of the common tags provided by spectatord.
+// The nf.container tag (from NETFLIX_CONTAINER_NAME, falling back to TITUS_CONTAINER_NAME) and the nf.process
+// tag (from NETFLIX_PROCESS_NAME) are read from the environment and take precedence over the same keys in
+// extraCommonTags. Use NewConfigWithoutEnvTags to leave them out.
 //
 // Possible values for location are:
 //
@@ -69,13 +72,39 @@ func NewConfigWithBuffer(
 	log logger.Logger, // defaults to default logger
 	bufferSize int, // defaults to 0 (disabled)
 	flushInterval time.Duration, // defaults to 5 seconds
+) (*Config, error) {
+	return newConfig(location, extraCommonTags, log, bufferSize, flushInterval, true)
+}
+
+// NewConfigWithoutEnvTags creates a new configuration with the provided location, extra common tags, logger,
+// bufferSize, and flushInterval. It is the same as NewConfigWithBuffer, but does not add the nf.container and
+// nf.process tags from the environment. Use it in processes that report metrics on behalf of other containers or
+// processes, such as relays and agents, so their own identity does not overwrite the tags on the metrics they
+// forward.
+func NewConfigWithoutEnvTags(
+	location string, // defaults to `udp`
+	extraCommonTags map[string]string, // defaults to empty map
+	log logger.Logger, // defaults to default logger
+	bufferSize int, // defaults to 0 (disabled)
+	flushInterval time.Duration, // used when bufferSize > 0; NewConfig uses 5 seconds
+) (*Config, error) {
+	return newConfig(location, extraCommonTags, log, bufferSize, flushInterval, false)
+}
+
+func newConfig(
+	location string,
+	extraCommonTags map[string]string,
+	log logger.Logger,
+	bufferSize int,
+	flushInterval time.Duration,
+	includeEnvTags bool,
 ) (*Config, error) {
 	location, err := calculateLocation(location)
 	if err != nil {
 		return nil, err
 	}
 
-	mergedTags := calculateExtraCommonTags(extraCommonTags)
+	mergedTags := calculateExtraCommonTags(extraCommonTags, includeEnvTags)
 
 	return &Config{
 		location:        location,
@@ -94,7 +123,7 @@ func calculateLogger(log logger.Logger) logger.Logger {
 	}
 }
 
-func calculateExtraCommonTags(extraCommonTags map[string]string) map[string]string {
+func calculateExtraCommonTags(extraCommonTags map[string]string, includeEnvTags bool) map[string]string {
 	mergedTags := make(map[string]string)
 
 	for k, v := range extraCommonTags {
@@ -104,6 +133,10 @@ func calculateExtraCommonTags(extraCommonTags map[string]string) map[string]stri
 		}
 	}
 
+	if !includeEnvTags {
+		return mergedTags
+	}
+
 	// merge extra common tags with select env var tags; env var tags take precedence
 	for k, v := range tagsFromEnvVars() {
 		// env tags are validated to be non-empty

spectator/config_test.go

@@ -4,13 +4,15 @@ import (
 	"os"
 	"reflect"
 	"testing"
+	"time"
 )
 
 func TestConfigMergesCommonTagsWithEnvVariables(t *testing.T) {
 	_ = os.Setenv("TITUS_CONTAINER_NAME", "container_name")
 	_ = os.Setenv("NETFLIX_PROCESS_NAME", "process_name")
 	defer os.Unsetenv("TITUS_CONTAINER_NAME")
 	defer os.Unsetenv("NETFLIX_PROCESS_NAME")
+	t.Setenv("NETFLIX_CONTAINER_NAME", "") // an injected value would otherwise beat TITUS_CONTAINER_NAME
 
 	tags := map[string]string{
 		"nf.app":     "app",
@@ -39,12 +41,13 @@ func TestConfigMergesCommonTagsWithEnvVariables(t *testing.T) {
 	}
 }
 
-// Test passed in values wins over env variables
+// Test env variables win over passed in values
 func TestConfigMergesCommonTagsWithEnvVariablesAndPassedInValues(t *testing.T) {
 	_ = os.Setenv("TITUS_CONTAINER_NAME", "container_name_via_env")
 	_ = os.Setenv("NETFLIX_PROCESS_NAME", "process_name_by_env")
 	defer os.Unsetenv("TITUS_CONTAINER_NAME")
 	defer os.Unsetenv("NETFLIX_PROCESS_NAME")
+	t.Setenv("NETFLIX_CONTAINER_NAME", "")
 
 	tags := map[string]string{
 		"nf.app":       "app",
@@ -75,6 +78,66 @@ func TestConfigMergesCommonTagsWithEnvVariablesAndPassedInValues(t *testing.T) {
 	}
 }
 
+// Test NETFLIX_CONTAINER_NAME wins over TITUS_CONTAINER_NAME, and env variables win over passed in values
+func TestConfigNetflixContainerNameWinsOverTitusAndPassedInValues(t *testing.T) {
+	t.Setenv("NETFLIX_CONTAINER_NAME", "web")
+	t.Setenv("TITUS_CONTAINER_NAME", "main")
+	t.Setenv("NETFLIX_PROCESS_NAME", "processA")
+
+	tags := map[string]string{
+		"nf.app":       "app",
+		"nf.container": "passed_in_container",
+		"nf.process":   "passed_in_process",
+	}
+
+	config, err := NewConfig("", tags, nil)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	expectedTags := map[string]string{
+		"nf.app":       "app",
+		"nf.container": "web",
+		"nf.process":   "processA",
+	}
+
+	if !reflect.DeepEqual(expectedTags, config.extraCommonTags) {
+		t.Errorf("Expected tags %#v, got %#v", expectedTags, config.extraCommonTags)
+	}
+}
+
+// Test NewConfigWithoutEnvTags keeps the passed in values and ignores the env variables
+func TestNewConfigWithoutEnvTags(t *testing.T) {
+	t.Setenv("NETFLIX_CONTAINER_NAME", "web")
+	t.Setenv("TITUS_CONTAINER_NAME", "main")
+	t.Setenv("NETFLIX_PROCESS_NAME", "processA")
+
+	tags := map[string]string{
+		"nf.app":       "app",
+		"nf.container": "passed_in_container",
+	}
+
+	config, err := NewConfigWithoutEnvTags("memory", tags, nil, 1024, time.Second)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	// the passed in nf.container is kept, and no nf.process is added from the env variables
+	expectedTags := map[string]string{
+		"nf.app":       "app",
+		"nf.container": "passed_in_container",
+	}
+
+	if !reflect.DeepEqual(expectedTags, config.extraCommonTags) {
+		t.Errorf("Expected tags %#v, got %#v", expectedTags, config.extraCommonTags)
+	}
+
+	if config.location != "memory" || config.bufferSize != 1024 || config.flushInterval != time.Second {
+		t.Errorf("Expected location 'memory', bufferSize 1024 and flushInterval 1s, got '%s', %d and %v",
+			config.location, config.bufferSize, config.flushInterval)
+	}
+}
+
 func TestGetLocation_ConfigValue(t *testing.T) {
 	cfg, err := NewConfig("memory", nil, nil)
 	if err != nil {
@@ -130,6 +193,7 @@ func TestConfigMergesIgnoresCommonTagsWithEnvVariablesEmptyValues(t *testing.T)
 	_ = os.Setenv("NETFLIX_PROCESS_NAME", "")
 	defer os.Unsetenv("TITUS_CONTAINER_NAME")
 	defer os.Unsetenv("NETFLIX_PROCESS_NAME")
+	t.Setenv("NETFLIX_CONTAINER_NAME", "")
 
 	tags := map[string]string{
 		"nf.app":     "app",
@@ -157,6 +221,8 @@ func TestConfigMergesIgnoresCommonTagsWithEnvVariablesEmptyValues(t *testing.T)
 }
 
 func TestConfigMergesIgnoresTagsWithPassedInValuesEmptyValues(t *testing.T) {
+	clearEnvTags(t)
+
 	tags := map[string]string{
 		"nf.app":     "app",
 		"nf.account": "",