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.

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

[demo] Remove release name prefix from resources #1392

Open
puckpuck opened this issue Oct 18, 2024 · 3 comments
Open

[demo] Remove release name prefix from resources #1392

puckpuck opened this issue Oct 18, 2024 · 3 comments
Labels
chart:demo Issues related to opentelemetry-demo helm chart

Comments

@puckpuck
Copy link
Contributor

puckpuck commented Oct 18, 2024

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:

  • 1 Statefulset
  • 23 Deployments
  • 24 Pods
  • 25 Services

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
Copy link
Member

I like the idea and have nothing against it. 🥳

@TylerHelmuth TylerHelmuth added the chart:demo Issues related to opentelemetry-demo helm chart label Oct 18, 2024
@TylerHelmuth
Copy link
Member

Implementing this as an option seems reasonable.

@puckpuck
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart:demo Issues related to opentelemetry-demo helm chart
Projects
None yet
Development

No branches or pull requests

3 participants