Created annotations filter

[yuriolisa]

Description:

Created ability to filter out Annotations
Link to tracking Issue(s):

• Resolves: Propagated annotations causes issues for Deployment, StatefulSet and DaemonSet #2627

Testing:
Unit tests have been modified accordingly.

Documentation:

[jaronoff97]

a few small things, also can you augment or create an e2e test for this? do we have one for the label filter that we could tack this on to?

[jaronoff97]

i wonder if we should also move annotations.go and labels.go to a shared thing in manifestutils?

[yuriolisa]

To be honest, I've tried to do that, but I faced an import cycle issue. I believe we could dig into that in the future, moving the annotations to the manifestutils. wdyt?

[jaronoff97]

yeah that makes sense, want to open an issue about that?

[yuriolisa]

a few small things, also can you augment or create an e2e test for this? do we have one for the label filter that we could tack this on to?

Added some e2e for Labels and Annotations filters.

Do you think we should change the parameter from labels to labels-filter? I know it's a breaking change, however, this name makes more sense for that purpose.

[jaronoff97]

@yuriolisa i think if we make that change (renaming labels) we should do it in a follow up

[jaronoff97]

also, looks like the e2e tests are failing on make:
make: unrecognized option '--annotations=*filter.out'

[yuriolisa]

@yuriolisa i think if we make that change (renaming labels) we should do it in a follow-up

So, if we intend to do a follow-up, I decided to set the annotations parameters as annotations-filter.

[yuriolisa]

also, looks like the e2e tests are failing on make: make: unrecognized option '--annotations=*filter.out'

Thanks for catching-up on that, it should be labels instead, once we don't have any e2e labels filter yet.

[jaronoff97]

one small nit, otherwise this LGTM. given the size of this, I'm going to wait to merge on another approver to give it a ✅

[swiatekm]

All of this looks good to me, although I think we ended up putting way too many changes into this PR. For example, the config hash annotation for opamp bridge definitely should've been a separate PR. I'm fine with merging as-is, though I wouldn't mind splitting it either.

The main thing that's actually unclear to me, is what the format of the --annotations-filter value is supposed to be. It looks like a glob, but then we parse it into a regex in a weird way. I'd like us to document it better, even if it's just a description of the flag.

[yuriolisa]

All of this looks good to me, although I think we ended up putting way too many changes into this PR. For example, the config hash annotation for opamp bridge definitely should've been a separate PR. I'm fine with merging as-is, though I wouldn't mind splitting it either.

The main thing that's actually unclear to me, is what the format of the --annotations-filter value is supposed to be. It looks like a glob, but then we parse it into a regex in a weird way. I'd like us to document it better, even if it's just a description of the flag.

@swiatekm-sumo, thank you for pointing out these topics. I will be aware next time to split into smaller parts. I've just sent an improvement to the parameter. Let me know wdyt.

[ryanohnemus]

@yuriolisa Thank you for adding this!!

[yuriolisa]

@yuriolisa Thank you for adding this!!

No problem, happy to contribute; I hope it's gonna helpful for your use case.

[ryanohnemus]

I just tested this out using the latest v0.96.0 image and the annotations aren't being filtered out as I'd expect. I'm not sure what i'm missing.

I setup a kind cluster via:

kind create cluster --config kind-1.29.yaml
kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.14.4/cert-manager.yaml

then I downloaded the latest install here:

curl -Lo otel-op.yaml https://github.com/open-telemetry/opentelemetry-operator/releases/latest/download/opentelemetry-operator.yaml

I then edited that to add the --annotations-filter arg below for the operator's Deployment:

kind: Deployment
metadata:
  labels:
    app.kubernetes.io/name: opentelemetry-operator
    control-plane: controller-manager
  name: opentelemetry-operator-controller-manager
  namespace: opentelemetry-operator-system
spec:
  replicas: 1
  selector:
    matchLabels:
      app.kubernetes.io/name: opentelemetry-operator
      control-plane: controller-manager
  template:
    metadata:
      labels:
        app.kubernetes.io/name: opentelemetry-operator
        control-plane: controller-manager
    spec:
      containers:
      - args:
        - --metrics-addr=127.0.0.1:8080
        - --enable-leader-election
        - --zap-log-level=info
        - --zap-time-encoding=rfc3339nano
        - --feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx
        - --annotations-filter="config.*.gke.io.*"
        image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.96.0

Finally I added an OpenTelemetryCollector resource that has an annotation I was expecting filtered out:

apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: simplest
  annotations:
    configmanagement.gke.io/token: "asdfasdf"
spec:
  config: |
    receivers:
      otlp:
        protocols:
          grpc:
          http:
    processors:
      memory_limiter:
        check_interval: 1s
        limit_percentage: 75
        spike_limit_percentage: 15
      batch:
        send_batch_size: 10000
        timeout: 10s

    exporters:
      debug:

    service:
      pipelines:
        traces:
          receivers: [otlp]
          processors: []
          exporters: [debug]

The configmanagement.gke.io/token: asdfasdf is still added to the template for the deployment of the collector and is still added to the pod.

Am I using the --annotations-filter option incorrectly?