Skip to content

fix(fluent-bit): support additionalLabels in ServiceMonitor#691

Open
kalavt wants to merge 3 commits intofluent:mainfrom
kalavt:fix/servicemonitor-additionallabels
Open

fix(fluent-bit): support additionalLabels in ServiceMonitor#691
kalavt wants to merge 3 commits intofluent:mainfrom
kalavt:fix/servicemonitor-additionallabels

Conversation

@kalavt
Copy link

@kalavt kalavt commented Jan 28, 2026

PR: Rename serviceMonitor.selector to serviceMonitor.additionalLabels (Breaking Change)

Summary

This PR renames serviceMonitor.selector to serviceMonitor.additionalLabels to align with PrometheusRule's naming convention and eliminate confusion with ServiceMonitor's spec.selector field.

Problem Statement

Currently, the ServiceMonitor template uses .Values.serviceMonitor.selector for metadata labels, which:

  1. Is inconsistent with PrometheusRule which uses additionalLabels
  2. Creates confusion with spec.selector (the ServiceMonitor's pod selector field)
  3. Results in poor discoverability when Prometheus cannot find ServiceMonitors without proper labels

Example of the current confusing configuration:

serviceMonitor:
  enabled: true
  namespace: observability
  selector:  # Unclear - is this metadata labels or spec.selector?
    prometheus: kube-prometheus
    release: prometheus

Users often struggle to understand why their ServiceMonitor isn't being discovered by Prometheus, not realizing these are metadata labels, not the spec selector.

Solution

Replace serviceMonitor.selector with serviceMonitor.additionalLabels for clear semantics:

serviceMonitor:
  enabled: true
  namespace: observability
  additionalLabels:  # Clear - these are metadata labels
    prometheus: kube-prometheus
    release: prometheus

Why Breaking Change is Acceptable

This is a breaking change but acceptable because:

  1. Pre-release status: Chart version is v0.55.0 (v0.x series), where breaking changes are expected per semantic versioning
  2. Simple migration: Users only need to rename one parameter in their values.yaml
  3. Long-term benefits: No deprecated code to maintain, cleaner codebase
  4. Clear documentation: Breaking change is clearly documented in CHANGELOG

Benefits

  1. Consistent Naming - Aligns with PrometheusRule configuration pattern
  2. Clear Semantics - additionalLabels clearly indicates these are metadata labels, not pod selectors
  3. No Technical Debt - No deprecated parameters to maintain
  4. Better UX - Reduces confusion and improves discoverability

Changes

Files Modified

  1. templates/servicemonitor.yaml

    • Changed metadata labels from .Values.serviceMonitor.selector to .Values.serviceMonitor.additionalLabels
  2. values.yaml

    • Renamed selector parameter to additionalLabels in serviceMonitor section
    • Updated documentation comments
  3. CHANGELOG.md

    • Added breaking change entry under [UNRELEASED]

Migration Guide

Users need to update their values.yaml:

Before:

serviceMonitor:
  selector:
    prometheus: kube-prometheus
    release: prometheus

After:

serviceMonitor:
  additionalLabels:
    prometheus: kube-prometheus
    release: prometheus

Testing

# Test with additionalLabels
helm template test charts/fluent-bit \
  --set serviceMonitor.enabled=true \
  --set serviceMonitor.additionalLabels.prometheus=kube-prometheus \
  --set serviceMonitor.additionalLabels.release=prometheus

# Verify lint passes
helm lint charts/fluent-bit

# Verify documentation is updated
just docs

Checklist

  • Breaking change documented in CHANGELOG.md
  • values.yaml updated with new parameter name
  • servicemonitor.yaml template updated
  • helm-docs comments updated in values.yaml
  • Documentation regenerated with just docs
  • Markdown formatted with just fmt
  • Helm lint passes
  • Testing completed with new parameter
  • DCO sign-off included in commits

Add support for serviceMonitor.additionalLabels to align with
prometheusRule.additionalLabels pattern. This allows Prometheus
Operator to discover the ServiceMonitor using label selectors.

The existing selector field is maintained for backward compatibility,
with additionalLabels taking precedence when both are defined.

Fixes fluent#1234

Signed-off-by: Arbin <arbin.cheng@coins.ph>
Copy link
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @kalavt. Could you please read the CONTRIBUTING
guide as this PR is missing changelog entries.

Given that this chart is a pre-release we could remove serviceMonitor.selector as long as we make sure to record it correctly in the changelog.

- Renamed serviceMonitor.selector to serviceMonitor.additionalLabels for consistency with PrometheusRule
- Updated CHANGELOG.md with breaking change entry (fluent#691)
- Updated values.yaml and templates/servicemonitor.yaml
- Regenerated documentation with helm-docs
- Fixed markdown formatting with rumdl
- All tests verified: helm lint passes

This is a breaking change acceptable for v0.x chart versions.
Users need to rename 'selector' to 'additionalLabels' in their values.yaml.

Signed-off-by: Arbin <arbin.cheng@coins.ph>
@kalavt kalavt requested a review from stevehipwell January 29, 2026 04:35
@kalavt
Copy link
Author

kalavt commented Feb 27, 2026

Thanks for the PR @kalavt. Could you please read the CONTRIBUTING guide as this PR is missing changelog entries.

Given that this chart is a pre-release we could remove serviceMonitor.selector as long as we make sure to record it correctly in the changelog.

Updated PR description and removed serviceMonitor.selector

@stevehipwell
Copy link
Collaborator

@kalavt what exactly is the purpose of the this PR, given that the serviceMonitor.additionalLabels input is already supporte and has priority over the deprecated serviceMonitor.selector value?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants