Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
May 7, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
74abb6b
Send both origin the container-id and the ENTITY_ID
AliDatadog Jan 8, 2024
221f7a3
fix unit tests
AliDatadog Jan 11, 2024
0ef253f
fix unit tests using global state
AliDatadog Jan 11, 2024
8c18762
update comments
AliDatadog Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 14 additions & 4 deletions statsd/end_to_end_udp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand All @@ -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",
Expand Down Expand Up @@ -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"
Expand All @@ -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)
}

Expand Down
9 changes: 5 additions & 4 deletions statsd/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
24 changes: 6 additions & 18 deletions statsd/statsd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would have left the rest of the comment but that's not a blocker.

// 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
}

Expand Down
15 changes: 1 addition & 14 deletions statsd/statsd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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))
})
}
}
Expand Down