Monitoring service ignores `.spec.config.service.telemetry` config for new metrics readers config

[at-ishikawa]

Component(s)

collector

What happened?

Description

The service for monitoring used to be created with a port number defined in .spec.config.service.telemetry.metrics.address.

> kubectl get opentelemetrycollectors.opentelemetry.io logs -o yaml | yq .spec.config.service.telemetry
metrics:
  address: 0.0.0.0:8889
> kubectl get svc logs-collector-monitoring -o yaml | yq .spec.ports
- name: monitoring
  port: 8889
  protocol: TCP
  targetPort: 8889

However, the service monitoring isn't created with the new metrics readers config, for example, .spec.config.service.telemetry.readers[].pull.exporter.prometheus.port.

> kubectl get opentelemetrycollectors.opentelemetry.io logs -o yaml | yq .spec.config.service.telemetry
metrics:
  level: detailed
  readers:
    - pull:
        exporter:
          prometheus:
            host: 0.0.0.0
            port: 8889
> kubectl get svc logs-collector-monitoring -o yaml
...
  ports:
  - name: monitoring
    port: 8888
    protocol: TCP
    targetPort: 8888

Thus, if the metrics port is set other than 8888, then a monitoring service no longer works correctly.

Steps to Reproduce

Define a port to expose an internal telemetry metrics other than 8888.

apiVersion: opentelemetry.io/v1beta1
kind: OpenTelemetryCollector
metadata:
  name: logs
spec:
  ...
  config:
    service:
      telemetry:
        metrics:
          level: detailed
          readers:
            - pull:
                exporter:
                  prometheus:
                    host: '0.0.0.0'
                    port: 8889

Then deploy the manifest and see a service created with monitoring.

Expected Result

The monitoring service is created with a port number defined in the config.

Actual Result

The monitoring service is created with a port number 8888.

Kubernetes Version

v1.32.3

Operator version

v0.126.0

Collector version

v0.126.0

Environment information

Environment

Operator is deployed by a helm chart (https://open-telemetry.github.io/opentelemetry-helm-charts, opentelemetry-operator: v0.90.0

Log output

Additional context

helm chart values is nothing to do with this bug. But I just add here for reference

manager:
  collectorImage:
    repository: /path/to/otel-collector
  resources:
    limits:
      memory: 1Gi
    requests:
      cpu: 100m
      memory: 512Mi

[Kidsan]

Can confirm the same issue in v0.122.0. Any kind of configuration provided for .spec.config.service.telemetry.readers is not applied, and the collector is created with the default prometheus config.

[iblancasa]

Can confirm the same issue in v0.122.0. Any kind of configuration provided for .spec.config.service.telemetry.readers is not applied, and the collector is created with the default prometheus config.

That is not the same issue. It is different and was fixed #3874

[Kidsan]

Can confirm the same issue in v0.122.0. Any kind of configuration provided for .spec.config.service.telemetry.readers is not applied, and the collector is created with the default prometheus config.

That is not the same issue. It is different and was fixed #3874

Ah, thank you and sorry for the noise on this issue then.

[alorenzo175]

Perhaps a slightly different issue, but the same port mismatch is present in the containerPort exposed on the Pod. With telemetry config like

      telemetry:
        metrics:
          level: detailed
          readers:
            - pull:
                exporter:
                  prometheus:
                    host: '0.0.0.0'
                    port: 8889

The metrics port is set like

        name: otc-container
        ports:
        - containerPort: 8888
          name: metrics
          protocol: TCP

The same pod config is present when adding the address and keeping the readers

      telemetry:
        metrics:
          address: 0.0.0.0:8889
          level: detailed
          readers:
            - pull:
                exporter:
                  prometheus:
                    host: '0.0.0.0'
                    port: 8889

However, removing the readers shows the correct behavior for the metrics port:

      telemetry:
        metrics:
          address: 0.0.0.0:8889
          level: detailed
          readers: []

        name: otc-container
        ports:
        - containerPort: 8889
          name: metrics
          protocol: TCP

[purkhusid]

Also encountering this issue. This breaks users that have more than one Daemonset of OTEL collectors running since they all try to use port 8888 as a host port. The operator does not take .service.telemetry.metrics.readers[x].pull.exporter.prometheus.port into account when choosing the metrics port.

[iblancasa]

Also encountering this issue. This breaks users that have more than one Daemonset of OTEL collectors running since they all try to use port 8888 as a host port. The operator does not take .service.telemetry.metrics.readers[x].pull.exporter.prometheus.port into account when choosing the metrics port.

What operator version are you using?

[purkhusid]

@iblancasa I'm using 0.129.1. I can see that there is a newer version available. The release notes do no mention anything about this issue though. I can try out the latest version later today if you think it might have a fix for this.

[tstirrat15]

We're seeing a similar issue with 0.133.0. We had manually specified the default in our config, and the operator is ignoring it and adding a second entry. I think the behavior we see is slightly different than other users because we're creating the CR with SSA + a field manager, so we get two entries instead of the behavior where the configuration is getting whacked.

Is it possible that GetTelemetry is returning nil when it shouldn't?

[shuheiktgw]

Let me work on this one 🙏