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.

Sign up for GitHub

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

Jump to bottom

feat: extra OPTEL collector configuration. #581

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: extra OPTEL collector configuration. #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions charts/kubewarden-controller/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ spec:
{{- range keys .Values.podAnnotations }}
{{ . | quote }}: {{ get $.Values.podAnnotations . | quote}}
{{- end }}
{{- if or .Values.telemetry.metrics.enabled .Values.telemetry.tracing.enabled}}
{{- if or .Values.telemetry.metrics.enabled .Values.telemetry.tracing.enabled }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to add a new boolean flag inside of the helm chart values file that determines if he have to trigger otel's sidecar injection.

If a user has already otel deployed using a daemonset he will not want to have the sidecar injected.

"sidecar.opentelemetry.io/inject": "true"
{{- end }}
{{- include "kubewarden-controller.annotations" . | nindent 8 }}
Expand Down Expand Up @@ -56,10 +56,9 @@ spec:
- --zap-log-level={{ .Values.logLevel }}
command:
- /manager
{{- if .Values.telemetry.metrics.enabled }}
{{- if .Values.env }}
env:
- name: KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT
value: "{{ .Values.telemetry.metrics.port | default 8080 }}"
{{- toYaml .Values.env | nindent 10 }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: the kubewarden controller is in charge of creating the Deployment resource of the Policy Server. We have to extend the controller to have a new flag/env variable that determines whether the sidecar annotation has to be added to the Policy Server Deployment.

This env variable must then be exposed here

{{- end }}
image: '{{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}'
imagePullPolicy: {{ .Values.image.pullPolicy }}
Expand Down
40 changes: 2 additions & 38 deletions charts/kubewarden-controller/templates/opentelemetry-collector.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{ if or .Values.telemetry.metrics.enabled .Values.telemetry.tracing.enabled }}
{{ if or .Values.telemetry.metrics.enabled .Values.telemetry.tracing.enabled }}
apiVersion: opentelemetry.io/v1beta1
kind: OpenTelemetryCollector
metadata:
Expand All @@ -9,41 +9,5 @@ metadata:
annotations:
{{- include "kubewarden-controller.annotations" . | nindent 4 }}
spec:
mode: sidecar
config:
receivers:
otlp:
protocols:
grpc: {}
processors:
batch: {}
exporters:
{{- if and .Values.telemetry.tracing.enabled .Values.telemetry.tracing.jaeger.endpoint }}
otlp/jaeger:
endpoint: {{ .Values.telemetry.tracing.jaeger.endpoint }}
{{- if hasKey .Values.telemetry.tracing.jaeger "tls" }}
{{- if .Values.telemetry.tracing.jaeger.tls.insecure }}
tls:
insecure: {{ .Values.telemetry.tracing.jaeger.tls.insecure }}
{{- end }}
{{- end }}
{{- end }}
{{- if and .Values.telemetry.metrics.enabled .Values.telemetry.metrics.port }}
prometheus:
endpoint: ":{{ .Values.telemetry.metrics.port }}"
{{- end }}
service:
pipelines:
{{- if and .Values.telemetry.metrics.enabled .Values.telemetry.metrics.port }}
metrics:
receivers: [otlp]
processors: []
exporters: [prometheus]
{{- end }}
{{- if and .Values.telemetry.tracing.enabled .Values.telemetry.tracing.jaeger.endpoint }}
traces:
receivers: [otlp]
processors: [batch]
exporters: [otlp/jaeger]
{{- end }}
{{- toYaml .Values.telemetry.otelSpec | nindent 2 }}
{{ end }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jvanz, please don't kill me... but today, while looking into a failure of the e2e tests against this PR we realized everything is a bit more complicated.

Assume this PR is merged. Inside of the values.yaml file we have what used to be the previous configuration of the collector. As a user I can do .Values.telemetry.enabled = true (we still have this key) and set .Values.telemetry.tracing.enabled = false. In this scenario the tracing configuration is still under .Values.telemetry.otelSpec unless the user changes it. As a result, the collector would be configured to enable metrics, but it would also have the tracing configuration provided by the default values (which would constantly complain while attempting to send the traces to a non-existing service).

@kravciak, @fabriziosestito and I discussed what to do. Our idea, which we can discuss further with the other maintainers, is to support two scenarios:

  • The user enables otel integration with the sidecar model. We take the old code, add a new boolean flag inside of the Values.yaml and add some new if statements here.
  • The user disables the otel integration done with the sidecar and creates on his own otel Collector configuration (maybe create the actual object on his own, without this helm chart?).

We have to do some brainstorming about that...

28 changes: 28 additions & 0 deletions charts/kubewarden-controller/tests/deployment_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
suite: Kubewarden controller deployment test
templates:
- deployment.yaml
release:
namespace: "kubewarden"
tests:
- it: "should not add the environment variable in the controller deployment when it is not set"
asserts:
- notExists:
path: spec.template.spec.containers[0].env
- it: "should not add the environment variable in the controller deployment when it is empty"
set:
env: []
asserts:
- notExists:
path: spec.template.spec.containers[0].env
- it: "should adds the environment variable in the controller deployment"
set:
env:
- name: KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT
value: "8080"
asserts:
- isSubset:
path: spec.template.spec.containers[0]
content:
env:
- name: KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT
value: "8080"
241 changes: 241 additions & 0 deletions charts/kubewarden-controller/tests/telemetry_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
suite: Telemetry tests
templates:
- opentelemetry-collector.yaml
- deployment.yaml
release:
name: "kubewarden-controller"
namespace: "kubewarden"
tests:
- it: "should adds cli flags in the controller deployment when tracing or metrics are enabled"
template: deployment.yaml
set:
telemetry:
tracing:
enabled: true
metrics:
enabled: true
asserts:
- equal:
path: spec.template.spec.containers[0].args[3]
value: --enable-metrics
- equal:
path: spec.template.spec.containers[0].args[4]
value: --enable-tracing
- it: "should not creates the OpenTelemetryCollector when telemetry is disabled"
template: opentelemetry-collector.yaml
set:
telemetry:
tracing:
enabled: false
metrics:
enabled: false
asserts:
- containsDocument:
kind: OpenTelemetryCollector
apiVersion: opentelemetry.io/v1beta1
name: kubewarden
namespace: kubewarden
not: true
- it: "should creates the OpenTelemetryCollector when metrics is enabled"
template: opentelemetry-collector.yaml
set:
telemetry:
tracing:
enabled: false
metrics:
enabled: true
asserts:
- containsDocument:
kind: OpenTelemetryCollector
apiVersion: opentelemetry.io/v1beta1
name: kubewarden
namespace: kubewarden
- it: "should creates the OpenTelemetryCollector when tracing is enabled"
template: opentelemetry-collector.yaml
set:
telemetry:
tracing:
enabled: true
metrics:
enabled: false
asserts:
- containsDocument:
kind: OpenTelemetryCollector
apiVersion: opentelemetry.io/v1beta1
name: kubewarden
namespace: kubewarden
- it: "should use the default OpenTelemetryCollector configuration when telemetry is enabled"
template: opentelemetry-collector.yaml
set:
telemetry:
metrics:
enabled: true
asserts:
- containsDocument:
kind: OpenTelemetryCollector
apiVersion: opentelemetry.io/v1beta1
name: kubewarden
namespace: kubewarden
- equal:
path: spec
value:
config:
connectors: {}
exporters:
otlp/jaeger:
endpoint: my-open-telemetry-collector.jaeger.svc.cluster.local:4317
tls:
insecure: true
prometheus:
endpoint: :8080
extensions: {}
processors:
batch: {}
receivers:
otlp:
protocols:
grpc: {}
service:
extensions: {}
pipelines:
metrics:
exporters:
- prometheus
processors: []
receivers:
- otlp
traces:
exporters:
- otlp/jaeger
processors:
- batch
receivers:
- otlp
envFrom: {}
mode: sidecar
- it: "should use the user defined OpenTelemetryCollector spec configuration"
template: opentelemetry-collector.yaml
set:
telemetry:
tracing:
enabled: true
otelSpec:
mode: sidecar
envFrom:
- secretRef:
name: open-telemetry-collector
config:
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]
asserts:
- containsDocument:
kind: OpenTelemetryCollector
apiVersion: opentelemetry.io/v1beta1
name: kubewarden
namespace: kubewarden
- equal:
path: spec
value:
config:
connectors: {}
exporters:
debug:
verbosity: normal
otlp/jaeger:
endpoint: my-open-telemetry-collector.jaeger.svc.cluster.local:4317
tls:
insecure: true
otlphttp/stackstate:
auth:
authenticator: bearertokenauth
endpoint: https://otlp-stackstate.oldfield.arch.nue2.suse.org:443
tls:
insecure_skip_verify: true
prometheus:
endpoint: :8080
extensions:
bearertokenauth:
scheme: SUSEObservability
token: ${env:API_KEY}
processors:
batch: {}
resource:
attributes:
- action: upsert
key: k8s.cluster.name
value: k3d-kubewarden
- action: insert
from_attribute: k8s.pod.uid
key: service.instance.id
receivers:
otlp:
protocols:
grpc: {}
service:
extensions:
- bearertokenauth
pipelines:
metrics:
exporters:
- prometheus
processors: []
receivers:
- otlp
metrics/stackstate:
exporters:
- debug
- otlphttp/stackstate
processors:
- resource
receivers:
- otlp
traces:
exporters:
- otlp/jaeger
processors:
- batch
receivers:
- otlp
traces/stackstate:
exporters:
- otlphttp/stackstate
processors:
- resource
receivers:
- otlp
envFrom:
- secretRef:
name: open-telemetry-collector
mode: sidecar
Loading
Loading