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

Chart doesn't respect null jaeger/zipkin/prometheus default receivers when used as a subchart #1357

Closed
stonkie opened this issue Sep 25, 2024 · 7 comments
Labels
chart:collector Issue related to opentelemetry-collector helm chart question Further information is requested

Comments

@stonkie
Copy link

stonkie commented Sep 25, 2024

Hello,

The documentation says to set the default receivers to null to disable them. This doesn't work and still outputs the receiver configs, so a chizeled otel collector image that doesn't contain those receivers fails to load in the default charts.

Link to doc : https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-collector/README.md#basic-top-level-configuration

I would expect the jaeger entry to be absent from the ConfigMap here :

image

To show this way of configuring actually works :

image

To repro, drop this Chart.yaml file into a templates directory :

apiVersion: v2
name: opentelemetry-collector
description: Sets up an OpenTelemetry Collector
type: application
version: 1.0.0
appVersion: "0.77.0"
dependencies:
  - name: opentelemetry-collector
    repository: https://open-telemetry.github.io/opentelemetry-helm-charts
    version: 0.106.1

Then run those commands :

helm repo add opentelemetry-helm-charts https://open-telemetry.github.io/opentelemetry-helm-charts
helm dependency build
helm template test . --set "opentelemetry-collector.image.repository=test,opentelemetry-collector.mode=deployment,opentelemetry-collector.config.receivers.jaeger=null"
@stonkie
Copy link
Author

stonkie commented Sep 25, 2024

Update : just tested with 0.106.1 instead of 0.104. Same result

@TylerHelmuth
Copy link
Member

@stonkie are you using the opentelemetry-collector chart as a subchart?

@TylerHelmuth
Copy link
Member

Here is an example showing how null does work to remove configuration when the chart is not used as a subchart.

Also remember our chart only supports Helm 3.9+. Please check that your helm version is within the supported range.

@TylerHelmuth TylerHelmuth added question Further information is requested chart:collector Issue related to opentelemetry-collector helm chart labels Sep 26, 2024
@stonkie
Copy link
Author

stonkie commented Sep 26, 2024

Hi @TylerHelmuth,

Yeah I'm using it as a subchart named "opentelemetry-collector", hence the prefix to the variable names. We use this processor to lock versions and apply renovate monthly and to adjust some defaults and I figured it made for easy repro steps.

I do use a supported version of helm...

image

I also used the second screenshot to show the parameters were passed in properly.

But you're right, if I skip the subchart and use the chart directly, it applies properly.

image

I think there's something I misunderstand about null and how it merges into subchart values...

@stonkie
Copy link
Author

stonkie commented Sep 26, 2024

This only occurs with subchart. Still haven't figured out exactly why, but it isn't related to a bug in the charts themselves as I initially thought.

@stonkie stonkie closed this as completed Sep 26, 2024
@stonkie stonkie changed the title Chart doesn't respect null jaeger/zipkin/prometheus default receivers Chart doesn't respect null jaeger/zipkin/prometheus default receivers when used as a subchart Sep 26, 2024
@stonkie
Copy link
Author

stonkie commented Sep 26, 2024

Looks like this is what I'm hitting : helm/helm#12879

@TylerHelmuth
Copy link
Member

Helm has a bug where using null to remove content from a subchart does not work. It is extremely annoying.

You'll want to use alternateConfig instead. You still would be able to null values from alternate config, so you should set no values for alternateConfig in the default values.yaml, and then your user config should set opentelemetry-collector.alternateConfig.

See #1301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart:collector Issue related to opentelemetry-collector helm chart question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants