Collector Instances Not Discovered Due to Case Sensitivity in matchLabels

[nicolastakashi]

Component(s)

target allocator

What happened?

Description

While running TA without using the OTel Operator, I spent some time trying to understand why the TA was not discovering my collector instances and just figured out that I was using this config file:

allocation_strategy: consistent-hashing
filter_strategy: relabel-config
collector_selector:
  matchLabels:
    app.kubernetes.io/instance: otel-integration
    app.kubernetes.io/name: opentelemetry-collector
prometheus_cr:
  enabled: true
  scrape_interval: 30s
  pod_monitor_selector: {}
  service_monitor_selector: {}

Highlight the matchLabels key. This is using camel case, which is the convention while working with Kubernetes resources, but TA’s unmarshal method is only accepting lower case.

Even though TA is using theLabelSelector under the k8s.io/apimachinery/pkg/apis/meta/v1 package, this is not working because this package doesn’t contain any annotations to YAML marshal.

Initially, I was thinking about converting the whole config into JSON and using the YAMLToJSON method available in the github.com/ghodss/yaml package, but after doing that I noticed the configs related to Prometheus from the github.com/prometheus/prometheus/config package stopped working, since they don’t support any JSON annotations for marshalling.

I had another idea in mind! Keep using YAML for marshalling and redefine theLabelMatcher struct inside the TA project.

I’d like to get your opinion on that and check if you folks see any other solution. Maybe ask Prometheus maintainers to add JSON annotations to the Config struct? Not sure. Looking forward to your feedback.

Steps to Reproduce

Start the TA using the following config

allocation_strategy: consistent-hashing
filter_strategy: relabel-config
collector_selector:
  matchLabels:
    app.kubernetes.io/instance: otel-integration
    app.kubernetes.io/name: opentelemetry-collector
prometheus_cr:
  enabled: true
  scrape_interval: 30s
  pod_monitor_selector: {}
  service_monitor_selector: {}

Expected Result

Be compatible with K8S semantics

Actual Result

Not compatibility with K8S semantics

Kubernetes Version

NA

Operator version

NA

Collector version

NA

Environment information

Environment

NA

Log output

No response

Additional context

No response

[davidhaja]

I would like to work on this

[swiatekm]

It would be great if we could figure out a way to do this without redefining all the K8s and Prometheus structs ourselves. I originally did it this way because it was simple and standalone TA wasn't a very popular thing to do. @davidhaja feel free to take a stab at it.

[nicolastakashi]

@swiatekm Maybe if we implement a custon unmarshal function this could work, wdyt?

[swiatekm]

Maybe, I'm honestly not that familiar with the unmarshalling magic for these structs. I'm hoping that someone more knowledgeable can make it work, or at least try different approaches to see what conclusively doesn't work.

[davidhaja]

The way I see is either we try to support the flexible/case insensitive unmarshalling in TA, or we change both the marshalling and unmarshalling to follow (and require) the k8s camel case convention.
The latter one is a breaking change, which I believe requires more effort. (Although probably results in cleaner logic, code as well)
In the submitted PR I've made some changes that implement the former option.

I've tried multiple yaml packages, but none of them supports case insensitive parsing of YAML files.

• github.com/ghodss/yaml: A wrapper around go-yaml. This library first converts YAML to JSON using go-yaml and then uses json.Marshal and json.Unmarshal to convert to or from the struct. -> This doesn't work with the prometheus config, since they don’t support any JSON annotations for marshalling.
• kubernetes-sigs/yaml: A permanent fork of ghodss/yaml.
• gopkg.in/yaml.v3: v3 version of the currently used YAML library. Last commit is 2 years old. There was already a request to support the case insensitive unmarshalling, but it got rejected (Feature request: case-insensitive keys when Unmarshalling ? go-yaml/yaml#123).

I ended up with github.com/goccy/go-yaml because of its feature set. I've described the feature that my change is using for providing case insensitive parsing.