Skip to content

Selenium Grid: Update metric name generated without part of empty#6772

Merged
JorTurFer merged 2 commits into
kedacore:mainfrom
NDViet:selenium-grid-20250512
May 12, 2025
Merged

Selenium Grid: Update metric name generated without part of empty#6772
JorTurFer merged 2 commits into
kedacore:mainfrom
NDViet:selenium-grid-20250512

Conversation

@VietND96

@VietND96 VietND96 commented May 12, 2025

Copy link
Copy Markdown
Contributor

Update the metric name generated from Grid scaler, since it is combined from trigger param browserName, browserVersion, and platformName. However, version and platform are optional and might be empty.
Sometimes the name shown in the log was selenium-grid-chrome--, which might cause confusion.
With this change, this helps make the metric name meaningful, only appending non-empty values.

Checklist

Fixes #

Relates to #

@VietND96
VietND96 requested a review from a team as a code owner May 12, 2025 17:50
Signed-off-by: Viet Nguyen Duc <[email protected]>
@VietND96 VietND96 mentioned this pull request May 12, 2025
18 tasks
@JorTurFer

JorTurFer commented May 12, 2025

Copy link
Copy Markdown
Member

/run-e2e selenium
Update: You can check the progress here

@JorTurFer
JorTurFer merged commit 505f97a into kedacore:main May 12, 2025
@VietND96
VietND96 deleted the selenium-grid-20250512 branch May 13, 2025 02:13
@rickbrouwer

Copy link
Copy Markdown
Member

Perhaps this could happen more often with other triggers as well. Shouldn't we have solved this in the pkg/util/normalize_string.go so that other triggers could benefit from it as well?

We could have added something to that feature like:

// Replace multiple dashes with a single dash
re := regexp.MustCompile(`-+`)
s = re.ReplaceAllString(s, "-")

// Remove trailing dash at end if present
s = strings.Trim(s, "-")

@wozniakjan

Copy link
Copy Markdown
Member

good point @rickbrouwer, for the 2.17.1 release I will cherry-pick this particular version but for 2.18.0 we can reconsider and address this in a more general way as you outlined here

@rickbrouwer

rickbrouwer commented May 15, 2025

Copy link
Copy Markdown
Member

@wozniakjan

It's also a bit of a question for me whether this PR should be included with 2.17.1 since it's an improvement, not a fix. Improvements (such as performance tweaks or small updates) are not officially bug fixes in my opinion and therefore belong in a minor release.

wozniakjan pushed a commit to wozniakjan/keda that referenced this pull request May 15, 2025
…dacore#6772)

* Selenium Grid: Update metric name generated without part of empty

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update CHANGELOG with the PR

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
wozniakjan pushed a commit to wozniakjan/keda that referenced this pull request May 15, 2025
…dacore#6772)

* Selenium Grid: Update metric name generated without part of empty

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update CHANGELOG with the PR

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
wozniakjan added a commit that referenced this pull request May 15, 2025
* fix: Admission Webhook blocks ScaledObject without metricType with fallback (#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <[email protected]>

* Add unit test

Signed-off-by: Rick Brouwer <[email protected]>

* Add e2e test

Signed-off-by: rickbrouwer <[email protected]>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <[email protected]>

* Update changelog

Signed-off-by: Rick Brouwer <[email protected]>

* Update

Signed-off-by: Rick Brouwer <[email protected]>

---------

Signed-off-by: rickbrouwer <[email protected]>
Signed-off-by: Rick Brouwer <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>

* fix: AWS SQS Queue queueURLFromEnv not working (#6713)

Signed-off-by: rickbrouwer <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>

* fix: Temporal scaler with API Key (#6707)

Signed-off-by: Rick Brouwer <[email protected]>
Signed-off-by: rickbrouwer <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>

* fix: add default Operation in Azure Service Bus scaler (#6731)

Signed-off-by: Rick Brouwer <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>

* fix: ScalerCache gets the lock before operate the scalers (#6739)

Signed-off-by: Jan Wozniak <[email protected]>

* fix: Use pinned version for nginx image (#6737)

* fix: Use pinned version for nginx image

Signed-off-by: Jorge Turrado <[email protected]>

* .

Signed-off-by: Jorge Turrado <[email protected]>

* fix panic in gcp scaler

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>

* Selenium Grid: Update metric name generated without part of empty (#6772)

* Selenium Grid: Update metric name generated without part of empty

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update CHANGELOG with the PR

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>

* chore: changelog and issue template v2.17.1

Signed-off-by: Jan Wozniak <[email protected]>

---------

Signed-off-by: rickbrouwer <[email protected]>
Signed-off-by: Rick Brouwer <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Viet Nguyen Duc <[email protected]>
Co-authored-by: rickbrouwer <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Viet Nguyen Duc <[email protected]>
dpochopsky pushed a commit to dpochopsky/keda that referenced this pull request Sep 12, 2025
…dacore#6772)

* Selenium Grid: Update metric name generated without part of empty

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update CHANGELOG with the PR

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
Signed-off-by: David Pochopsky <[email protected]>
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.

4 participants