Skip to content

Feat: Add support for params field in Probe#7755

Merged
simonpasquier merged 18 commits intoprometheus-operator:mainfrom
nutmos:feat/probe-add-params
Aug 5, 2025
Merged

Feat: Add support for params field in Probe#7755
simonpasquier merged 18 commits intoprometheus-operator:mainfrom
nutmos:feat/probe-add-params

Conversation

@nutmos
Copy link
Contributor

@nutmos nutmos commented Jul 30, 2025

Description

Add support for params fields in the Probe CRD.

Closes: #7541

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

  • CHANGE (fix or feature that would cause existing functionality to not work as expected)
  • FEATURE (non-breaking change which adds functionality)
  • BUGFIX (non-breaking change which fixes an issue)
  • ENHANCEMENT (non-breaking change which improves existing functionality)
  • NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Verification

Using the unit tests

Changelog entry

- Update Probe CRD to support Params fields in Prometheus

paramsMapSlice = append(paramsMapSlice, yaml.MapItem{Key: "module", Value: []string{m.Spec.Module}})
}

if len(m.Spec.Params) != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) we don't need to check the length.


if len(m.Spec.Params) != 0 {
for k, v := range m.Spec.Params {
paramsMapSlice = append(paramsMapSlice, yaml.MapItem{Key: k, Value: v})
Copy link
Contributor

Choose a reason for hiding this comment

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

we should prevent conflicts with the module field. E.g. can we skip the parameter if the key is equal to module?

// The parameters for the scrape
// +optional
// +kubebuilder:validation:MinProperties=1
Params map[string][]string `json:"params,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not aligned with the Kubernetes API conventions which recommend named subobjects over maps:

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#lists-of-named-subobjects-preferred-over-maps

I know that we use map[string][]string everywhere else but I'd rather follow the Kubernetes recommendation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 31, 2025
@nutmos
Copy link
Contributor Author

nutmos commented Jul 31, 2025

still pending test cases

@nutmos nutmos marked this pull request as ready for review August 3, 2025 02:33
@nutmos nutmos requested a review from a team as a code owner August 3, 2025 02:33
@nutmos nutmos requested a review from simonpasquier August 3, 2025 02:33
@nutmos
Copy link
Contributor Author

nutmos commented Aug 3, 2025

@simonpasquier All done. Please review

Copy link
Contributor Author

@nutmos nutmos left a comment

Choose a reason for hiding this comment

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

Update validation and description as recommended.

simonpasquier
simonpasquier previously approved these changes Aug 4, 2025
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

Thanks!

@nutmos
Copy link
Contributor Author

nutmos commented Aug 5, 2025

@simonpasquier can you help review it again? I updated the Name field to be required.

@simonpasquier simonpasquier merged commit 38ff903 into prometheus-operator:main Aug 5, 2025
23 checks passed
miinsun pushed a commit to miinsun/prometheus-operator that referenced this pull request Aug 15, 2025
…d-params

Feat: Add support for params field in Probe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prober support more exporters

2 participants