Skip to content

(feat): Add Sumo Logic scaler#6736

Merged
wozniakjan merged 36 commits into
kedacore:mainfrom
mittalvaibhav1:main
Jul 10, 2025
Merged

(feat): Add Sumo Logic scaler#6736
wozniakjan merged 36 commits into
kedacore:mainfrom
mittalvaibhav1:main

Conversation

@mittalvaibhav1

@mittalvaibhav1 mittalvaibhav1 commented Apr 26, 2025

Copy link
Copy Markdown
Contributor

Add Sumo Logic scaler to scale based and logs and metrics from Sumo Logic

  1. Issue: Support Sumo Logic scaler #6734
  2. Docs PR: (feat): Add sumologic scaler documentation keda-docs#1566

Checklist

Comment thread tests/scalers/sumologic/sumologic_test.go Outdated
@mittalvaibhav1
mittalvaibhav1 force-pushed the main branch 3 times, most recently from e31fa20 to b1d44bd Compare April 28, 2025 17:44
@rickbrouwer

rickbrouwer commented Apr 30, 2025

Copy link
Copy Markdown
Member

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.
You can also store the defaults here, like:

	// Connection settings
	Host      string `keda:"name=host,      order=triggerMetadata"`
	UnsafeSsl bool   `keda:"name=unsafeSsl, order=triggerMetadata, optional"`

	// Authentication
	AccessID  string `keda:"name=access_id,  order=authParams"`
	AccessKey string `keda:"name=access_key, order=authParams"`

	// Query configuration
	QueryType        string            `keda:"name=queryType,        order=triggerMetadata, enum=logs;metrics"`
	Query            string            `keda:"name=query,            order=triggerMetadata, optional"`
	Queries          map[string]string `keda:"name=query.*,          order=triggerMetadata, optional"`
	ResultQueryRowID string            `keda:"name=resultQueryRowID, order=triggerMetadata, optional"`
	Quantization     time.Duration     `keda:"name=quantization,     order=triggerMetadata, optional"`
	Rollup           string            `keda:"name=rollup,           order=triggerMetadata, default=avg"`
	ResultField      string            `keda:"name=resultField,      order=triggerMetadata, optional"`
	Timerange        time.Duration     `keda:"name=timerange,        order=triggerMetadata"`
	Timezone         string            `keda:"name=timezone,         order=triggerMetadata, default=UTC"`
	QueryAggregator  string            `keda:"name=queryAggregator,  order=triggerMetadata, enum=Latest;Avg;Sum;Count;Min;Max, default=Sum"`

	// Scaling parameters
	ActivationThreshold float64 `keda:"name=activationThreshold, order=triggerMetadata, default=0"`
	Threshold           float64 `keda:"name=threshold,           order=triggerMetadata"`

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:

// GetMetricsAndActivity retrieves metrics from sumologic
func (s *sumologicScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) {
	num, err := s.executeQuery(ctx)
	if err != nil {
		s.logger.Error(err, "error getting metrics from sumologic")
		return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error getting metrics from sumologic: %w", err)
	}

	metric := GenerateMetricInMili(metricName, num)
	isActive := num > s.metadata.ActivationThreshold

	return []external_metrics.ExternalMetricValue{metric}, isActive, nil
}

func (s *sumologicScaler) executeQuery(ctx context.Context) (float64, error) {
	return s.client.GetQueryResult(
		s.metadata.QueryType,
		s.metadata.Query,
		s.metadata.Queries,
		s.metadata.ResultQueryRowID,
		s.metadata.Quantization,
		s.metadata.Rollup,
		s.metadata.ResultField,
		s.metadata.Timerange,
		s.metadata.Timezone,
		s.metadata.QueryAggregator,
	)
}

Further, I am very curious to see how the Multi-Metrics Query Trigger is viewed by the community/maintainers (because of this discussion)

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing! I'll try out the things you mentioned above.

Further, I am very curious to see how the Multi-Metrics Query Trigger is viewed by the community/maintainers (because of #6697)

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 -

query.A: "metric=requests_total | rate"
query.B: "metric=request_capacity"
query.C: "(#A / #B) * 100 along service"
resultQueryRowID: "C"          # Which query result to extract

Here, we are just interested in the result of query.C but the system expects the input this way. Unlike the usecase in #6697, this is just a single trigger.

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

I'll try out the things you mentioned above

Done!
Looks much cleaner now. Thanks!

@rickbrouwer

Copy link
Copy Markdown
Member

I'll try out the things you mentioned above

Done! Looks much cleaner now. Thanks!

Great! Looks good! Just curious, in the sumologicMetadata, what are those mapstructure and validate:"required" for? I don't think that's necessary, unless you use it somewhere else in the code (I haven't looked into it yet)

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

You are right, it's not needed. Removed it. Thanks!

Comment thread pkg/scalers/sumologic/sumologic.go Outdated
@wozniakjan

wozniakjan commented May 20, 2025

Copy link
Copy Markdown
Member

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

@wozniakjan

wozniakjan commented Jun 9, 2025

Copy link
Copy Markdown
Member

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

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
@wozniakjan

wozniakjan commented Jun 16, 2025

Copy link
Copy Markdown
Member

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

@wozniakjan
wozniakjan requested a review from Copilot June 17, 2025 10:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/scalers and 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) {

@wozniakjan wozniakjan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm, thank you!

@wozniakjan

Copy link
Copy Markdown
Member

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]>
@wozniakjan

wozniakjan commented Jul 7, 2025

Copy link
Copy Markdown
Member

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

@mittalvaibhav1
mittalvaibhav1 requested a review from zroubalik July 8, 2025 08:37
@wozniakjan wozniakjan mentioned this pull request Jul 1, 2025
22 tasks
@wozniakjan

wozniakjan commented Jul 10, 2025

Copy link
Copy Markdown
Member

@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.

@wozniakjan
wozniakjan merged commit f5dd08a into kedacore:main Jul 10, 2025
27 checks passed
@rickbrouwer

Copy link
Copy Markdown
Member

It looks like the e2e test is failing on the main. Is that something that needs to be looked into?

https://github.com/kedacore/keda/actions/runs/16222531975

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

@rickbrouwer Ack, looking into it.

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

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?

@rickbrouwer

Copy link
Copy Markdown
Member

I think we can wait a little longer until you have figured it out. As @wozniakjan earlier said :

we have time to watch the e2es and debug until 2.18 release

so, keep us updated :)

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

Thank you! I should have a response by Monday.

@rickbrouwer

rickbrouwer commented Jul 14, 2025

Copy link
Copy Markdown
Member

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

@wozniakjan

Copy link
Copy Markdown
Member

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?

2025-07-07T20:42:42.2386347Z ***0***5-07-07T***0:4***:***5Z	ERROR	scale_handler	error getting scale decision	***"scaledObject.Namespace": "sumologic-keda-test-ns", "scaledObject.Name": "sumologic-keda-test-so", "scaler": "sumologicScaler", "error": "error getting metrics from sumologic: only agg queries are supported, please check your query"***
2025-07-07T20:42:42.2388719Z github.com/***/keda/v***/pkg/scaling.(*scaleHandler).getScalerState
2025-07-07T20:42:42.2390543Z 	/workspace/pkg/scaling/scale_handler.go:78***
2025-07-07T20:42:42.2393956Z github.com/***/keda/v***/pkg/scaling.(*scaleHandler).getScaledObjectState.func***
2025-07-07T20:42:42.2395775Z 	/workspace/pkg/scaling/scale_handler.go:6***4

@rickbrouwer

Copy link
Copy Markdown
Member

@mittalvaibhav1

Copy link
Copy Markdown
Contributor Author

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!

dpochopsky pushed a commit to dpochopsky/keda that referenced this pull request Sep 12, 2025
* (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]>
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.

7 participants