search

open-telemetry / opentelemetry-helm-charts

OpenTelemetry Helm Charts
https://opentelemetry.io
Apache License 2.0
390 stars 479 forks source link

[demo] Remove release name prefix from resources #1392

Open puckpuck opened 4 days ago

puckpuck commented 4 days ago

Currently all Kubernetes resources created by the demo will have a name that is prefixed by the demo's release name. This pattern is typical for many, but not all Helm charts.

The demo is a rather large Helm chart in terms of the # of resources that it installs. Currently the demo will create:

All of these resources except the OpenSearch stateful set and related ones are installed with the Helm chart release name as a prefix. This causes several challenges when using the demo for its intended purpose, which is to demonstrate OpenTelemetry technologies.

When consumed in observability tooling, the long prefix on names for the pods causes clipping on charts and visuals associated with that resource. Deploying and validating the demo through bastion hosts or limited terminal interfaces can lead to line wrapping and a poor experience for the user. Additional complexity/tech debt is also associated with supporting the release name prefix for all demo components and subchart dependencies required by the demo.

I propose to remove the release name prefix from the names of all Kubernetes resources created by the demo.

To note, the Demo can only be installed a single time per namespace due to the variety of subcharts and RBAC dependencies.

Screenshot 2024-10-18 at 12 21 58 AM

julianocosta89 commented 4 days ago

I like the idea and have nothing against it. 🥳

TylerHelmuth commented 3 days ago

Implementing this as an option seems reasonable.

puckpuck commented 22 hours ago

Implementing this as an option seems reasonable.

A tenet of this is to remove much of the code where we have to use http://{{ include "otel-demo.name" . }} and require the Helm chart to use tpl ... to render it. We do this to ensure the name prefix is used for all network communications. This is mostly a challenge for sub-charts, like the Prometheus scrape config and OpenSearch name override, which require special casing since those sub-charts don't implement tpl ... for all chart properties. Each time we add a new component to the demo Helm chart, we have to be mindful of this and accommodate for it.

Mind you, removing this tech debt isn't the only reason for wanting to do this change, but doing this as an option would increase our tech debt.