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.

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

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

Merged
merged 10 commits into from
Aug 30, 2024
16 changes: 16 additions & 0 deletions .chloggen/feature_3090.yaml
Original file line number Diff line number Diff line change
@@ -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:
44 changes: 35 additions & 9 deletions pkg/instrumentation/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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]++
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}
12 changes: 6 additions & 6 deletions pkg/instrumentation/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]"),
},
}
Expand All @@ -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,
},
}
Expand All @@ -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,
},
}
Expand Down
Loading
Loading