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

'(' is not sanitized from datadog scalar #3789

Closed
tehlers320 opened this issue Oct 31, 2022 · 4 comments · Fixed by #3862
Closed

'(' is not sanitized from datadog scalar #3789

tehlers320 opened this issue Oct 31, 2022 · 4 comments · Fixed by #3862
Assignees
Labels
bug Something isn't working

Comments

@tehlers320
Copy link

Report

Not sure if this is a bug or working as intended but having "(" in kubernetes objects doesnt feel right.

metrics name:

s1-datadog-moving_rollup(sum-istio-mesh-request-count

Expected Behavior

metrics name:

s1-datadog-moving_rollup-sum-istio-mesh-request-count

Actual Behavior

metrics name:

s1-datadog-moving_rollup(sum-istio-mesh-request-count

Steps to Reproduce the Problem

  1. use query
moving_rollup(sum:istio.mesh.request.count{}.as_rate().rollup(sum, 30), 120, 'max')

Logs from KEDA operator

example

KEDA Version

2.8.1

Kubernetes Version

1.25

Platform

Any

Scaler Details

datadog

Anything else?

This is probably just a simple update to this function

// NormalizeString will replace all slashes, dots, colons and percent signs with dashes
func NormalizeString(s string) string {
	s = strings.ReplaceAll(s, "/", "-")
	s = strings.ReplaceAll(s, ".", "-")
	s = strings.ReplaceAll(s, ":", "-")
	s = strings.ReplaceAll(s, "%", "-")
	return s
}
@tehlers320 tehlers320 added the bug Something isn't working label Oct 31, 2022
@tomkerkhove tomkerkhove moved this to Proposed in Roadmap - KEDA Core Oct 31, 2022
@JorTurFer
Copy link
Member

Hey,
Thanks for reporting it, one question, does KEDA works properly? I mean, is it just a visual thing, or you are facing with problems with autoscaling? I mean, that function sanitize some characters that produce issues with url formats, but IDK if this is the case

@JorTurFer JorTurFer moved this from Proposed to Pending End-User Feedback in Roadmap - KEDA Core Nov 3, 2022
@tehlers320
Copy link
Author

@JorTurFer sorry i didnt see this, yes it does work perfectly fine. Its just very weird to have '(' in kubernetes object names.

@zroubalik
Copy link
Member

I agree, that we should sanitize this. @tehlers320 are you up for opening a PR?

@JorTurFer
Copy link
Member

I agree, that we should sanitize this. @tehlers320 are you up for opening a PR?

Yeah, +1

@JorTurFer JorTurFer self-assigned this Nov 15, 2022
Repository owner moved this from Pending End-User Feedback to Ready To Ship in Roadmap - KEDA Core Nov 16, 2022
@JorTurFer JorTurFer moved this from Ready To Ship to Done in Roadmap - KEDA Core Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants