@kubewarden/kubewarden-developers I'm adding this in the Pending review column as draft to give you the oportunity to sneak and peak and give initial feedback while I'm testing this with a Stackstack cluster.

I think this looks good, there's still the chance of someone re-defining (and I guess overload?) our configuration by specifying the same receiver/processor/... There's nothing we can do about that.

Yes, to mitigate this issue I've added a schema validation file in the chart. Therefore, helm chart will block the usage of the same names.

I just wonder if we shouldn't just give up and just let the user overwrite everything with its values. That would simplify the helm chart (get rid of all the if .Values.<> that check for non empty values).

I've considered that first. But then I've remember that our components expect the collector to be a sidecar. Therefore, considering that we are not thinking on change that for now, if we allow the free form approach we can break out environment because the user change the deployment mode intentionally or not. I understand that we can document this. But I think even documenting that, we will get issues from users complaining that "telemetry is broken". But this is open to discussion. :)

Finally, I love the unit tests you added 👏 ❤️

Thanks! I've really liked the tests as well.

@jvanz I like the approach (and the tests).

Just one comment: the right short name for OpenTelemetry is otel, not optel, so we might want to change the names of the values accordingly.

I also like it, I think is understandable. Maybe a commented line saying that the extra* values have precedence over the other collector.

Another option to consider would be to provide an "enum switch" like we and other Rancher charts used to do for cert-manager (example), if we foresee that this is going to be complicated and we can provide some defaults. E.g: manual-config-with-extra, stackstate-config, etc.

I also like it, I think is understandable. Maybe a commented line saying that the extra* values have precedence over the other collector.

Actually, the extra configuration does not have precedence. It will live together with the previous configuration. In other words, the old otel pipeline remains the same. Nothing changes. But if the user define an extra configuration it will be merged into the otel collector configuration as well.

@kubewarden/kubewarden-developers using this PR with the following values file, I'm able to have a minimum configuration to keep the original Kubewarden OTEL pipelines and add another to send data to Stackstate:

telemetry:
  metrics:
    enabled: True
    port: 8080
  tracing:
    enabled: True
    jaeger:
      endpoint: "my-open-telemetry-collector.jaeger.svc.cluster.local:4317"
      tls:
        insecure: true
  extraOtelConfig:
    envFrom:
      - secretRef:
          name: open-telemetry-collector
  extraConfig: 
    processors:
      resource:
        attributes:
        - key: k8s.cluster.name
          action: upsert
          value: k3d-kubewarden
        - key: service.instance.id
          from_attribute: k8s.pod.uid
          action: insert
    extensions:
      bearertokenauth:
        scheme: SUSEObservability
        token: "${env:API_KEY}"
    exporters:
      debug:
        verbosity: normal
      otlphttp/stackstate:
        auth:
          authenticator: bearertokenauth
        endpoint: https://otlp-stackstate.oldfield.arch.nue2.suse.org:443
        tls:
          insecure_skip_verify: true
    service:
      extensions:
        - bearertokenauth
      pipelines:
        traces/stackstate:
          receivers: [otlp]
          processors: [resource]
          exporters: [otlphttp/stackstate]
        metrics/stackstate:
          receivers: [otlp]
          processors: [resource]
          exporters: [debug, otlphttp/stackstate]

This is the result OTEL collector:

---
# Source: kubewarden-controller/templates/opentelemetry-collector.yaml
apiVersion: opentelemetry.io/v1beta1
kind: OpenTelemetryCollector
metadata:
  name: kubewarden
  namespace: default
  labels:
    helm.sh/chart: kubewarden-controller-3.1.0
    app.kubernetes.io/name: kubewarden-controller
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v1.18.0"
    app.kubernetes.io/component: controller
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/part-of: kubewarden
  annotations:
    
spec:
  mode: sidecar
  envFrom:
    - secretRef:
        name: open-telemetry-collector
  config:
    extensions:
      bearertokenauth:
        scheme: SUSEObservability
        token: ${env:API_KEY}
    receivers:
      otlp:
        protocols:
          grpc: {}
    processors:
      batch: {}
      resource:
        attributes:
        - action: upsert
          key: k8s.cluster.name
          value: k3d-kubewarden
        - action: insert
          from_attribute: k8s.pod.uid
          key: service.instance.id
    exporters:
      otlp/jaeger:
        endpoint: my-open-telemetry-collector.jaeger.svc.cluster.local:4317
        tls:
          insecure: true
      prometheus:
        endpoint: ":8080"
      
      debug:
        verbosity: normal
      otlphttp/stackstate:
        auth:
          authenticator: bearertokenauth
        endpoint: https://otlp-stackstate.oldfield.arch.nue2.suse.org:443
        tls:
          insecure_skip_verify: true
    service:
      extensions:
        - bearertokenauth
      pipelines:
        metrics/stackstate:
          exporters:
          - debug
          - otlphttp/stackstate
          processors:
          - resource
          receivers:
          - otlp
        traces/stackstate:
          exporters:
          - otlphttp/stackstate
          processors:
          - resource
          receivers:
          - otlp
        metrics:
          receivers: [otlp]
          processors: []
          exporters: [prometheus]
        traces:
          receivers: [otlp]
          processors: [batch]
          exporters: [otlp/jaeger]

@kubewarden/kubewarden-developers , during the tests I've figured out that our current approach to deploy OTEL collector using sidecar, prevents us from using the Kubernetes Attributes processor. Which pretty important to fetch metadata from the cluster and allow the association of the metrics/traces with the resources running in the cluster:

The Kubernetes Attributes Processor is one of the most important components for a collector running in Kubernetes. Any collector receiving application data should use it. Because it adds Kubernetes context to your telemetry, the Kubernetes Attributes Processor lets you correlate your application’s traces, metrics, and logs signals with your Kubernetes telemetry, such as pod metrics and traces.

This is configuration required for Stackstate as show in the documentation example. Therefore, I think we need a second iteration on this to improve our OTEL deployment allowing us to enable this processor available only in DaemonSet and Deployment deployments mode. This may require changes in policy server and Kubewarden controller because these components always consider the collector accessible in the same pod.

@kubewarden/kubewarden-developers as we've discussed during a call, I've updated this PR to move the OTEL configuration from the template to the values file. I've also update the tests to reflect that and start a new test to cover the controller deployment to test a change there as well.

I'm working in the end-to-end tests repository to make it work with the latest changes as well.