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

fix: Replace $namepsace by $exported_namespace in keda-dashboard #4539

Merged
merged 1 commit into from
May 31, 2023

Conversation

ganievs
Copy link
Contributor

@ganievs ganievs commented May 15, 2023

When The operator and s scaled objects are deployed in different namespaces, some metrics are not shown due to namespace labels conflict.

Screenshot 2023-05-15 at 14 25 51

Checklist

@ganievs ganievs requested a review from a team as a code owner May 15, 2023 10:32
@JorTurFer
Copy link
Member

JorTurFer commented May 15, 2023

This was solved in the chart directly: kedacore/charts@1a56c48
The problem was the serviceMonitor didn't honor namespaces, and that's why see the mismatch between them (and the extra label). If you add the flag to the serviceMonitor/podMonitor, is the problem solved?

@zroubalik
Copy link
Member

I think we should also fix this here in the core repo, not just charts.

@JorTurFer
Copy link
Member

In that case, we should rever the change in the helm repo. It exports these metrics, if we change them here, we should change the helm chart too

@zroubalik
Copy link
Member

In that case, we should rever the change in the helm repo. It exports these metrics, if we change them here, we should change the helm chart too

@ganievs are you willing to do this?

@ganievs
Copy link
Contributor Author

ganievs commented May 18, 2023

In that case, we should rever the change in the helm repo. It exports these metrics, if we change them here, we should change the helm chart too

@ganievs are you willing to do this?

Ok, will do that!

@ganievs
Copy link
Contributor Author

ganievs commented May 18, 2023

A PR with changes in the chart repo: kedacore/charts#449

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Signed-off-by: Shamil Ganiev <ganiev@pm.me>
@ganievs ganievs force-pushed the fix-grafana-dashboard branch from ccc5fdf to 6923a4f Compare May 18, 2023 13:41
@ganievs
Copy link
Contributor Author

ganievs commented May 18, 2023

Signed

@zroubalik
Copy link
Member

A PR with changes in the chart repo: kedacore/charts#449

@JorTurFer please confirm the chart PR is correct.

@tomkerkhove
Copy link
Member

I'm not sure I get why honorLabels needs to be removed though?

@ganievs
Copy link
Contributor Author

ganievs commented May 22, 2023

Hi folks! In my opinion, it will be better when the dashboard will follow a default Prometheus labeling behavior because it can be used not only with the initial Keda's helm chart or requirement to enable the honor labels should be mentioned in a documentation.

@JorTurFer
Copy link
Member

I agree with the approach of using defaults.
I have already reviewed the chart changes, there is some feedback to be done there

@JorTurFer JorTurFer merged commit 42849ae into kedacore:main May 31, 2023
@ganievs ganievs deleted the fix-grafana-dashboard branch May 31, 2023 12:16
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.

4 participants