Not ignore the container-names annotation when the multi-instrumentat…

…ion is enabled (#3213)

* Not ignore the container-names annotation when the multi-instrumentation is enabled

Signed-off-by: Israel Blancas <[email protected]>

* Fixes

Signed-off-by: Israel Blancas <[email protected]>

* Fix unit tests

Signed-off-by: Israel Blancas <[email protected]>

* Apply changes requested in code review

Signed-off-by: Israel Blancas <[email protected]>

---------

Signed-off-by: Israel Blancas <[email protected]>

.chloggen/feature_3090.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: 'enhancement'
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: auto-instrumentation
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Not ignore the `instrumentation.opentelemetry.io/container-names` annotation when the multi-instrumentation is enabled"
+
+# One or more tracking issues related to the change
+issues: [3090]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:

pkg/instrumentation/helper.go

@@ -16,11 +16,13 @@ package instrumentation
 
 import (
 	"fmt"
+	"regexp"
 	"sort"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/strings/slices"
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/naming"
@@ -77,15 +79,9 @@ func isAutoInstrumentationInjected(pod corev1.Pod) bool {
 
 // Look for duplicates in the provided containers.
 func findDuplicatedContainers(ctrs []string) error {
-	// Merge is needed because of multiple containers can be provided for single instrumentation.
-	mergedContainers := strings.Join(ctrs, ",")
-
-	// Split all containers.
-	splitContainers := strings.Split(mergedContainers, ",")
-
 	countMap := make(map[string]int)
 	var duplicates []string
-	for _, str := range splitContainers {
+	for _, str := range ctrs {
 		countMap[str]++
 	}
 
@@ -111,7 +107,7 @@ func findDuplicatedContainers(ctrs []string) error {
 
 // Return positive for instrumentation with defined containers.
 func isInstrWithContainers(inst instrumentationWithContainers) int {
-	if inst.Containers != "" {
+	if len(inst.Containers) > 0 {
 		return 1
 	}
 
@@ -120,7 +116,7 @@ func isInstrWithContainers(inst instrumentationWithContainers) int {
 
 // Return positive for instrumentation without defined containers.
 func isInstrWithoutContainers(inst instrumentationWithContainers) int {
-	if inst.Containers == "" {
+	if len(inst.Containers) == 0 {
 		return 1
 	}
 
@@ -133,3 +129,33 @@ func volumeSize(quantity *resource.Quantity) *resource.Quantity {
 	}
 	return quantity
 }
+
+func isValidContainersAnnotation(containersAnnotation string) error {
+	if containersAnnotation == "" {
+		return nil
+	}
+
+	matched, err := regexp.MatchString("^[a-zA-Z0-9-,]+$", containersAnnotation)
+	if err != nil {
+		return fmt.Errorf("error while checking for instrumentation container annotations %w", err)
+	}
+	if !matched {
+		return fmt.Errorf("not valid characters included in the instrumentation container annotation %s", containersAnnotation)
+	}
+	return nil
+}
+
+// setContainersFromAnnotation sets the containers associated to one intrumentation based on the content of the provided annotation.
+func setContainersFromAnnotation(inst *instrumentationWithContainers, annotation string, ns metav1.ObjectMeta, pod metav1.ObjectMeta) error {
+	annotationValue := annotationValue(ns, pod, annotation)
+	if annotationValue == "" {
+		return nil
+	}
+
+	if err := isValidContainersAnnotation(annotationValue); err != nil {
+		return err
+	}
+	languageContainers := strings.Split(annotationValue, ",")
+	inst.Containers = append(inst.Containers, languageContainers...)
+	return nil
+}

pkg/instrumentation/helper_test.go

@@ -170,12 +170,12 @@ func TestDuplicatedContainers(t *testing.T) {
 	}{
 		{
 			name:               "No duplicates",
-			containers:         []string{"app1,app2", "app3", "app4,app5"},
+			containers:         []string{"app1", "app2", "app3", "app4", "app5"},
 			expectedDuplicates: nil,
 		},
 		{
 			name:               "Duplicates in containers",
-			containers:         []string{"app1,app2", "app1", "app1,app3,app4", "app4"},
+			containers:         []string{"app1", "app2", "app1", "app1", "app3", "app4", "app4"},
 			expectedDuplicates: fmt.Errorf("duplicated container names detected: [app1 app4]"),
 		},
 	}
@@ -196,12 +196,12 @@ func TestInstrWithContainers(t *testing.T) {
 	}{
 		{
 			name:           "No containers",
-			containers:     instrumentationWithContainers{Containers: ""},
+			containers:     instrumentationWithContainers{Containers: []string{}},
 			expectedResult: 0,
 		},
 		{
 			name:           "With containers",
-			containers:     instrumentationWithContainers{Containers: "ct1"},
+			containers:     instrumentationWithContainers{Containers: []string{"ct1"}},
 			expectedResult: 1,
 		},
 	}
@@ -222,12 +222,12 @@ func TestInstrWithoutContainers(t *testing.T) {
 	}{
 		{
 			name:           "No containers",
-			containers:     instrumentationWithContainers{Containers: ""},
+			containers:     instrumentationWithContainers{Containers: []string{}},
 			expectedResult: 1,
 		},
 		{
 			name:           "With containers",
-			containers:     instrumentationWithContainers{Containers: "ct1"},
+			containers:     instrumentationWithContainers{Containers: []string{"ct1"}},
 			expectedResult: 0,
 		},
 	}