(feat): Add Sumo Logic scaler#6736
Conversation
7074035 to
f802504
Compare
f1222ad to
3a0e20b
Compare
e31fa20 to
b1d44bd
Compare
|
My feedback will focus on the possibility of using TypedConfig. This scaler does not use the new TypedConfig pattern available in the scalersconfig package. The metadata is manually parsed in parseSumoMetadata instead of declaratively with tags. Also there is a some of manual validation logic that can be handled automatically with TypedConfig. Furthermore, I would keep GetMetricsAndActivity cleaner like the other scalers, like this: Further, I am very curious to see how the Multi-Metrics Query Trigger is viewed by the community/maintainers (because of this discussion) |
|
Thanks for reviewing! I'll try out the things you mentioned above.
I see, it's a bit similar but the use-case is different. In Sumo Logic's case, that is how you write chained queries. Unlike prometheus where you can just let's say divide two metrics to calculate % utilisation or something, in Sumo Logic you have to define those metrics in separate queries and write another query which can reference those queries to do the chained calculation. The backend does all the calculation and we just pick result of the chained query via the input resultQueryRowID and ignore the rest. For example - Here, we are just interested in the result of |
Done! |
Great! Looks good! Just curious, in the |
|
You are right, it's not needed. Removed it. Thanks! |
|
/run-e2e sumologic |
|
/run-e2e sumologic |
|
/run-e2e sumologic |
There was a problem hiding this comment.
Pull Request Overview
Adds a new Sumo Logic scaler to KEDA for logs- and metrics-based autoscaling, including implementation, registration, and tests.
- Introduce Sumo Logic scaler under
pkg/scalersand register in the builder - Add unit tests for metadata parsing/metric spec and an e2e test
- Update test environment variables and document the new scaler in CHANGELOG
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/scalers/sumologic/sumologic_test.go | Add end-to-end test for Sumo Logic scaler |
| tests/.env | Add Sumo Logic environment variables |
| pkg/scaling/scalers_builder.go | Register the sumologic scaler |
| pkg/scalers/sumologic_scaler_test.go | Unit tests for metadata parsing and metric naming |
| pkg/scalers/sumologic_scaler.go | Core Sumo Logic scaler implementation |
| pkg/scalers/sumologic/types.go | HTTP client and type definitions for Sumo Logic API |
| pkg/scalers/sumologic/sumologic.go | API logic for log and metrics queries |
| pkg/scalers/sumologic/query.go | Query builder for Sumo Logic API |
| pkg/scalers/sumologic/metrics.go | Metrics query creation and response parsing |
| pkg/scalers/sumologic/logs.go | Logs search, paging, and processing |
| CHANGELOG.md | Document new Sumo Logic scaler |
Comments suppressed due to low confidence (4)
tests/scalers/sumologic/sumologic_test.go:200
- The assertion message says "after 3 minutes" but the timeout is set to 60 seconds; update the message or adjust the timeout to match.
"replica count should be %d after 3 minutes", maxReplicaCount)
tests/scalers/sumologic/sumologic_test.go:211
- The assertion message says "after 3 minutes" but the timeout is set to 60 seconds; either extend the timeout or correct the message.
"replica count should be %d after 3 minutes", minReplicaCount)
tests/scalers/sumologic/sumologic_test.go:216
- Use t.Skip or t.Skipf when SUMO_LOGIC_COLLECTOR_URL is not set to explicitly skip the data-push loop, preventing silent test passes with no data pushed.
t.Logf("No collector URL set, failed data push to Sumo Logic")
tests/scalers/sumologic/sumologic_test.go:161
- [nitpick] Add checks at the start of this test to verify SUMO_LOGIC_ACCESS_ID, SUMO_LOGIC_ACCESS_KEY, and SUMO_LOGIC_COLLECTOR_URL are set and call t.Skipf if any are missing to avoid misleading failures.
func TestSumologicScaler(t *testing.T) {
|
ptal @JorTurFer, @zroubalik |
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
Signed-off-by: mittalvaibhav1 <[email protected]>
|
/run-e2e sumologic |
|
@JorTurFer, @zroubalik, given this has approval from @rickbrouwer and myself, I am going to merge this so we have time to watch the e2es and debug until 2.18 release. In the worst case, we can always revert. |
|
It looks like the e2e test is failing on the main. Is that something that needs to be looked into? |
|
@rickbrouwer Ack, looking into it. |
|
Looks like the search job API isn't allowed for this free account type. Can we mute this E2E or revert the PR until I can sort this out internally? |
|
I think we can wait a little longer until you have figured it out. As @wozniakjan earlier said :
so, keep us updated :) |
|
Thank you! I should have a response by Monday. |
|
/run-e2e sumologic |
|
I missed this earlier, but looks like the sumologic e2e test even though it passed, it had issues even before, can you please take a look at that too @mittalvaibhav1? |
|
Yeah, also in the latest run I see those logs: |
|
It's a logical error when query results in no data. This should fix it. => #6906 (Currently in Draft mode, will try to finish it up by tomorrow.) Apologies for the inconvenience caused! |
* (feat): Add barebones for sumologic scaler Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): refactoring... Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): add client support for metric queries Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): process and return log search response as floats Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): fix tests Signed-off-by: mittalvaibhav1 <[email protected]> * Adding sumologic scaler file Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): refactoring sumologic scaler Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): adopt a logging frameworkin in sumologic package Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): fix tests.. Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): fix metrics queries Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): fixing metric name Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Adding resultfield and rollup for query result method Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): allow users to specify result field and rollup type Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): use cookies as recommended Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Updating test cases for sumologic_scaler Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Add support for multi metrics queries Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): fix tests Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): bug fixes Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): fix typo + readability improvements Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): pre-commit changes + add tests Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Fix PR checks Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Update changelog Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): PR checks Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): use TypedConfig pattern Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): remove unnecessary fields Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Use builder Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Address comments Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Fix e2e Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Address comments Signed-off-by: mittalvaibhav1 <[email protected]> * (feat): Trigger tests again Signed-off-by: mittalvaibhav1 <[email protected]> --------- Signed-off-by: mittalvaibhav1 <[email protected]> Co-authored-by: Anoop Bansal <[email protected]> Signed-off-by: David Pochopsky <[email protected]>
Add Sumo Logic scaler to scale based and logs and metrics from Sumo Logic
Checklist