Repository: honeycombio/honeycomb-kubernetes-agent (go)

Issue:
drop_field processor is done before the kubernetes metadata so you can't drop kubernetes.container.env and others
<!---
Thank you for taking the time to report bugs!

We love code snippets and links to repositories that reproduce the issue, but understand if you don't have the time to add them. We'll do our best with the info you provide, and might ask follow-up questions.

Please see our [OSS process document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#) to get an idea of how we operate.
--->

**Versions**

honeycomb-kubernetes-agent 2.5.4
Kubernetes is k3s 1.24

**Steps to reproduce**

1. Add a drop_field processor which specifies a `kubernetes.` field
2. Note that it's not dropped

**Additional context**

Looking at the code, it appears that the processors are appended to an array and processed in order. The configured processors happen first followed by the kubernetes metadata and finally the global fields. 

Suggest that it be processed in the order of k8s metadata first, then configured processors and finally global fields.


Pull request:
Put the k8s metadata processor first
## Which problem is this PR solving?

- Processors in the k8s agent might want to work on fields injected by the k8s processor, but the implementation put that processor at the end of the list. 

## Short description of the changes

- Put the k8s processor at the beginning of the processor list.

## Notes

Because the k8s metadata processor is not optional and has no configuration, there was no easy way to make it an optional feature to move it in the processing order. There's a slight possibility that this change could cause an existing configuration to deliver different results, but I believe that the only chance of that happening is in a configuration where the k8s metadata processor was overwriting fields that were modified by another processor. That doesn't seem like a useful configuration for anyone so I don't think this needs a "breaking" change warning (and in fact, #296 calls it a bug).

@puckpuck and @mterhar I would appreciate your review on this one.

Fixes #296.



Patch:
diff --git a/handlers/handlers.go b/handlers/handlers.go
index 996b08d..faa7d07 100644
--- a/handlers/handlers.go
+++ b/handlers/handlers.go
@@ -63,8 +63,14 @@ func NewLineHandlerFactoryFromConfig(
 		}
 		ret.processors = append(ret.processors, processor)
 	}
-	ret.processors = append(ret.processors, extraProcessors...)
-
+	for _, ep := range extraProcessors {
+		if _, ok := ep.(*processors.KubernetesMetadataProcessor); ok {
+			// put the K8s metadata processor first in line (see #296)
+			ret.processors = append([]processors.Processor{ep}, ret.processors...)
+		} else {
+			ret.processors = append(ret.processors, ep)
+		}
+	}
 	return ret, nil
 }
 


Test patch:
diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go
index 9208f12..e3d2415 100644
--- a/handlers/handlers_test.go
+++ b/handlers/handlers_test.go
@@ -767,3 +767,30 @@ func TestKubernetesMetadata(t *testing.T) {
 	assert.Equal(t, mt.events[0].Data["kubernetes.container.image"], "containerImage")
 	assert.Equal(t, mt.events[0].Data["kubernetes.pod.UID"], "examplePodUID")
 }
+
+func TestKubernetesMetadataGoesFirst(t *testing.T) {
+	mt := &MockTransmitter{}
+	cfg := &config.WatcherConfig{
+		Dataset: "kubernetestest",
+		Parser:  &config.ParserConfig{Name: "json"},
+		Processors: []map[string]map[string]interface{}{
+			{
+				"drop_field": {"field": "kubernetes.pod.UID"},
+			},
+		},
+	}
+	k8sProcessor := &processors.KubernetesMetadataProcessor{
+		UID:       types.UID("examplePodUID"),
+		PodGetter: &mockPodGetter{},
+	}
+	hf, err := NewLineHandlerFactoryFromConfig(cfg, &unwrappers.RawLogUnwrapper{}, mt, k8sProcessor)
+	assert.NoError(t, err)
+
+	handler := hf.New("/var/log/pods/examplePodUID/container/0.log")
+	handler.Handle(`{"field": "a"}`)
+	assert.Equal(t, len(mt.events), 1)
+
+	assert.Equal(t, "container", mt.events[0].Data["kubernetes.container.name"])
+	assert.Equal(t, "containerImage", mt.events[0].Data["kubernetes.container.image"])
+	assert.Nil(t, mt.events[0].Data["kubernetes.pod.UID"]) // this field exists in the source but should be dropped
+}