Merge pull request #300 from DataDog/ali/fix-origin-detection

[CONTINT-3864] Send both origin the container-id and the ENTITY_ID

Author: wdhif

statsd/end_to_end_udp_test.go

@@ -110,7 +110,10 @@ func TestContainerIDWithEntityID(t *testing.T) {
 	resetContainerID()
 
 	entityIDEnvName := "DD_ENTITY_ID"
-	defer func() { os.Unsetenv(entityIDEnvName) }()
+	defer func() {
+		os.Unsetenv(entityIDEnvName)
+		resetContainerID()
+	}()
 	os.Setenv(entityIDEnvName, "pod-uid")
 
 	expectedTags := []string{"dd.internal.entity_id:pod-uid"}
@@ -123,14 +126,18 @@ func TestContainerIDWithEntityID(t *testing.T) {
 
 	sort.Strings(client.tags)
 	assert.Equal(t, expectedTags, client.tags)
-	ts.assertContainerID(t, "")
+	ts.assertContainerID(t, "fake-container-id")
 	ts.sendAllAndAssert(t, client)
 }
 
 func TestContainerIDWithoutEntityID(t *testing.T) {
 	resetContainerID()
 	os.Unsetenv("DD_ENTITY_ID")
 
+	defer func() {
+		resetContainerID()
+	}()
+
 	ts, client := newClientAndTestServer(t,
 		"udp",
 		"localhost:8765",
@@ -164,7 +171,10 @@ func TestOriginDetectionEnabledWithEntityID(t *testing.T) {
 	resetContainerID()
 
 	entityIDEnvName := "DD_ENTITY_ID"
-	defer func() { os.Unsetenv(entityIDEnvName) }()
+	defer func() {
+		os.Unsetenv(entityIDEnvName)
+		resetContainerID()
+	}()
 	os.Setenv(entityIDEnvName, "pod-uid")
 
 	originDetectionEnvName := "DD_ORIGIN_DETECTION_ENABLED"
@@ -181,7 +191,7 @@ func TestOriginDetectionEnabledWithEntityID(t *testing.T) {
 
 	sort.Strings(client.tags)
 	assert.Equal(t, expectedTags, client.tags)
-	ts.assertContainerID(t, "")
+	ts.assertContainerID(t, "fake-container-id")
 	ts.sendAllAndAssert(t, client)
 }
 

statsd/options.go

@@ -373,9 +373,10 @@ func WithTelemetryAddr(addr string) Option {
 // WithoutOriginDetection disables the client origin detection.
 // When enabled, the client tries to discover its container ID and sends it to the Agent
 // to enrich the metrics with container tags.
+// If the container id is not found and the client is running in a private cgroup namespace, the client
+// sends the base cgroup controller inode.
 // Origin detection can also be disabled by configuring the environment variabe DD_ORIGIN_DETECTION_ENABLED=false
 // The client tries to read the container ID by parsing the file /proc/self/cgroup, this is not supported on Windows.
-// The client prioritizes the value passed via DD_ENTITY_ID (if set) over the container ID.
 //
 // More on this here: https://docs.datadoghq.com/developers/dogstatsd/?tab=kubernetes#origin-detection-over-udp
 func WithoutOriginDetection() Option {
@@ -389,9 +390,9 @@ func WithoutOriginDetection() Option {
 // This feature requires Datadog Agent version >=6.35.0 && <7.0.0 or Agent versions >=7.35.0.
 // When enabled, the client tries to discover its container ID and sends it to the Agent
 // to enrich the metrics with container tags.
-// Origin detection can be disabled by configuring the environment variabe DD_ORIGIN_DETECTION_ENABLED=false
-// The client tries to read the container ID by parsing the file /proc/self/cgroup, this is not supported on Windows.
-// The client prioritizes the value passed via DD_ENTITY_ID (if set) over the container ID.
+// If the container id is not found and the client is running in a private cgroup namespace, the client
+// sends the base cgroup controller inode.
+// Origin detection can be disabled by configuring the environment variable DD_ORIGIN_DETECTION_ENABLED=false
 //
 // More on this here: https://docs.datadoghq.com/developers/dogstatsd/?tab=kubernetes#origin-detection-over-udp
 func WithOriginDetection() Option {

statsd/statsd.go

@@ -454,21 +454,14 @@ func newWithWriter(w Transport, o *Options, writerName string) (*Client, error)
 		errorHandler:          o.errorHandler,
 	}
 
-	hasEntityID := false
 	// Inject values of DD_* environment variables as global tags.
 	for _, mapping := range ddEnvTagsMapping {
 		if value := os.Getenv(mapping.envName); value != "" {
-			if mapping.envName == ddEntityID {
-				hasEntityID = true
-			}
 			c.tags = append(c.tags, fmt.Sprintf("%s:%s", mapping.tagName, value))
 		}
 	}
 
-	if !hasEntityID {
-		initContainerID(o.containerID, isOriginDetectionEnabled(o, hasEntityID), isHostCgroupNamespace())
-	}
-
+	initContainerID(o.containerID, isOriginDetectionEnabled(o), isHostCgroupNamespace())
 	isUDS := writerName == writerNameUDS
 
 	if o.maxBytesPerPayload == 0 {
@@ -888,16 +881,11 @@ func (c *Client) Close() error {
 
 // isOriginDetectionEnabled returns whether the clients should fill the container field.
 //
-// If DD_ENTITY_ID is set, we don't send the container ID
-// If a user-defined container ID is provided, we don't ignore origin detection
-// as dd.internal.entity_id is prioritized over the container field for backward compatibility.
-// If DD_ENTITY_ID is not set, we try to fill the container field automatically unless
-// DD_ORIGIN_DETECTION_ENABLED is explicitly set to false.
-func isOriginDetectionEnabled(o *Options, hasEntityID bool) bool {
-	if !o.originDetection || hasEntityID || o.containerID != "" {
-		// originDetection is explicitly disabled
-		// or DD_ENTITY_ID was found
-		// or a user-defined container ID was provided
+// Disable origin detection only in one of the following cases:
+// - DD_ORIGIN_DETECTION_ENABLED is explicitly set to false
+// - o.originDetection is explicitly set to false, which is true by default
+func isOriginDetectionEnabled(o *Options) bool {
+	if !o.originDetection || o.containerID != "" {
 		return false
 	}
 

statsd/statsd_test.go

@@ -311,49 +311,36 @@ func Test_isOriginDetectionEnabled(t *testing.T) {
 	tests := []struct {
 		name              string
 		o                 *Options
-		hasEntityID       bool
 		configEnvVarValue string
 		want              bool
 	}{
 		{
 			name:              "nominal case",
 			o:                 &Options{originDetection: defaultOriginDetection},
-			hasEntityID:       false,
 			configEnvVarValue: "",
 			want:              true,
 		},
-		{
-			name:              "has entity ID",
-			o:                 &Options{originDetection: defaultOriginDetection},
-			hasEntityID:       true,
-			configEnvVarValue: "",
-			want:              false,
-		},
 		{
 			name:              "has user-provided container ID",
 			o:                 &Options{containerID: "user-provided"},
-			hasEntityID:       true,
 			configEnvVarValue: "",
 			want:              false,
 		},
 		{
 			name:              "originDetection option disabled",
 			o:                 &Options{originDetection: false},
-			hasEntityID:       false,
 			configEnvVarValue: "",
 			want:              false,
 		},
 		{
 			name:              "DD_ORIGIN_DETECTION_ENABLED=false",
 			o:                 &Options{originDetection: defaultOriginDetection},
-			hasEntityID:       false,
 			configEnvVarValue: "false",
 			want:              false,
 		},
 		{
 			name:              "invalid DD_ORIGIN_DETECTION_ENABLED value",
 			o:                 &Options{originDetection: defaultOriginDetection},
-			hasEntityID:       false,
 			configEnvVarValue: "invalid",
 			want:              true,
 		},
@@ -363,7 +350,7 @@ func Test_isOriginDetectionEnabled(t *testing.T) {
 			os.Setenv("DD_ORIGIN_DETECTION_ENABLED", tt.configEnvVarValue)
 			defer os.Unsetenv("DD_ORIGIN_DETECTION_ENABLED")
 
-			assert.Equal(t, tt.want, isOriginDetectionEnabled(tt.o, tt.hasEntityID))
+			assert.Equal(t, tt.want, isOriginDetectionEnabled(tt.o))
 		})
 	}
 }