Skip to content

Commit 4f7cbe8

Browse files
atoulmeatoulme
committed
[chore] simplify code by using a method that appends to a list if the value is not present yet
1 parent 1a0a729 commit 4f7cbe8

10 files changed

+145
-118
lines changed

‎pkg/instrumentation/apachehttpd.go

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

6969
// inject env vars
70-
for _, env := range apacheSpec.Env {
71-
idx := getIndexOfEnv(container.Env, env.Name)
72-
if idx == -1 {
73-
container.Env = append(container.Env, env)
74-
}
75-
}
70+
container.Env = appendIfNotSet(container.Env, apacheSpec.Env...)
7671

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

‎pkg/instrumentation/dotnet.go

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

8787
// inject .NET instrumentation spec env vars.
88-
for _, env := range dotNetSpec.Env {
89-
idx := getIndexOfEnv(container.Env, env.Name)
90-
if idx == -1 {
91-
container.Env = append(container.Env, env)
92-
}
93-
}
88+
container.Env = appendIfNotSet(container.Env, dotNetSpec.Env...)
9489

9590
const (
9691
doNotConcatEnvValues = false

‎pkg/instrumentation/exporter.go

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

2828
func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *corev1.Container) {
2929
if exporter.Endpoint != "" {
30-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterOTLPEndpoint) == -1 {
31-
container.Env = append(container.Env, corev1.EnvVar{
32-
Name: constants.EnvOTELExporterOTLPEndpoint,
33-
Value: exporter.Endpoint,
34-
})
35-
}
30+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
31+
Name: constants.EnvOTELExporterOTLPEndpoint,
32+
Value: exporter.Endpoint,
33+
})
3634
}
3735
if exporter.TLS == nil {
3836
return
@@ -52,36 +50,30 @@ func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *c
5250
if filepath.IsAbs(exporter.TLS.CA) {
5351
envVarVal = exporter.TLS.CA
5452
}
55-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterCertificate) == -1 {
56-
container.Env = append(container.Env, corev1.EnvVar{
57-
Name: constants.EnvOTELExporterCertificate,
58-
Value: envVarVal,
59-
})
60-
}
53+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
54+
Name: constants.EnvOTELExporterCertificate,
55+
Value: envVarVal,
56+
})
6157
}
6258
if exporter.TLS.Cert != "" {
6359
envVarVal := fmt.Sprintf("%s/%s", secretMountPath, exporter.TLS.Cert)
6460
if filepath.IsAbs(exporter.TLS.Cert) {
6561
envVarVal = exporter.TLS.Cert
6662
}
67-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterClientCertificate) == -1 {
68-
container.Env = append(container.Env, corev1.EnvVar{
69-
Name: constants.EnvOTELExporterClientCertificate,
70-
Value: envVarVal,
71-
})
72-
}
63+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
64+
Name: constants.EnvOTELExporterClientCertificate,
65+
Value: envVarVal,
66+
})
7367
}
7468
if exporter.TLS.Key != "" {
7569
envVarVar := fmt.Sprintf("%s/%s", secretMountPath, exporter.TLS.Key)
7670
if filepath.IsAbs(exporter.TLS.Key) {
7771
envVarVar = exporter.TLS.Key
7872
}
79-
if getIndexOfEnv(container.Env, constants.EnvOTELExporterClientKey) == -1 {
80-
container.Env = append(container.Env, corev1.EnvVar{
81-
Name: constants.EnvOTELExporterClientKey,
82-
Value: envVarVar,
83-
})
84-
}
73+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
74+
Name: constants.EnvOTELExporterClientKey,
75+
Value: envVarVar,
76+
})
8577
}
8678

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

‎pkg/instrumentation/golang.go

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

8282
// Inject Go instrumentation spec env vars.
8383
// For Go, env vars must be added to the agent contain
84-
for _, env := range goSpec.Env {
85-
idx := getIndexOfEnv(goAgent.Env, env.Name)
86-
if idx == -1 {
87-
goAgent.Env = append(goAgent.Env, env)
88-
}
89-
}
84+
goAgent.Env = appendIfNotSet(goAgent.Env, goSpec.Env...)
9085

9186
pod.Spec.Containers = append(pod.Spec.Containers, goAgent)
9287
pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{

‎pkg/instrumentation/javaagent.go

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

4444
// inject Java instrumentation spec env vars.
45-
for _, env := range javaSpec.Env {
46-
idx := getIndexOfEnv(container.Env, env.Name)
47-
if idx == -1 {
48-
container.Env = append(container.Env, env)
49-
}
50-
}
45+
container.Env = appendIfNotSet(container.Env, javaSpec.Env...)
5146

5247
javaJVMArgument := javaAgent
5348
if len(javaSpec.Extensions) > 0 {

‎pkg/instrumentation/nginx.go

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

7070
// inject env vars
71-
for _, env := range nginxSpec.Env {
72-
idx := getIndexOfEnv(container.Env, env.Name)
73-
if idx == -1 {
74-
container.Env = append(container.Env, env)
75-
}
76-
}
71+
container.Env = appendIfNotSet(container.Env, nginxSpec.Env...)
7772

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

‎pkg/instrumentation/nodejs.go

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

4242
// inject NodeJS instrumentation spec env vars.
43-
for _, env := range nodeJSSpec.Env {
44-
idx := getIndexOfEnv(container.Env, env.Name)
45-
if idx == -1 {
46-
container.Env = append(container.Env, env)
47-
}
48-
}
43+
container.Env = appendIfNotSet(container.Env, nodeJSSpec.Env...)
4944

5045
idx := getIndexOfEnv(container.Env, envNodeOptions)
5146
if idx == -1 {

‎pkg/instrumentation/python.go

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

6363
// inject Python instrumentation spec env vars.
64-
for _, env := range pythonSpec.Env {
65-
idx := getIndexOfEnv(container.Env, env.Name)
66-
if idx == -1 {
67-
container.Env = append(container.Env, env)
68-
}
69-
}
64+
container.Env = appendIfNotSet(container.Env, pythonSpec.Env...)
7065

7166
idx := getIndexOfEnv(container.Env, envPythonPath)
7267
if idx == -1 {
@@ -78,41 +73,28 @@ func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int, plat
7873
container.Env[idx].Value = fmt.Sprintf("%s:%s:%s", pythonPathPrefix, container.Env[idx].Value, pythonPathSuffix)
7974
}
8075

81-
// Set OTEL_EXPORTER_OTLP_PROTOCOL to http/protobuf if not set by user because it is what our autoinstrumentation supports.
82-
idx = getIndexOfEnv(container.Env, envOtelExporterOTLPProtocol)
83-
if idx == -1 {
84-
container.Env = append(container.Env, corev1.EnvVar{
76+
container.Env = appendIfNotSet(container.Env,
77+
// Set OTEL_EXPORTER_OTLP_PROTOCOL to http/protobuf if not set by user because it is what our autoinstrumentation supports.
78+
corev1.EnvVar{
8579
Name: envOtelExporterOTLPProtocol,
8680
Value: "http/protobuf",
87-
})
88-
}
89-
90-
// Set OTEL_TRACES_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
91-
idx = getIndexOfEnv(container.Env, envOtelTracesExporter)
92-
if idx == -1 {
93-
container.Env = append(container.Env, corev1.EnvVar{
81+
},
82+
// Set OTEL_TRACES_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
83+
corev1.EnvVar{
9484
Name: envOtelTracesExporter,
9585
Value: "otlp",
96-
})
97-
}
98-
99-
// Set OTEL_METRICS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
100-
idx = getIndexOfEnv(container.Env, envOtelMetricsExporter)
101-
if idx == -1 {
102-
container.Env = append(container.Env, corev1.EnvVar{
86+
},
87+
// Set OTEL_METRICS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
88+
corev1.EnvVar{
10389
Name: envOtelMetricsExporter,
10490
Value: "otlp",
105-
})
106-
}
107-
108-
// Set OTEL_LOGS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
109-
idx = getIndexOfEnv(container.Env, envOtelLogsExporter)
110-
if idx == -1 {
111-
container.Env = append(container.Env, corev1.EnvVar{
91+
},
92+
// Set OTEL_LOGS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports.
93+
corev1.EnvVar{
11294
Name: envOtelLogsExporter,
11395
Value: "otlp",
114-
})
115-
}
96+
},
97+
)
11698

11799
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
118100
Name: volume.Name,

‎pkg/instrumentation/sdk.go

+26-25
Original file line numberDiff line numberDiff line change
@@ -253,36 +253,26 @@ func getContainerIndex(containerName string, pod corev1.Pod) int {
253253
func (i *sdkInjector) injectCommonEnvVar(otelinst v1alpha1.Instrumentation, pod corev1.Pod, index int) corev1.Pod {
254254
container := &pod.Spec.Containers[index]
255255

256-
idx := getIndexOfEnv(container.Env, constants.EnvPodIP)
257-
if idx == -1 {
258-
container.Env = append([]corev1.EnvVar{{
256+
container.Env = appendIfNotSet(container.Env,
257+
corev1.EnvVar{
259258
Name: constants.EnvPodIP,
260259
ValueFrom: &corev1.EnvVarSource{
261260
FieldRef: &corev1.ObjectFieldSelector{
262261
FieldPath: "status.podIP",
263262
},
264263
},
265-
}}, container.Env...)
266-
}
267-
268-
idx = getIndexOfEnv(container.Env, constants.EnvNodeIP)
269-
if idx == -1 {
270-
container.Env = append([]corev1.EnvVar{{
264+
},
265+
corev1.EnvVar{
271266
Name: constants.EnvNodeIP,
272267
ValueFrom: &corev1.EnvVarSource{
273268
FieldRef: &corev1.ObjectFieldSelector{
274269
FieldPath: "status.hostIP",
275270
},
276271
},
277-
}}, container.Env...)
278-
}
272+
},
273+
)
279274

280-
for _, env := range otelinst.Spec.Env {
281-
idx := getIndexOfEnv(container.Env, env.Name)
282-
if idx == -1 {
283-
container.Env = append(container.Env, env)
284-
}
285-
}
275+
container.Env = appendIfNotSet(container.Env, otelinst.Spec.Env...)
286276
return pod
287277
}
288278

@@ -297,13 +287,10 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph
297287
container := &pod.Spec.Containers[agentIndex]
298288
useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes
299289
resourceMap := i.createResourceMap(ctx, otelinst, ns, pod, appIndex)
300-
idx := getIndexOfEnv(container.Env, constants.EnvOTELServiceName)
301-
if idx == -1 {
302-
container.Env = append(container.Env, corev1.EnvVar{
303-
Name: constants.EnvOTELServiceName,
304-
Value: chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, appIndex),
305-
})
306-
}
290+
container.Env = appendIfNotSet(container.Env, corev1.EnvVar{
291+
Name: constants.EnvOTELServiceName,
292+
Value: chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, appIndex),
293+
})
307294
configureExporter(otelinst.Spec.Exporter, &pod, container)
308295

309296
// Always retrieve the pod name from the Downward API. Ensure that the OTEL_RESOURCE_ATTRIBUTES_POD_NAME env exists.
@@ -332,7 +319,7 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph
332319
}
333320
}
334321

335-
idx = getIndexOfEnv(container.Env, constants.EnvOTELResourceAttrs)
322+
idx := getIndexOfEnv(container.Env, constants.EnvOTELResourceAttrs)
336323
if idx == -1 || !strings.Contains(container.Env[idx].Value, string(semconv.ServiceVersionKey)) {
337324
vsn := chooseServiceVersion(pod, useLabelsForResourceAttributes, appIndex)
338325
if vsn != "" {
@@ -661,6 +648,20 @@ func getIndexOfEnv(envs []corev1.EnvVar, name string) int {
661648
return -1
662649
}
663650

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

‎pkg/instrumentation/sdk_test.go

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

0 commit comments

Comments
 (0)