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

make it possuble to just supply a full config #1160

Closed
trondhindenes opened this issue Apr 30, 2024 · 7 comments · Fixed by #1301
Closed

make it possuble to just supply a full config #1160

trondhindenes opened this issue Apr 30, 2024 · 7 comments · Fixed by #1301
Labels
chart:collector Issue related to opentelemetry-collector helm chart question Further information is requested

Comments

@trondhindenes
Copy link

I'm finding it incredibly difficult to set up a custom collector config, because I need to disable the things I don't want. I can't just supply port values if I need non-standard ones either.

Please consider making the config easier by providing an option to just supply a full config and list of ports

@JaredTan95
Copy link
Member

JaredTan95 commented May 1, 2024

have you tried --set configMap.create=false --set configMap.existingName=your-own-configmap-for-collector?

@JaredTan95 JaredTan95 added question Further information is requested chart:collector Issue related to opentelemetry-collector helm chart labels May 1, 2024
@TylerHelmuth
Copy link
Member

You should be able to do

ports:
  custom:
    enabled: true
    containerPort: 9999
    servicePort: 9999
    hostPort: 9999
    protocol: TCP

to add custom ports to both the container and service. ports is a map so it will merge cleanly.

@TylerHelmuth
Copy link
Member

Can you be more specific about what is difficult to configure? You can remove default components using helm's null feature such as

config:
  receivers:
    jaeger: null
    prometheus: null
    zipkin: null
  service:
    pipelines:
      traces:
        receivers:
          - otlp
      metrics: null
      logs: null

Full example: https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-collector/examples/deployment-otlp-traces/values.yaml

@boxidau
Copy link

boxidau commented May 21, 2024

+1 on this issue. Having to null out the default config requires merging the nulled structure into our config which seems a little messy. Changes to the default config would then be breaking changes on chart updates.

Having a second alternative key customConfig (or similar) in values where a user can define their entire config would be much easier to deal with in cases where a user has an already well formed config they'd like to use.

The other option of an externally managed ConfigMap is okay and gives the flexibility but loses the auto-reload functionality on config updates

@lerminou
Copy link

lerminou commented Jul 12, 2024

@TylerHelmuth , it seems to not work with null inside the config directly, when using a subchart:
https://helm.sh/docs/chart_template_guide/values_files/#deleting-a-default-key
helm/helm#11440
helm/helm#11617
helm/helm#12637

If I try on my own, following the 'otel-trace only' https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector/examples/deployment-otlp-traces

I tried:

opentelemetry-collector:
  config:
    receivers:
      jaeger: null
      prometheus: null
      zipkin: null
    service:
      pipelines:
        traces:
          receivers:
            - otlp
        metrics: null
        logs: null

or more specific:

opentelemetry-collector:
  config:
    service:
      pipelines:
        traces:
          exporters:
            - otlp
          receivers:
            - otlp
            - zipkin
        metrics:
          exporters: {} // tried null or [] or {}
          processors: {}
          receivers: {}
        logs:
          exporters: {}
          processors: {}
          receivers: {}

in each case I have an error during the helm install:

coalesce.go:286: warning: cannot overwrite table with non table for otel-collector.opentelemetry-collector.config.service.pipelines.metrics (map[exporters:[debug] processors:[memory_limiter batch] receivers:[otlp prometheus]])
coalesce.go:286: warning: cannot overwrite table with non table for otel-collector.opentelemetry-collector.config.service.pipelines.metrics (map[exporters:[debug] processors:[memory_limiter batch] receivers:[otlp prometheus]])
coalesce.go:286: warning: cannot overwrite table with non table for otel-collector.opentelemetry-collector.config.service.pipelines.metrics (map[exporters:[debug] processors:[memory_limiter batch] receivers:[otlp prometheus]])

And the generated configmap in my cluster has all the keys.

Helm 3.13 or 3.15
so yeah, it's difficult to use

@TylerHelmuth
Copy link
Member

I really really don't want to have to do anything special in the chart when this is truly a helm bug. But, considering this bug still isn't fixed, I'm willing to discuss work around solutions.

@TylerHelmuth
Copy link
Member

Things I'd like to see in a solution:

  1. non-breaking change.
  2. Maybe still works with presets

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
5 participants