fix: webhook env don't duplicate value forever (#1619)

odigos-io · Oct 30, 2024 · de9178e · de9178e
1 parent 1e0c3fd
commit de9178e
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 0 deletions.
diff --git a/common/envOverwrite/overwriter.go b/common/envOverwrite/overwriter.go
@@ -110,6 +110,25 @@ func GetPatchedEnvValue(envName string, observedValue string, currentSdk *common
 			if sdkEnvValue == desiredOdigosPart {
 				// shortcut, the value is already patched
 				// both the odigos part equals to the new value, and the user part we want to keep
+				// Exception: if there is a webhook involved that inject the env value,
+				// we need to remove duplicate values, otherwise it will grow indefinitely in each iteration
+				parts := strings.Split(observedValue, envMetadata.delim)
+				specialEnvValue := "-javaagent:/opt/sre-agent/sre-agent.jar"
+				specialFound := false
+				newValues := []string{}
+				for _, part := range parts {
+					if part == specialEnvValue {
+						if specialFound {
+							continue
+						}
+						specialFound = true
+					}
+					if strings.TrimSpace(part) == "" {
+						continue
+					}
+					newValues = append(newValues, part)
+				}
+				observedValue = strings.Join(newValues, envMetadata.delim)
 				return &observedValue
 			} else {
 				// The environment variable is patched by some other odigos sdk.

diff --git a/common/envOverwrite/overwriter_test.go b/common/envOverwrite/overwriter_test.go
@@ -1,6 +1,7 @@
 package envOverwrite
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/odigos-io/odigos/common"
@@ -10,7 +11,9 @@ import (
 func TestGetPatchedEnvValue(t *testing.T) {
 	nodeOptionsNativeCommunity, _ := ValToAppend("NODE_OPTIONS", common.OtelSdkNativeCommunity)
 	nodeOptionsEbpfEnterprise, _ := ValToAppend("NODE_OPTIONS", common.OtelSdkEbpfEnterprise)
+	javaToolsNativeCommunity, _ := ValToAppend("JAVA_TOOL_OPTIONS", common.OtelSdkNativeCommunity)
 	userVal := "--max-old-space-size=4096"
+	specialEnvValueJava := "-javaagent:/opt/sre-agent/sre-agent.jar"
 
 	// test different cases
 	tests := []struct {
@@ -87,6 +90,30 @@ func TestGetPatchedEnvValue(t *testing.T) {
 			programmingLanguage:  common.UnknownProgrammingLanguage,
 			patchedValueExpected: "",
 		},
+		{
+			name:                 "multiple values in env var",
+			envName:              "JAVA_TOOL_OPTIONS",
+			observedValue:        fmt.Sprintf("%s %s %s %s", specialEnvValueJava, specialEnvValueJava, specialEnvValueJava, javaToolsNativeCommunity),
+			sdk:                  &common.OtelSdkNativeCommunity,
+			programmingLanguage:  common.JavaProgrammingLanguage,
+			patchedValueExpected: specialEnvValueJava + " " + javaToolsNativeCommunity,
+		},
+		{
+			name:                 "multiple spaces in special env value",
+			envName:              "JAVA_TOOL_OPTIONS",
+			observedValue:        fmt.Sprintf("%s %s              %s", specialEnvValueJava, specialEnvValueJava, javaToolsNativeCommunity),
+			sdk:                  &common.OtelSdkNativeCommunity,
+			programmingLanguage:  common.JavaProgrammingLanguage,
+			patchedValueExpected: specialEnvValueJava + " " + javaToolsNativeCommunity,
+		},
+		{
+			name:                 "tabs in special env value",
+			envName:              "JAVA_TOOL_OPTIONS",
+			observedValue:        fmt.Sprintf("%s \t %s \t %s", specialEnvValueJava, specialEnvValueJava, javaToolsNativeCommunity),
+			sdk:                  &common.OtelSdkNativeCommunity,
+			programmingLanguage:  common.JavaProgrammingLanguage,
+			patchedValueExpected: specialEnvValueJava + " " + javaToolsNativeCommunity,
+		},
 	}
 
 	for _, tt := range tests {