Skip to content

Commit 8128cdb

Browse files
atoulmeatoulme
authored
[chore] simplify code by using a utility method (#3668)
* [chore] simplify code by using a method that appends to a list if the value is not present yet * order env vars to match existing
1 parent addb683 commit 8128cdb

10 files changed

+133
-93
lines changed

‎pkg/instrumentation/apachehttpd.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod
5656
container := &pod.Spec.Containers[index]
5757

5858
// inject env vars
59-
for _, env := range apacheSpec.Env {
60-
idx := getIndexOfEnv(container.Env, env.Name)
61-
if idx == -1 {
62-
container.Env = append(container.Env, env)
63-
}
64-
}
59+
container.Env = appendIfNotSet(container.Env, apacheSpec.Env...)
6560

6661
// First make a clone of the instrumented container to take the existing Apache configuration from
6762
// and create init container from it

‎pkg/instrumentation/dotnet.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,7 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runt
7474
}
7575

7676
// inject .NET instrumentation spec env vars.
77-
for _, env := range dotNetSpec.Env {
78-
idx := getIndexOfEnv(container.Env, env.Name)
79-
if idx == -1 {
80-
container.Env = append(container.Env, env)
81-
}
82-
}
77+
container.Env = appendIfNotSet(container.Env, dotNetSpec.Env...)
8378

8479
const (
8580
doNotConcatEnvValues = false

‎pkg/instrumentation/exporter.go

+16-24
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@ import (
1616

1717
func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *corev1.Container) {
1818
if exporter.Endpoint != "" {
19-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterOTLPEndpoint) == -1 {
20-
container.Env = append(container.Env, corev1.EnvVar{
21-
Name: constants.EnvOTELExporterOTLPEndpoint,
22-
Value: exporter.Endpoint,
23-
})
24-
}
19+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
20+
Name: constants.EnvOTELExporterOTLPEndpoint,
21+
Value: exporter.Endpoint,
22+
})
2523
}
2624
if exporter.TLS == nil {
2725
return
@@ -41,36 +39,30 @@ func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *c
4139
if filepath.IsAbs(exporter.TLS.CA) {
4240
envVarVal = exporter.TLS.CA
4341
}
44-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterCertificate) == -1 {
45-
container.Env = append(container.Env, corev1.EnvVar{
46-
Name: constants.EnvOTELExporterCertificate,
47-
Value: envVarVal,
48-
})
49-
}
42+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
43+
Name: constants.EnvOTELExporterCertificate,
44+
Value: envVarVal,
45+
})
5046
}
5147
if exporter.TLS.Cert != "" {
5248
envVarVal := fmt.Sprintf("%s/%s", secretMountPath, exporter.TLS.Cert)
5349
if filepath.IsAbs(exporter.TLS.Cert) {
5450
envVarVal = exporter.TLS.Cert
5551
}
56-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterClientCertificate) == -1 {
57-
container.Env = append(container.Env, corev1.EnvVar{
58-
Name: constants.EnvOTELExporterClientCertificate,
59-
Value: envVarVal,
60-
})
61-
}
52+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
53+
Name: constants.EnvOTELExporterClientCertificate,
54+
Value: envVarVal,
55+
})
6256
}
6357
if exporter.TLS.Key != "" {
6458
envVarVar := fmt.Sprintf("%s/%s", secretMountPath, exporter.TLS.Key)
6559
if filepath.IsAbs(exporter.TLS.Key) {
6660
envVarVar = exporter.TLS.Key
6761
}
68-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterClientKey) == -1 {
69-
container.Env = append(container.Env, corev1.EnvVar{
70-
Name: constants.EnvOTELExporterClientKey,
71-
Value: envVarVar,
72-
})
73-
}
62+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
63+
Name: constants.EnvOTELExporterClientKey,
64+
Value: envVarVar,
65+
})
7466
}
7567

7668
if exporter.TLS.SecretName != "" {

‎pkg/instrumentation/golang.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,7 @@ func injectGoSDK(goSpec v1alpha1.Go, pod corev1.Pod, cfg config.Config) (corev1.
7070

7171
// Inject Go instrumentation spec env vars.
7272
// For Go, env vars must be added to the agent contain
73-
for _, env := range goSpec.Env {
74-
idx := getIndexOfEnv(goAgent.Env, env.Name)
75-
if idx == -1 {
76-
goAgent.Env = append(goAgent.Env, env)
77-
}
78-
}
73+
goAgent.Env = appendIfNotSet(goAgent.Env, goSpec.Env...)
7974

8075
pod.Spec.Containers = append(pod.Spec.Containers, goAgent)
8176
pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{

‎pkg/instrumentation/javaagent.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,7 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
3131
}
3232

3333
// inject Java instrumentation spec env vars.
34-
for _, env := range javaSpec.Env {
35-
idx := getIndexOfEnv(container.Env, env.Name)
36-
if idx == -1 {
37-
container.Env = append(container.Env, env)
38-
}
39-
}
34+
container.Env = appendIfNotSet(container.Env, javaSpec.Env...)
4035

4136
javaJVMArgument := javaAgent
4237
if len(javaSpec.Extensions) > 0 {

‎pkg/instrumentation/nginx.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,7 @@ func injectNginxSDK(_ logr.Logger, nginxSpec v1alpha1.Nginx, pod corev1.Pod, use
5757
container := &pod.Spec.Containers[index]
5858

5959
// inject env vars
60-
for _, env := range nginxSpec.Env {
61-
idx := getIndexOfEnv(container.Env, env.Name)
62-
if idx == -1 {
63-
container.Env = append(container.Env, env)
64-
}
65-
}
60+
container.Env = appendIfNotSet(container.Env, nginxSpec.Env...)
6661

6762
// First make a clone of the instrumented container to take the existing Nginx configuration from
6863
// and create init container from it

‎pkg/instrumentation/nodejs.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,7 @@ func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int) (cor
2929
}
3030

3131
// inject NodeJS instrumentation spec env vars.
32-
for _, env := range nodeJSSpec.Env {
33-
idx := getIndexOfEnv(container.Env, env.Name)
34-
if idx == -1 {
35-
container.Env = append(container.Env, env)
36-
}
37-
}
32+
container.Env = appendIfNotSet(container.Env, nodeJSSpec.Env...)
3833

3934
idx := getIndexOfEnv(container.Env, envNodeOptions)
4035
if idx == -1 {

‎pkg/instrumentation/python.go

+15-33
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,7 @@ func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int, plat
5050
}
5151

5252
// inject Python instrumentation spec env vars.
53-
for _, env := range pythonSpec.Env {
54-
idx := getIndexOfEnv(container.Env, env.Name)
55-
if idx == -1 {
56-
container.Env = append(container.Env, env)
57-
}
58-
}
53+
container.Env = appendIfNotSet(container.Env, pythonSpec.Env...)
5954

6055
idx := getIndexOfEnv(container.Env, envPythonPath)
6156
if idx == -1 {
@@ -67,41 +62,28 @@ func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int, plat
6762
container.Env[idx].Value = fmt.Sprintf("%s:%s:%s", pythonPathPrefix, container.Env[idx].Value, pythonPathSuffix)
6863
}
6964

70-
// Set OTEL_EXPORTER_OTLP_PROTOCOL to http/protobuf if not set by user because it is what our autoinstrumentation supports.
71-
idx = getIndexOfEnv(container.Env, envOtelExporterOTLPProtocol)
72-
if idx == -1 {
73-
container.Env = append(container.Env, corev1.EnvVar{
65+
container.Env = appendIfNotSet(container.Env,
66+
// Set OTEL_EXPORTER_OTLP_PROTOCOL to http/protobuf if not set by user because it is what our autoinstrumentation supports.
67+
corev1.EnvVar{
7468
Name: envOtelExporterOTLPProtocol,
7569
Value: "http/protobuf",
76-
})
77-
}
78-
79-
// Set OTEL_TRACES_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
80-
idx = getIndexOfEnv(container.Env, envOtelTracesExporter)
81-
if idx == -1 {
82-
container.Env = append(container.Env, corev1.EnvVar{
70+
},
71+
// Set OTEL_TRACES_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
72+
corev1.EnvVar{
8373
Name: envOtelTracesExporter,
8474
Value: "otlp",
85-
})
86-
}
87-
88-
// Set OTEL_METRICS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
89-
idx = getIndexOfEnv(container.Env, envOtelMetricsExporter)
90-
if idx == -1 {
91-
container.Env = append(container.Env, corev1.EnvVar{
75+
},
76+
// Set OTEL_METRICS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
77+
corev1.EnvVar{
9278
Name: envOtelMetricsExporter,
9379
Value: "otlp",
94-
})
95-
}
96-
97-
// Set OTEL_LOGS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
98-
idx = getIndexOfEnv(container.Env, envOtelLogsExporter)
99-
if idx == -1 {
100-
container.Env = append(container.Env, corev1.EnvVar{
80+
},
81+
// Set OTEL_LOGS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
82+
corev1.EnvVar{
10183
Name: envOtelLogsExporter,
10284
Value: "otlp",
103-
})
104-
}
85+
},
86+
)
10587

10688
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
10789
Name: volume.Name,

‎pkg/instrumentation/sdk.go

+14
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,20 @@ func getIndexOfEnv(envs []corev1.EnvVar, name string) int {
650650
return -1
651651
}
652652

653+
func appendIfNotSet(envs []corev1.EnvVar, newEnvVars ...corev1.EnvVar) []corev1.EnvVar {
654+
keys := make(map[string]struct{}, len(envs))
655+
for _, e := range envs {
656+
keys[e.Name] = struct{}{}
657+
}
658+
for _, envVar := range newEnvVars {
659+
if _, ok := keys[envVar.Name]; !ok {
660+
envs = append(envs, envVar)
661+
keys[envVar.Name] = struct{}{}
662+
}
663+
}
664+
return envs
665+
}
666+
653667
func moveEnvToListEnd(envs []corev1.EnvVar, idx int) []corev1.EnvVar {
654668
if idx >= 0 && idx < len(envs) {
655669
envToMove := envs[idx]

‎pkg/instrumentation/sdk_test.go

+82
Original file line numberDiff line numberDiff line change
@@ -2552,3 +2552,85 @@ func TestChooseServiceName(t *testing.T) {
25522552
})
25532553
}
25542554
}
2555+
2556+
func TestAppendIfNotSet(t *testing.T) {
2557+
tests := []struct {
2558+
name string
2559+
envVars []corev1.EnvVar
2560+
newVars []corev1.EnvVar
2561+
result []corev1.EnvVar
2562+
}{
2563+
{
2564+
name: "add to empty",
2565+
newVars: []corev1.EnvVar{
2566+
{
2567+
Name: "foo",
2568+
Value: "bar",
2569+
},
2570+
},
2571+
result: []corev1.EnvVar{
2572+
{
2573+
Name: "foo",
2574+
Value: "bar",
2575+
},
2576+
},
2577+
},
2578+
{
2579+
name: "do not update if set",
2580+
envVars: []corev1.EnvVar{
2581+
{
2582+
Name: "foo",
2583+
Value: "bar",
2584+
},
2585+
},
2586+
newVars: []corev1.EnvVar{
2587+
{
2588+
Name: "foo",
2589+
Value: "foobar",
2590+
},
2591+
},
2592+
result: []corev1.EnvVar{
2593+
{
2594+
Name: "foo",
2595+
Value: "bar",
2596+
},
2597+
},
2598+
},
2599+
{
2600+
name: "mixed use",
2601+
envVars: []corev1.EnvVar{
2602+
{
2603+
Name: "foo",
2604+
Value: "bar",
2605+
},
2606+
},
2607+
newVars: []corev1.EnvVar{
2608+
{
2609+
Name: "foo",
2610+
Value: "foobar",
2611+
},
2612+
{
2613+
Name: "alpha",
2614+
Value: "beta",
2615+
},
2616+
},
2617+
result: []corev1.EnvVar{
2618+
{
2619+
Name: "foo",
2620+
Value: "bar",
2621+
},
2622+
{
2623+
Name: "alpha",
2624+
Value: "beta",
2625+
},
2626+
},
2627+
},
2628+
}
2629+
2630+
for _, test := range tests {
2631+
t.Run(test.name, func(t *testing.T) {
2632+
result := appendIfNotSet(test.envVars, test.newVars...)
2633+
assert.Equal(t, test.result, result)
2634+
})
2635+
}
2636+
}

0 commit comments

Comments
 (0)