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

Remove deprecated prometheus metrics from MS #432

Merged
merged 5 commits into from
Jun 6, 2023

Conversation

JorTurFer
Copy link
Member

@JorTurFer JorTurFer commented Apr 24, 2023

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable)
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Related to kedacore/keda#3930

@EppO
Copy link

EppO commented May 11, 2023

keda-operator is also exposing metrics on default port 8080 like keda_scaler_, keda_scaled_objects_, keda_resource_* or keda_trigger_*. There are no annotations for that currently, we would need to add:

    prometheus.io/scrape: "true"
    prometheus.io/port: {{ .Values.prometheus.operator.port | quote }}
    prometheus.io/path: "/metrics"

in https://github.com/kedacore/charts/tree/main/keda/templates/manager/deployment.yaml
I also noticed that https://github.com/kedacore/charts/blob/main/keda/templates/manager/deployment.yaml#LL101C32-L101C32 is static, so if you change the value of prometheus.operator.port in the values.yaml, it may not work

@JorTurFer
Copy link
Member Author

JorTurFer commented May 17, 2023

nice catches!
Thanks for the feedback ❤️
I have to redo this PR soon, so I'll address them 😄

@zroubalik
Copy link
Member

@JorTurFer could you please rebase this PR?

@JorTurFer
Copy link
Member Author

@JorTurFer could you please rebase this PR?

Yes, I'll do it during this week

@JorTurFer
Copy link
Member Author

I still got this pending but I'm out of the office :(

@JorTurFer JorTurFer force-pushed the remove-deprecated-ms branch from a8ef126 to a49d7d0 Compare May 26, 2023 17:20
@JorTurFer
Copy link
Member Author

@JorTurFer could you please rebase this PR?

done!

@JorTurFer JorTurFer changed the title Breaking change: Remove deprecated prometheus metrics from MS Remove deprecated prometheus metrics from MS May 26, 2023
@JorTurFer JorTurFer mentioned this pull request Jun 6, 2023
2 tasks
JorTurFer and others added 4 commits June 6, 2023 09:25
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
@JorTurFer JorTurFer force-pushed the remove-deprecated-ms branch from e8dd757 to a3b97a7 Compare June 6, 2023 07:25
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
@JorTurFer
Copy link
Member Author

JorTurFer commented Jun 6, 2023

PTAL @zroubalik , we need to merge this because the chart is "broken" until it due to the parameters change in MS

@zroubalik zroubalik merged commit 502980f into kedacore:main Jun 6, 2023
@JorTurFer JorTurFer deleted the remove-deprecated-ms branch June 6, 2023 08:55
@emctl emctl mentioned this pull request Jul 8, 2023
4 tasks
@outime
Copy link

outime commented Jul 21, 2023

@JorTurFer @zroubalik @tomkerkhove my apologies for the multi-mention but I've just hit this and I was wondering if you're willing to push up a revision of the current KEDA chart to contain this and few other commits?

@JorTurFer
Copy link
Member Author

@JorTurFer @zroubalik @tomkerkhove my apologies for the multi-mention but I've just hit this and I was wondering if you're willing to push up a revision of the current KEDA chart to contain this and few other commits?

No worries for the mention 😄
We plan to cut a release next week: kedacore/keda#4795

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.

5 participants