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

fix: overwrite env variables only for the detected language #1461

Merged
merged 2 commits into from
Aug 25, 2024
Merged

fix: overwrite env variables only for the detected language #1461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32f82ad
test: add test case for env different form lang
blumamir Aug 25, 2024
0625038
fix: verify language when overwriting env vars
blumamir Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions common/envOverwrite/overwriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import (
)

type envValues struct {
delim string
values map[common.OtelSdk]string
delim string
programmingLanguage common.ProgrammingLanguage
values map[common.OtelSdk]string
}

// EnvValuesMap is a map of environment variables odigos uses for various languages and goals.
Expand All @@ -20,21 +21,24 @@ type envValues struct {
// If the paths are changed in the odigos images, the values here should be updated accordingly.
var EnvValuesMap = map[string]envValues{
"NODE_OPTIONS": {
delim: " ",
delim: " ",
programmingLanguage: common.JavascriptProgrammingLanguage,
values: map[common.OtelSdk]string{
common.OtelSdkNativeCommunity: "--require /var/odigos/nodejs/autoinstrumentation.js",
common.OtelSdkEbpfEnterprise: "--require /var/odigos/nodejs-ebpf/autoinstrumentation.js",
},
},
"PYTHONPATH": {
delim: ":",
delim: ":",
programmingLanguage: common.PythonProgrammingLanguage,
values: map[common.OtelSdk]string{
common.OtelSdkNativeCommunity: "/var/odigos/python:/var/odigos/python/opentelemetry/instrumentation/auto_instrumentation",
common.OtelSdkEbpfEnterprise: "/var/odigos/python-ebpf:/var/odigos/python/opentelemetry/instrumentation/auto_instrumentation:/var/odigos/python",
},
},
"JAVA_OPTS": {
delim: " ",
delim: " ",
programmingLanguage: common.JavaProgrammingLanguage,
values: map[common.OtelSdk]string{
common.OtelSdkNativeCommunity: "-javaagent:/var/odigos/java/javaagent.jar",
common.OtelSdkEbpfEnterprise: "-javaagent:/var/odigos/java-ebpf/dtrace-injector.jar",
Expand All @@ -43,7 +47,8 @@ var EnvValuesMap = map[string]envValues{
},
},
"JAVA_TOOL_OPTIONS": {
delim: " ",
delim: " ",
programmingLanguage: common.JavaProgrammingLanguage,
values: map[common.OtelSdk]string{
common.OtelSdkNativeCommunity: "-javaagent:/var/odigos/java/javaagent.jar",
common.OtelSdkEbpfEnterprise: "-javaagent:/var/odigos/java-ebpf/dtrace-injector.jar",
Expand All @@ -58,13 +63,18 @@ var EnvValuesMap = map[string]envValues{
// the are 2 parts to the environment value: odigos part and user part.
// either one can be set or empty.
// so we have 4 cases to handle:
func GetPatchedEnvValue(envName string, observedValue string, currentSdk common.OtelSdk) *string {
func GetPatchedEnvValue(envName string, observedValue string, currentSdk common.OtelSdk, language common.ProgrammingLanguage) *string {
envMetadata, ok := EnvValuesMap[envName]
if !ok {
// Odigos does not manipulate this environment variable, so ignore it
return nil
}

if envMetadata.programmingLanguage != language {
// Odigos does not manipulate this environment variable for the given language, so ignore it
return nil
}

desiredOdigosPart, ok := envMetadata.values[currentSdk]
if !ok {
// No specific overwrite is required for this SDK
Expand Down
17 changes: 16 additions & 1 deletion common/envOverwrite/overwriter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,47 @@ func TestGetPatchedEnvValue(t *testing.T) {
envName string
observedValue string
sdk common.OtelSdk
programmingLanguage common.ProgrammingLanguage
patchedValueExpected string
}{
{
name: "un-relevant env var",
envName: "PATH",
observedValue: "/usr/local/bin:/usr/bin:/bin",
sdk: common.OtelSdkNativeCommunity,
programmingLanguage: common.JavascriptProgrammingLanguage,
patchedValueExpected: "",
},
{
name: "only user value",
envName: "NODE_OPTIONS",
observedValue: userVal,
sdk: common.OtelSdkNativeCommunity,
programmingLanguage: common.JavascriptProgrammingLanguage,
patchedValueExpected: userVal + " " + nodeOptionsNativeCommunity,
},
{
name: "only odigos value",
envName: "NODE_OPTIONS",
observedValue: nodeOptionsNativeCommunity,
sdk: common.OtelSdkNativeCommunity,
programmingLanguage: common.JavascriptProgrammingLanguage,
patchedValueExpected: "",
},
{
name: "user value with odigos value matching SDKs",
envName: "NODE_OPTIONS",
observedValue: userVal + " " + nodeOptionsNativeCommunity,
sdk: common.OtelSdkNativeCommunity,
programmingLanguage: common.JavascriptProgrammingLanguage,
patchedValueExpected: userVal + " " + nodeOptionsNativeCommunity,
},
{
name: "user value with odigos value with different SDKs",
envName: "NODE_OPTIONS",
observedValue: userVal + " " + nodeOptionsNativeCommunity,
sdk: common.OtelSdkEbpfEnterprise,
programmingLanguage: common.JavascriptProgrammingLanguage,
patchedValueExpected: userVal + " " + nodeOptionsEbpfEnterprise,
},
{
Expand All @@ -62,13 +68,22 @@ func TestGetPatchedEnvValue(t *testing.T) {
envName: "NODE_OPTIONS",
observedValue: nodeOptionsNativeCommunity,
sdk: common.OtelSdkEbpfEnterprise,
programmingLanguage: common.JavascriptProgrammingLanguage,
patchedValueExpected: "",
},
{
name: "observed env is for a different programming language than what detected",
envName: "NODE_OPTIONS",
observedValue: userVal,
sdk: common.OtelSdkNativeCommunity,
programmingLanguage: common.PythonProgrammingLanguage,
patchedValueExpected: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
patchedValue := GetPatchedEnvValue(tt.envName, tt.observedValue, tt.sdk)
patchedValue := GetPatchedEnvValue(tt.envName, tt.observedValue, tt.sdk, tt.programmingLanguage)
if patchedValue == nil {
assert.Equal(t, tt.patchedValueExpected, "", "mismatch in GetPatchedEnvValue: %s", tt.name)
} else {
Expand Down
93 changes: 66 additions & 27 deletions instrumentor/controllers/instrumentationdevice/envoverwiter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ var _ = Describe("envoverwrite", func() {
var deployment *appsv1.Deployment
var instrumentedApplication *odigosv1.InstrumentedApplication

testProgrammingLanguage := common.PythonProgrammingLanguage
testProgrammingLanguagePython := common.PythonProgrammingLanguage
deploymentSdk := common.OtelSdkNativeCommunity
testEnvVar := "PYTHONPATH"
testEnvVarPythonPath := "PYTHONPATH"
// the following is the value that odigos will append to the user's env
var testEnvOdigosValue string

Expand All @@ -50,7 +50,7 @@ var _ = Describe("envoverwrite", func() {
namespace = testutil.NewMockNamespace()
Expect(k8sClient.Create(ctx, namespace)).Should(Succeed())

sdkEnvVal, found := envOverwrite.ValToAppend(testEnvVar, deploymentSdk)
sdkEnvVal, found := envOverwrite.ValToAppend(testEnvVarPythonPath, deploymentSdk)
Expect(found).Should(BeTrue())
testEnvOdigosValue = sdkEnvVal
})
Expand All @@ -61,7 +61,7 @@ var _ = Describe("envoverwrite", func() {
var odigosConfig common.OdigosConfiguration
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: consts.DefaultOdigosNamespace, Name: consts.OdigosConfigurationName}, &cm)).Should(Succeed())
GetOdigosConfig(&cm, &odigosConfig)
odigosConfig.DefaultSDKs[testProgrammingLanguage] = common.OtelSdkNativeCommunity
odigosConfig.DefaultSDKs[testProgrammingLanguagePython] = common.OtelSdkNativeCommunity
SetOdigosConfig(&cm, &odigosConfig)
Expect(k8sClient.Update(ctx, &cm)).Should(Succeed())
})
Expand All @@ -87,7 +87,7 @@ var _ = Describe("envoverwrite", func() {
// via the instrumentation device.
// instrumented application should be updated with the odigos env
k8sClient.Get(ctx, client.ObjectKeyFromObject(instrumentedApplication), instrumentedApplication)
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(instrumentedApplication, &testEnvVar, &testEnvOdigosValue, testProgrammingLanguage)
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(instrumentedApplication, &testEnvVarPythonPath, &testEnvOdigosValue, testProgrammingLanguagePython)
Expect(k8sClient.Update(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerEnvRemainEmpty(ctx, k8sClient, deployment)

Expand All @@ -110,22 +110,61 @@ var _ = Describe("envoverwrite", func() {

It("Should add the dockerfile env and odigos env to manifest and successfully revert", func() {
// initial state - should capture the env var from dockerfile only
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVar, &userEnvValue, testProgrammingLanguage)
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVarPythonPath, &userEnvValue, testProgrammingLanguagePython)
Expect(k8sClient.Create(ctx, instrumentedApplication)).Should(Succeed())

// odigos should merge the value from dockerfile and odigos env
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)

// after instrumentation is applied, now the value in the pod should be the merged value
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instrumentedApplication), instrumentedApplication)).Should(Succeed())
instrumentedApplication.Spec.RuntimeDetails[0].EnvVars[0].Value = mergedEnvValue
Expect(k8sClient.Update(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)

// when uninstrumented, the value should be reverted to the original value which was empty in manifest
Expect(k8sClient.Delete(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnvBecomesEmpty(ctx, k8sClient, deployment)
})

})

Describe("Container is run with env variables of other language than what it runs", func() {

userEnvValue := "/from_dockerfile"

BeforeEach(func() {
// the env var is in dockerfile, thus the manifest should start empty of env vars
deployment = testutil.SetOdigosInstrumentationEnabled(testutil.NewMockTestDeployment(namespace))
Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())
})

It("Should not add the unrelated env vars to the manifest", func() {
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVarPythonPath, &userEnvValue, common.JavaProgrammingLanguage)
Expect(k8sClient.Create(ctx, instrumentedApplication)).Should(Succeed())

// odigos found a relevant env var for python, but it should not be injected to the manifest
// as the language is not Python
testutil.AssertDepContainerEnvRemainEmpty(ctx, k8sClient, deployment)
})

It("Should not add the unrelated env vars with different otel SDKs", func() {
// make the SDK for python and java different
var cm corev1.ConfigMap
var odigosConfig common.OdigosConfiguration
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: consts.DefaultOdigosNamespace, Name: consts.OdigosConfigurationName}, &cm)).Should(Succeed())
GetOdigosConfig(&cm, &odigosConfig)
odigosConfig.DefaultSDKs[common.JavaProgrammingLanguage] = common.OtelSdkEbpfEnterprise
SetOdigosConfig(&cm, &odigosConfig)
Expect(k8sClient.Update(ctx, &cm)).Should(Succeed())

instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVarPythonPath, &userEnvValue, common.JavaProgrammingLanguage)
Expect(k8sClient.Create(ctx, instrumentedApplication)).Should(Succeed())

// odigos found a relevant env var for python, but it should not be injected to the manifest
// as the language is not Python
testutil.AssertDepContainerEnvRemainEmpty(ctx, k8sClient, deployment)
})
})

Describe("User set env var via manifest and not in dockerfile", func() {
Expand All @@ -139,27 +178,27 @@ var _ = Describe("envoverwrite", func() {
testutil.SetOdigosInstrumentationEnabled(
testutil.NewMockTestDeployment(namespace),
),
testEnvVar, userEnvValue)
testEnvVarPythonPath, userEnvValue)
Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())
})

It("Should update the manifest with merged value, and revet when uninstrumenting", func() {
// initial state - should capture the env var from manifest only
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVar, &userEnvValue, testProgrammingLanguage)
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVarPythonPath, &userEnvValue, testProgrammingLanguagePython)
Expect(k8sClient.Create(ctx, instrumentedApplication)).Should(Succeed())

// odigos should merge the value from manifest and odigos env
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)

// after instrumentation is applied, now the value in the pod should be the merged value
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instrumentedApplication), instrumentedApplication)).Should(Succeed())
instrumentedApplication.Spec.RuntimeDetails[0].EnvVars[0].Value = mergedEnvValue
Expect(k8sClient.Update(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)

// when uninstrumented, the value should be reverted to the original value which was in the manifest
Expect(k8sClient.Delete(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, userEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, userEnvValue)
})
})

Expand All @@ -173,20 +212,20 @@ var _ = Describe("envoverwrite", func() {
testutil.SetOdigosInstrumentationEnabled(
testutil.NewMockTestDeployment(namespace),
),
testEnvVar, userEnvValue)
testEnvVarPythonPath, userEnvValue)
Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())

// initial state - should capture the env var from manifest only
mergedEnvValue := userEnvValue + ":" + testEnvOdigosValue
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVar, &userEnvValue, testProgrammingLanguage)
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVarPythonPath, &userEnvValue, testProgrammingLanguagePython)
Expect(k8sClient.Create(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)

// after instrumentation is applied, now the value in the pod should be the merged value
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instrumentedApplication), instrumentedApplication)).Should(Succeed())
instrumentedApplication.Spec.RuntimeDetails[0].EnvVars[0].Value = mergedEnvValue
Expect(k8sClient.Update(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)
})

When("Default SDK changes to another SDK", func() {
Expand All @@ -197,22 +236,22 @@ var _ = Describe("envoverwrite", func() {
var odigosConfig common.OdigosConfiguration
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: consts.DefaultOdigosNamespace, Name: consts.OdigosConfigurationName}, &cm)).Should(Succeed())
GetOdigosConfig(&cm, &odigosConfig)
odigosConfig.DefaultSDKs[testProgrammingLanguage] = newSdk
odigosConfig.DefaultSDKs[testProgrammingLanguagePython] = newSdk
SetOdigosConfig(&cm, &odigosConfig)
Expect(k8sClient.Update(ctx, &cm)).Should(Succeed())
})

It("Should update the manifest with new odigos env value", func() {
newOdigosValue, found := envOverwrite.ValToAppend(testEnvVar, newSdk)
newOdigosValue, found := envOverwrite.ValToAppend(testEnvVarPythonPath, newSdk)
Expect(found).Should(BeTrue())
newMergedEnvValue := userEnvValue + ":" + newOdigosValue

// after the odigos config is updated, the deployment should be updated with the new odigos value
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, newMergedEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, newMergedEnvValue)

// when uninstrumented, the value should be reverted to the original value which was in the manifest
Expect(k8sClient.Delete(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, userEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, userEnvValue)
})
})
})
Expand All @@ -228,31 +267,31 @@ var _ = Describe("envoverwrite", func() {
testutil.SetOdigosInstrumentationEnabled(
testutil.NewMockTestDeployment(namespace),
),
testEnvVar, userEnvValue)
testEnvVarPythonPath, userEnvValue)
Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())

// initial state - should capture the env var from manifest only
mergedEnvValue = userEnvValue + ":" + testEnvOdigosValue
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVar, &userEnvValue, testProgrammingLanguage)
instrumentedApplication = testutil.SetInstrumentedApplicationContainer(testutil.NewMockInstrumentedApplication(deployment), &testEnvVarPythonPath, &userEnvValue, testProgrammingLanguagePython)
Expect(k8sClient.Create(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)

// after instrumentation is applied, now the value in the pod should be the merged value
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instrumentedApplication), instrumentedApplication)).Should(Succeed())
instrumentedApplication.Spec.RuntimeDetails[0].EnvVars[0].Value = mergedEnvValue
Expect(k8sClient.Update(ctx, instrumentedApplication)).Should(Succeed())
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnvRemainsSame(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)
})

It("Should reapply odigos value to the manifest", func() {
// when the deployment is updated, the odigos value should be reapplied
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), deployment)).Should(Succeed())
// restore the value to the original state
deployment = testutil.SetDeploymentContainerEnv(deployment, testEnvVar, userEnvValue)
deployment = testutil.SetDeploymentContainerEnv(deployment, testEnvVarPythonPath, userEnvValue)
Expect(k8sClient.Update(ctx, deployment)).Should(Succeed())

// the odigos value should be reapplied
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVar, mergedEnvValue)
testutil.AssertDepContainerSingleEnv(ctx, k8sClient, deployment, testEnvVarPythonPath, mergedEnvValue)
})
})

Expand Down
Loading
Loading