From 5938b1d8e95f89a0f354f16360f577d90ceda67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= Date: Tue, 30 Apr 2024 11:16:44 +0200 Subject: [PATCH] fix(sourceprocessor): don't ignore empty annotation values Annotation values which are empty strings should be treated like any other value, and not ignored as if they didn't exist. --- .changelog/1569.fixed.txt | 1 + .../sourceprocessor/source_category_filler.go | 34 ++++++++----------- .../source_category_filler_test.go | 27 +++++++++++++++ .../sourceprocessor/source_processor_test.go | 20 ++++++++--- 4 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 .changelog/1569.fixed.txt diff --git a/.changelog/1569.fixed.txt b/.changelog/1569.fixed.txt new file mode 100644 index 0000000000..6287d61d6c --- /dev/null +++ b/.changelog/1569.fixed.txt @@ -0,0 +1 @@ +fix(sourceprocessor): don't ignore empty annotation values \ No newline at end of file diff --git a/pkg/processor/sourceprocessor/source_category_filler.go b/pkg/processor/sourceprocessor/source_category_filler.go index cde80c710c..9816d815b6 100644 --- a/pkg/processor/sourceprocessor/source_category_filler.go +++ b/pkg/processor/sourceprocessor/source_category_filler.go @@ -104,35 +104,31 @@ func (f *sourceCategoryFiller) fill(attributes *pcommon.Map) { } func (f *sourceCategoryFiller) getSourceCategoryDashReplacement(attributes *pcommon.Map) string { - dashReplacement := getAnnotationAttributeValue(f.annotationPrefix, sourceCategoryReplaceDashAnnotation, attributes) - if dashReplacement != "" { + dashReplacement, found := getAnnotationAttributeValue(f.annotationPrefix, sourceCategoryReplaceDashAnnotation, attributes) + if found { return dashReplacement } - dashReplacement = getAnnotationAttributeValue(f.namespaceAnnotationPrefix, sourceCategoryReplaceDashAnnotation, attributes) - if dashReplacement != "" { + dashReplacement, found = getAnnotationAttributeValue(f.namespaceAnnotationPrefix, sourceCategoryReplaceDashAnnotation, attributes) + if found { return dashReplacement } return f.dashReplacement } func (f *sourceCategoryFiller) getSourceCategoryFromAnnotation(annotationPrefix string, attributes *pcommon.Map) (string, bool) { - doesUseAnnotation := false - - valueTemplate := getAnnotationAttributeValue(annotationPrefix, sourceCategorySpecialAnnotation, attributes) - if valueTemplate == "" { + valueTemplate, foundTemplate := getAnnotationAttributeValue(annotationPrefix, sourceCategorySpecialAnnotation, attributes) + if !foundTemplate { valueTemplate = f.valueTemplate - } else { - doesUseAnnotation = true } - prefix := getAnnotationAttributeValue(annotationPrefix, sourceCategoryPrefixAnnotation, attributes) - if prefix == "" { + prefix, foundPrefix := getAnnotationAttributeValue(annotationPrefix, sourceCategoryPrefixAnnotation, attributes) + if !foundPrefix { prefix = f.prefix - } else { - doesUseAnnotation = true } + valueTemplate = prefix + valueTemplate + doesUseAnnotation := foundPrefix || foundTemplate return valueTemplate, doesUseAnnotation } @@ -150,8 +146,8 @@ func (f *sourceCategoryFiller) getSourceCategoryFromContainerAnnotation(attribut for _, containerAnnotationPrefix := range f.containerAnnotationsPrefixes { annotationKey := fmt.Sprintf("%s%s.sourceCategory", containerAnnotationPrefix, containerName.Str()) - annotationValue := getAnnotationAttributeValue(f.annotationPrefix, annotationKey, attributes) - if annotationValue != "" { + annotationValue, found := getAnnotationAttributeValue(f.annotationPrefix, annotationKey, attributes) + if found { f.logger.Debug("Filled source category from container annotation", zap.String("annotation", annotationKey), zap.String("source_category", annotationValue), @@ -181,10 +177,10 @@ func (f *sourceCategoryFiller) replaceTemplateAttributes(template string, templa return strings.NewReplacer(replacerArgs...).Replace(template) } -func getAnnotationAttributeValue(annotationAttributePrefix string, annotation string, attributes *pcommon.Map) string { +func getAnnotationAttributeValue(annotationAttributePrefix string, annotation string, attributes *pcommon.Map) (string, bool) { annotationAttribute, found := attributes.Get(annotationAttributePrefix + annotation) if found { - return annotationAttribute.Str() + return annotationAttribute.Str(), found } - return "" + return "", false } diff --git a/pkg/processor/sourceprocessor/source_category_filler_test.go b/pkg/processor/sourceprocessor/source_category_filler_test.go index 8fb0c8a25b..e43fa4db79 100644 --- a/pkg/processor/sourceprocessor/source_category_filler_test.go +++ b/pkg/processor/sourceprocessor/source_category_filler_test.go @@ -66,6 +66,33 @@ func TestFillWithAnnotations(t *testing.T) { assertAttribute(t, attrs, "_sourceCategory", "ABC#123asd#Prefix:sc#from#annot#ns#1#123asd") } +func TestFillWithEmptyAnnotations(t *testing.T) { + cfg := createDefaultConfig().(*Config) + cfg.SourceCategoryPrefix = "prefix" + + attrs := pcommon.NewMap() + attrs.PutStr("k8s.namespace.name", "ns-1") + attrs.PutStr("k8s.pod.uid", "123asd") + attrs.PutStr("k8s.pod.pod_name", "ABC") + + filler := newSourceCategoryFiller(cfg, zap.NewNop()) + + // can replace prefix with an empty string + attrs.PutStr("k8s.pod.annotation.sumologic.com/sourceCategoryPrefix", "") + filler.fill(&attrs) + assertAttribute(t, attrs, "_sourceCategory", "ns/1/ABC") + + // can replace dash with an empty string + attrs.PutStr("k8s.pod.annotation.sumologic.com/sourceCategoryReplaceDash", "") + filler.fill(&attrs) + assertAttribute(t, attrs, "_sourceCategory", "ns1/ABC") + + // can replace source category with empty string + attrs.PutStr("k8s.pod.annotation.sumologic.com/sourceCategory", "") + filler.fill(&attrs) + assertAttribute(t, attrs, "_sourceCategory", "") +} + func TestFillWithNamespaceAnnotations(t *testing.T) { cfg := createDefaultConfig().(*Config) diff --git a/pkg/processor/sourceprocessor/source_processor_test.go b/pkg/processor/sourceprocessor/source_processor_test.go index edec35609a..ad1e4742d6 100644 --- a/pkg/processor/sourceprocessor/source_processor_test.go +++ b/pkg/processor/sourceprocessor/source_processor_test.go @@ -499,6 +499,18 @@ func TestSourceCategoryAnnotations(t *testing.T) { assertAttribute(t, processedAttributes, "_sourceCategory", "annot>namespace#1/pod") }) + t.Run("source category empty prefix annotation", func(t *testing.T) { + inputAttributes := createK8sLabels() + inputAttributes["pod_annotation_sumologic.com/sourceCategoryPrefix"] = "" + inputTraces := newTraceData(inputAttributes) + + processedTraces, err := newSourceProcessor(newProcessorCreateSettings(), cfg).ProcessTraces(context.Background(), inputTraces) + assert.NoError(t, err) + + processedAttributes := processedTraces.ResourceSpans().At(0).Resource().Attributes() + assertAttribute(t, processedAttributes, "_sourceCategory", "namespace#1/pod") + }) + t.Run("source category dash replacement annotation", func(t *testing.T) { inputAttributes := createK8sLabels() inputAttributes["pod_annotation_sumologic.com/sourceCategoryReplaceDash"] = "^" @@ -649,14 +661,12 @@ func TestSourceCategoryTemplateWithCustomAttribute(t *testing.T) { func assertAttribute(t *testing.T, attributes pcommon.Map, attributeName string, expectedValue string) { value, exists := attributes.Get(attributeName) - if expectedValue == "" { + if !exists { assert.False(t, exists, "Attribute '%s' should not exist.", attributeName) } else { assert.True(t, exists, "Attribute '%s' should exist, but it does not.", attributeName) - if exists { - actualValue := value.Str() - assert.Equal(t, expectedValue, actualValue, "Attribute '%s' should be '%s', but was '%s'.", attributeName, expectedValue, actualValue) - } + actualValue := value.Str() + assert.Equal(t, expectedValue, actualValue, "Attribute '%s' should be '%s', but was '%s'.", attributeName, expectedValue, actualValue) } }