add opentelemetry-ebpf-instrumentation helm chart #1704
Conversation
Hey @TylerHelmuth, just added a comment in the issue. Hope it clarifies everything. |
|
@mariomac iv'e added to k8s cache to the helm chart, so i would love a re-review |
- Integrate the chart with the Makefile for consistent tooling support. - Add examples and render via `make generate-examples'.
NimrodAvni78
force-pushed
the
otel-ebpf-instrumentation
branch
from
9a6914d to
f91fcfa
Compare
|
@TylerHelmuth i see that mario has approved |
TylerHelmuth
dismissed
their stale review
I still need to do a real review but wont have time till July. But I dont want to block anymore.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
hey @TylerHelmuth, are you available for a review of this? |
There was a problem hiding this comment.
Can you update the github workflows so that this chart is tested? chart-testing will do a default install if there is no ci folder
There was a problem hiding this comment.
Please add a section detailing requirements
| - https://github.com/open-telemetry/opentelemetry-helm-charts | ||
| icon: https://opentelemetry.io/img/logos/opentelemetry-logo-nav.png | ||
| maintainers: | ||
| - name: nimrodavni78 |
There was a problem hiding this comment.
| - name: nimrodavni78 | |
| - name: dmitryax | |
| - name: jaronoff97 | |
| - name: TylerHelmuth | |
| - name: nimrodavni78 |
| @@ -0,0 +1,14 @@ | |||
| --- | |||
| apiVersion: v2 | |||
| name: opentelemetry-ebpf-instrumentation | |||
There was a problem hiding this comment.
@open-telemetry/helm-approvers this name is a bit long, do you think we could shorten it to opentelemetry-ebpf-instr?
| {{/* | ||
| Expand the name of the chart. | ||
| */}} | ||
| {{- define "obi.name" -}} |
There was a problem hiding this comment.
Please prefix all the templates with the chart's name instead of obi
There was a problem hiding this comment.
rename file to daemonset.yaml please.
There was a problem hiding this comment.
Please rename to clusterrole.yaml
There was a problem hiding this comment.
please rename to clusterrolebinding.yaml
| ebpf-instrument-config.yml: | | ||
| {{- if eq .Values.preset "network" }} | ||
| {{- if not .Values.config.data.network }} | ||
| network: | ||
| enable: true | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- if eq .Values.preset "application" }} | ||
| {{- if not .Values.config.data.discovery }} | ||
| discovery: | ||
| services: | ||
| - k8s_namespace: . | ||
| exclude_services: | ||
| - exe_path: ".*ebpf-instrument.*|.*otelcol.*" | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- toYaml .Values.config.data | nindent 4}} | ||
| {{- end }} |
There was a problem hiding this comment.
Lets move the configmap logic into a helper template like the other charts do
| global: | ||
| image: | ||
| # -- Global image registry to use if it needs to be overridden for some specific use cases (e.g local registries, custom images, ...) | ||
| registry: "" | ||
| # -- Optional set of global image pull secrets. | ||
| pullSecrets: [] |
There was a problem hiding this comment.
Why do we need this and .Values.image?
Helm chart for opentelemetry-ebpf-instrumentation, the helm chart is based on the original beyla helm chart
issue