Merge branch 'DataDog:master' into simle/add-otel-support

DataDog · Jul 25, 2024 · 58d8d0d · 58d8d0d
2 parents 61914eb + 1139efe
commit 58d8d0d
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 57 deletions.
diff --git a/statsd/container_linux.go b/statsd/container_linux.go
@@ -35,9 +35,9 @@ const (
 
 	// ContainerRegexpStr defines the regexp used to match container IDs
 	// ([0-9a-f]{64}) is standard container id used pretty much everywhere
-	// ([0-9a-f]{32}-[0-9]{10}) is container id used by AWS ECS
+	// ([0-9a-f]{32}-\d+) is container id used by AWS ECS
 	// ([0-9a-f]{8}(-[0-9a-f]{4}){4}$) is container id used by Garden
-	containerRegexpStr = "([0-9a-f]{64})|([0-9a-f]{32}-[0-9]{10})|([0-9a-f]{8}(-[0-9a-f]{4}){4}$)"
+	containerRegexpStr = "([0-9a-f]{64})|([0-9a-f]{32}-\\d+)|([0-9a-f]{8}(-[0-9a-f]{4}){4}$)"
 	// cIDRegexpStr defines the regexp used to match container IDs in /proc/self/mountinfo
 	cIDRegexpStr = `.*/([^\s/]+)/(` + containerRegexpStr + `)/[\S]*hostname`
 
@@ -184,23 +184,36 @@ func inodeForPath(path string) string {
 
 // internalInitContainerID initializes the container ID.
 // It can either be provided by the user or read from cgroups.
-func internalInitContainerID(userProvidedID string, cgroupFallback bool) {
+func internalInitContainerID(userProvidedID string, cgroupFallback, isHostCgroupNs bool) {
 	initOnce.Do(func() {
-		if userProvidedID != "" {
-			containerID = userProvidedID
+		readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath, cgroupFallback, isHostCgroupNs)
+	})
+}
+
+// readCIDOrInode reads the container ID from the user provided ID, cgroups or mountinfo.
+func readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath string, cgroupFallback, isHostCgroupNs bool) {
+	if userProvidedID != "" {
+		containerID = userProvidedID
+		return
+	}
+
+	if cgroupFallback {
+		containerID = readContainerID(cgroupPath)
+		if containerID != "" {
+			return
+		}
+
+		containerID = readMountinfo(selfMountInfoPath)
+		if containerID != "" {
 			return
 		}
 
-		if cgroupFallback {
-			isHostCgroupNs := isHostCgroupNamespace()
-			if isHostCgroupNs {
-				containerID = readContainerID(cgroupPath)
-				return
-			}
-			containerID = readMountinfo(selfMountInfoPath)
-			if containerID != "" {
-				containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath)
-			}
+		// If we're in the host cgroup namespace, the cid should be retrievable in /proc/self/cgroup
+		// In private cgroup namespace, we can retrieve the cgroup controller inode.
+		if containerID == "" && isHostCgroupNs {
+			return
 		}
-	})
+
+		containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath)
+	}
 }
diff --git a/statsd/container_stub.go b/statsd/container_stub.go
@@ -3,7 +3,11 @@
 
 package statsd
 
-var initContainerID = func(userProvidedID string, cgroupFallback bool) {
+func isHostCgroupNamespace() bool {
+	return false
+}
+
+var initContainerID = func(userProvidedID string, _, _ bool) {
 	initOnce.Do(func() {
 		if userProvidedID != "" {
 			containerID = userProvidedID

diff --git a/statsd/container_test.go b/statsd/container_test.go
@@ -400,3 +400,90 @@ func TestGetCgroupInode(t *testing.T) {
 		})
 	}
 }
+
+func TestReadCIDOrInode(t *testing.T) {
+	tests := []struct {
+		description           string
+		procSelfCgroupContent string
+		mountInfoContent      string
+		isHostCgroupNs        bool
+		expectedResult        string
+		cgroupNodeDir         string
+	}{
+		{
+			description:           "extract container-id from /proc/self/cgroup",
+			procSelfCgroupContent: "4:blkio:/kubepods/burstable/podfd52ef25-a87d-11e9-9423-0800271a638e/8c046cb0b72cd4c99f51b5591cd5b095967f58ee003710a45280c28ee1a9c7fa\n",
+			isHostCgroupNs:        true,
+			expectedResult:        "8c046cb0b72cd4c99f51b5591cd5b095967f58ee003710a45280c28ee1a9c7fa", // Will be formatted with inode number
+		},
+		{
+			description:           "extract container-id from /proc/self/cgroup in private cgroup ns",
+			procSelfCgroupContent: "4:blkio:/kubepods/burstable/podfd52ef25-a87d-11e9-9423-0800271a638e/8c046cb0b72cd4c99f51b5591cd5b095967f58ee003710a45280c28ee1a9c7fa\n",
+			expectedResult:        "8c046cb0b72cd4c99f51b5591cd5b095967f58ee003710a45280c28ee1a9c7fa", // Will be formatted with inode number
+		},
+		{
+			description:      "extract container-id from mountinfo in private cgroup ns",
+			mountInfoContent: "2282 2269 8:1 /var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/c0a82a3506b0366c9666f6dbe71c783abeb26ba65e312e918a49e10a277196d0/hostname /host/var/run/containerd/io.containerd.runtime.v2.task/k8s.io/fc7038bc73a8d3850c66ddbfb0b2901afa378bfcbb942cc384b051767e4ac6b0/rootfs/etc/hostname rw,nosuid,nodev,relatime - ext4 /dev/sda1 rw,commit=30\n",
+			expectedResult:   "fc7038bc73a8d3850c66ddbfb0b2901afa378bfcbb942cc384b051767e4ac6b0",
+		},
+		{
+			description:      "extract container-id from mountinfo",
+			mountInfoContent: "2282 2269 8:1 /var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/c0a82a3506b0366c9666f6dbe71c783abeb26ba65e312e918a49e10a277196d0/hostname /host/var/run/containerd/io.containerd.runtime.v2.task/k8s.io/fc7038bc73a8d3850c66ddbfb0b2901afa378bfcbb942cc384b051767e4ac6b0/rootfs/etc/hostname rw,nosuid,nodev,relatime - ext4 /dev/sda1 rw,commit=30\n",
+			expectedResult:   "fc7038bc73a8d3850c66ddbfb0b2901afa378bfcbb942cc384b051767e4ac6b0",
+			isHostCgroupNs:   true,
+		},
+		{
+			description:           "extract inode only in private cgroup ns",
+			cgroupNodeDir:         "system.slice/docker-abcdef0123456789abcdef0123456789.scope",
+			procSelfCgroupContent: "0::/system.slice/docker-abcdef0123456789abcdef0123456789.scope\n",
+			expectedResult:        "in-%d",
+		},
+		{
+			description:           "do not extract inode in host cgroup ns",
+			cgroupNodeDir:         "system.slice/docker-abcdef0123456789abcdef0123456789.scope",
+			procSelfCgroupContent: "0::/system.slice/docker-abcdef0123456789abcdef0123456789.scope\n",
+			isHostCgroupNs:        true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.description, func(t *testing.T) {
+			prevContainerID := containerID
+			defer func() {
+				containerID = prevContainerID
+			}()
+
+			sysFsCgroupPath := path.Join(os.TempDir(), "sysfscgroup")
+			groupControllerPath := path.Join(sysFsCgroupPath, tc.cgroupNodeDir)
+			err := os.MkdirAll(groupControllerPath, 0755)
+			require.NoError(t, err)
+			defer os.RemoveAll(groupControllerPath)
+
+			stat, err := os.Stat(groupControllerPath)
+			require.NoError(t, err)
+			expectedResult := tc.expectedResult
+			if strings.HasPrefix(tc.expectedResult, "in-") {
+				expectedResult = fmt.Sprintf(tc.expectedResult, stat.Sys().(*syscall.Stat_t).Ino)
+			}
+
+			procSelfCgroup, err := ioutil.TempFile("", "procselfcgroup")
+			require.NoError(t, err)
+			defer os.Remove(procSelfCgroup.Name())
+			_, err = procSelfCgroup.WriteString(tc.procSelfCgroupContent)
+			require.NoError(t, err)
+			err = procSelfCgroup.Close()
+			require.NoError(t, err)
+
+			mountInfo, err := ioutil.TempFile("", "mountInfo")
+			require.NoError(t, err)
+			defer os.Remove(mountInfo.Name())
+			_, err = mountInfo.WriteString(tc.mountInfoContent)
+			require.NoError(t, err)
+			err = mountInfo.Close()
+			require.NoError(t, err)
+
+			readCIDOrInode("", procSelfCgroup.Name(), mountInfo.Name(), sysFsCgroupPath, true, tc.isHostCgroupNs)
+			require.Equal(t, expectedResult, containerID)
+		})
+	}
+}
diff --git a/statsd/end_to_end_udp_test.go b/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)
 }
 

diff --git a/statsd/options.go b/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 {

diff --git a/statsd/statsd.go b/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))
-	}
-
+	initContainerID(o.containerID, isOriginDetectionEnabled(o), isHostCgroupNamespace())
 	isUDS := writerName == writerNameUDS
 
 	if o.maxBytesPerPayload == 0 {
@@ -909,16 +902,11 @@ func (c *Client) collectMeter(interval time.Duration, f func() []metric) {
 
 // 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
 	}
 

diff --git a/statsd/statsd_test.go b/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))
 		})
 	}
 }