-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: honeycomb scaler #6896
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
base: main
Are you sure you want to change the base?
feat: honeycomb scaler #6896
Conversation
d7a2845 to
d66d23f
Compare
dttung2905
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @kmoonwright , thanks so much for working on this. I strongly believe this is a useful addition to the list of scaler we support. I know it is still a draft but i want to give some feedbacks/ nits as early as possible. Hope you don't mind 😄
|
Great feedback, thank you! Much appreciated even in the draft |
2a97850 to
8e7d334
Compare
…e#6856) Signed-off-by: Jorge Turrado <[email protected]> Signed-off-by: kmoonwright <[email protected]>
…re#6860) When the ScaledObject adopts an HPA or if the hpaName in the ScaledObject status is removed for any reason, the hpaName status will no longer get set. It is only set when the HPA is created by the ScaledObject. We add setting the HPA name in the ScaledObject status to the regular reconciliation loop for the ScaledObject. See kedacore#6336 Signed-off-by: Justin Miron <[email protected]> Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]> Signed-off-by: kmoonwright <[email protected]>
…ter usage (kedacore#6843) Signed-off-by: Rick Brouwer <[email protected]> Signed-off-by: kmoonwright <[email protected]>
* Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Fix Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> --------- Signed-off-by: zhenghanzhou <[email protected]> Co-authored-by: zhenghanzhou <[email protected]> Co-authored-by: Zbynek Roubalik <[email protected]> Signed-off-by: kmoonwright <[email protected]>
…re#6780) Signed-off-by: Alexander Huck <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]> Co-authored-by: Jorge Turrado Ferrero <[email protected]> Signed-off-by: kmoonwright <[email protected]>
) * fix: e2e test checks label value by name and not by index Signed-off-by: Jorge Turrado <[email protected]> * fix: e2e test checks label value by name and not by index Signed-off-by: Jorge Turrado <[email protected]> * fix: e2e test checks label value by name and not by index Signed-off-by: Jorge Turrado <[email protected]> --------- Signed-off-by: Jorge Turrado <[email protected]> Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
1338abf to
2d6edac
Compare
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
zroubalik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kmoonwright, thanks for the contribution.
Could you please also add e2e tests for this scaler? This is a requirement for us in order to proceed forward towards merging.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Honeycomb scaler that enables KEDA to scale workloads based on observability data from Honeycomb.io. The scaler supports both legacy configuration fields and flexible raw JSON queries, integrates with the Honeycomb API for query execution, and includes comprehensive error handling and edge case management.
- Adds complete Honeycomb scaler implementation with API integration
- Provides flexible query configuration supporting both legacy fields and raw JSON
- Includes comprehensive unit tests for metadata parsing and result extraction
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/scaling/scalers_builder.go | Registers the new honeycomb scaler in the scaler factory |
| pkg/scalers/honeycomb_scaler.go | Complete scaler implementation with API client and query logic |
| pkg/scalers/honeycomb_scaler_test.go | Comprehensive unit tests for metadata parsing and result extraction |
| CHANGELOG.md | Documents the addition of the new Honeycomb scaler |
Comments suppressed due to low confidence (1)
pkg/scalers/honeycomb_scaler.go:175
- [nitpick] The error message contains a very long URL and detailed explanation that may be truncated in logs. Consider shortening the message or moving the detailed explanation to documentation.
return 0, fmt.Errorf("honeycomb: unauthorized (401) - an Enterprise API key is required, check your API key permissions. See: https://api-docs.honeycomb.io/api/query-data for details. Response: %s", string(body))
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
…ithin 10 seconds Signed-off-by: kmoonwright <[email protected]>
…te limits Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Resolved merge conflict in CHANGELOG.md by properly placing Honeycomb Scaler entry in the New section following contributing guidelines.
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
Signed-off-by: kmoonwright <[email protected]>
* refactor: rename temporal scaler files for consistency Renames the temporal scaler files to follow the convention used by other scalers in the project. - -> - -> - -> Signed-off-by: Aniruddh Chaturvedi <[email protected]> * address feedback Signed-off-by: Aniruddh Chaturvedi <[email protected]> --------- Signed-off-by: Aniruddh Chaturvedi <[email protected]> Signed-off-by: Aniruddh Chaturvedi <[email protected]> Signed-off-by: kmoonwright <[email protected]>
Reorder AWS DynamoDB scalers to match KEDA's lexicographic sorting requirements: - aws-dynamodb-streams now comes before aws-dynamodb - This resolves the CI failure for scaler alphabetical ordering Signed-off-by: kmoonwright <[email protected]>
- Reorder AWS DynamoDB scalers: aws-dynamodb before aws-dynamodb-streams - Reorder external scalers: external before external-mock and external-push - Reorder redis scalers: redis first, then redis-cluster, redis-cluster-streams, etc. - This matches the exact order in the main repository Signed-off-by: kmoonwright <[email protected]>
3298c42 to
f8d02ed
Compare
|
/run-e2e honeycomb |
|
|
||
| ## Unreleased | ||
|
|
||
| - **General**: Introduce new Honeycomb Scaler ([#6896](https://github.com/kedacore/keda/pull/6896/)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to New section
| module github.com/kedacore/keda/v2 | ||
|
|
||
| go 1.23.8 | ||
| go 1.23 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this required?
| } | ||
|
|
||
| type honeycombMetadata struct { | ||
| APIKey string `keda:"name=apiKey, order=authParams;triggerMetadata"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't support reading secrets from metadata
| APIKey string `keda:"name=apiKey, order=authParams;triggerMetadata"` | |
| APIKey string `keda:"name=apiKey, order=authParams"` |
| meta.TriggerIndex = config.TriggerIndex | ||
|
|
||
| // Use queryRaw if provided, else build query from legacy fields | ||
| if raw, ok := config.TriggerMetadata["queryRaw"]; ok && raw != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we just use the parsed value?
| if raw, ok := config.TriggerMetadata["queryRaw"]; ok && raw != "" { | |
| if meta.QueryRaw != "" { |
| "disable_total_by_aggregate": true, | ||
| "disable_other_by_aggregate": true, | ||
| // Query results limit is 10000, see https://api-docs.honeycomb.io/api for details | ||
| "limit": honeycombQueryResultsLimit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to expose this to users? I mean, the limit must be enforced, but maybe a user prefers to query X items instead of the limit. Maybe we can expose this to users and check that value doesn't pass the limit.
Or maybe this is not a valid use case, saying the truth, I don't have any experience with honeycomb
| body, _ := io.ReadAll(runResp.Body) | ||
| return 0, fmt.Errorf("honeycomb: unauthorized (401) - an Enterprise API key is required, check your API key permissions. See: https://api-docs.honeycomb.io/api/query-data for details. Response: %s", string(body)) | ||
| } | ||
| if runResp.StatusCode != 200 && runResp.StatusCode != 201 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
| statusResp.Body.Close() | ||
| return 0, errors.New("honeycomb: rate limited (429) on poll, back off and try again later") | ||
| } | ||
| if statusResp.StatusCode != 200 && statusResp.StatusCode != 201 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
| @@ -0,0 +1,308 @@ | |||
| //go:build e2e | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are the metrics sent to honeycomb? I don't see any agent or so that collects them
zroubalik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kmoonwright any update here please?
Honeycomb Scaler: Add New External Metrics Scaler
This PR introduces a new scaler for Honeycomb.io, which enables KEDA to scale workloads based on observability data and queries executed against Honeycomb datasets.
What Has Changed
Adds
scalers/honeycomb_scaler.go(and associated test files) that allows users to scale workloads based on Honeycomb queries.Supports both legacy config fields and flexible
queryRawJSON for arbitrary query construction.Interacts with the Honeycomb API for query creation, execution, and polling.
Handles response parsing, missing fields, non-numeric results, and timeouts robustly.
Comprehensive tests for metadata parsing and result field extraction.
Updated CRD and deployment YAMLs to ensure compatibility with the new scaler.
See documentation PR for keda-docs repository #1587
Checklist
Fixes #
Relates to #<link docs/helm PRs when opened>
Notes: