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

feat: support indexName in dynamodb scaler #4682

Merged
merged 5 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
### Improvements

- **General**: Metrics Adapter: remove deprecated Prometheus Metrics and non-gRPC code ([#3930](https://github.com/kedacore/keda/issues/3930))
- **AWS DynamoDB:** Add support for `indexName` ([#4680](https://github.com/kedacore/keda/issues/4680))
- **Azure Data Explorer Scaler**: Use azidentity SDK ([#4489](https://github.com/kedacore/keda/issues/4489))
- **External Scaler**: Add tls options in TriggerAuth metadata. ([#3565](https://github.com/kedacore/keda/issues/3565))
- **GCP PubSub Scaler**: Make it more flexible for metrics ([#4243](https://github.com/kedacore/keda/issues/4243))
Expand Down
9 changes: 9 additions & 0 deletions pkg/scalers/aws_dynamodb_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type awsDynamoDBMetadata struct {
keyConditionExpression string
expressionAttributeNames map[string]*string
expressionAttributeValues map[string]*dynamodb.AttributeValue
indexName string
targetValue int64
activationTargetValue int64
awsAuthorization awsAuthorizationMetadata
Expand Down Expand Up @@ -106,6 +107,10 @@ func parseAwsDynamoDBMetadata(config *ScalerConfig) (*awsDynamoDBMetadata, error
meta.awsEndpoint = val
}

if val, ok := config.TriggerMetadata["indexName"]; ok {
meta.indexName = val
}

if val, ok := config.TriggerMetadata["keyConditionExpression"]; ok && val != "" {
meta.keyConditionExpression = val
} else {
Expand Down Expand Up @@ -218,6 +223,10 @@ func (s *awsDynamoDBScaler) GetQueryMetrics() (float64, error) {
ExpressionAttributeValues: s.metadata.expressionAttributeValues,
}

if s.metadata.indexName != "" {
dimensions.IndexName = aws.String(s.metadata.indexName)
}

res, err := s.dbClient.Query(&dimensions)
if err != nil {
s.logger.Error(err, "Failed to get output")
Expand Down
92 changes: 88 additions & 4 deletions pkg/scalers/aws_dynamodb_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
testAWSDynamoSecretAccessKey = "none"
testAWSDynamoErrorTable = "Error"
testAWSDynamoNoValueTable = "NoValue"
testAWSDynamoIndexTable = "Index"
)

var testAWSDynamoAuthentication = map[string]string{
Expand Down Expand Up @@ -226,6 +227,66 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{
},
},
},
{
name: "properly formed dynamo name and region with activationTargetValue",
metadata: map[string]string{
"tableName": "test",
"awsRegion": "eu-west-1",
"keyConditionExpression": "#yr = :yyyy",
"expressionAttributeNames": "{ \"#yr\" : \"year\" }",
"expressionAttributeValues": "{\":yyyy\": {\"N\": \"1994\"}}",
"activationTargetValue": "1",
"targetValue": "3",
},
authParams: testAWSDynamoAuthentication,
expectedError: nil,
expectedMetadata: &awsDynamoDBMetadata{
tableName: "test",
awsRegion: "eu-west-1",
keyConditionExpression: "#yr = :yyyy",
expressionAttributeNames: map[string]*string{"#yr": &year},
expressionAttributeValues: map[string]*dynamodb.AttributeValue{":yyyy": &yearAttr},
activationTargetValue: 1,
targetValue: 3,
scalerIndex: 1,
metricName: "s1-aws-dynamodb-test",
awsAuthorization: awsAuthorizationMetadata{
awsAccessKeyID: "none",
awsSecretAccessKey: "none",
podIdentityOwner: true,
},
},
},
{
name: "properly formed dynamo name and region with index name",
metadata: map[string]string{
"tableName": "test",
"awsRegion": "eu-west-1",
"indexName": "test-index",
"keyConditionExpression": "#yr = :yyyy",
"expressionAttributeNames": "{ \"#yr\" : \"year\" }",
"expressionAttributeValues": "{\":yyyy\": {\"N\": \"1994\"}}",
"targetValue": "3",
},
authParams: testAWSDynamoAuthentication,
expectedError: nil,
expectedMetadata: &awsDynamoDBMetadata{
tableName: "test",
awsRegion: "eu-west-1",
indexName: "test-index",
keyConditionExpression: "#yr = :yyyy",
expressionAttributeNames: map[string]*string{"#yr": &year},
expressionAttributeValues: map[string]*dynamodb.AttributeValue{":yyyy": &yearAttr},
targetValue: 3,
scalerIndex: 1,
metricName: "s1-aws-dynamodb-test",
awsAuthorization: awsAuthorizationMetadata{
awsAccessKeyID: "none",
awsSecretAccessKey: "none",
podIdentityOwner: true,
},
},
},
}

func TestParseDynamoMetadata(t *testing.T) {
Expand Down Expand Up @@ -253,6 +314,7 @@ type mockDynamoDB struct {
}

var result int64 = 4
var indexResult int64 = 2
var empty int64

func (c *mockDynamoDB) Query(input *dynamodb.QueryInput) (*dynamodb.QueryOutput, error) {
Expand All @@ -265,6 +327,12 @@ func (c *mockDynamoDB) Query(input *dynamodb.QueryInput) (*dynamodb.QueryOutput,
}, nil
}

if input.IndexName != nil {
return &dynamodb.QueryOutput{
Count: &indexResult,
}, nil
}

return &dynamodb.QueryOutput{
Count: &result,
}, nil
Expand Down Expand Up @@ -299,6 +367,16 @@ var awsDynamoDBGetMetricTestData = []awsDynamoDBMetadata{
expressionAttributeValues: map[string]*dynamodb.AttributeValue{":yyyy": &yearAttr},
targetValue: 3,
},
{
tableName: testAWSDynamoIndexTable,
awsRegion: "eu-west-1",
indexName: "test-index",
keyConditionExpression: "#yr = :yyyy",
expressionAttributeNames: map[string]*string{"#yr": &year},
expressionAttributeValues: map[string]*dynamodb.AttributeValue{":yyyy": &yearAttr},
activationTargetValue: 3,
targetValue: 3,
},
}

func TestDynamoGetMetrics(t *testing.T) {
Expand All @@ -309,9 +387,11 @@ func TestDynamoGetMetrics(t *testing.T) {
value, _, err := scaler.GetMetricsAndActivity(context.Background(), "aws-dynamodb")
switch meta.tableName {
case testAWSDynamoErrorTable:
assert.Error(t, err, "expect error because of dynamodb api error")
assert.EqualError(t, err, "error", "expect error because of dynamodb api error")
case testAWSDynamoNoValueTable:
assert.NoError(t, err, "dont expect error when returning empty result from dynamodb")
case testAWSDynamoIndexTable:
assert.EqualValues(t, int64(2), value[0].Value.Value())
default:
assert.EqualValues(t, int64(4), value[0].Value.Value())
}
Expand All @@ -327,9 +407,11 @@ func TestDynamoGetQueryMetrics(t *testing.T) {
value, err := scaler.GetQueryMetrics()
switch meta.tableName {
case testAWSDynamoErrorTable:
assert.Error(t, err, "expect error because of dynamodb api error")
assert.EqualError(t, err, "error", "expect error because of dynamodb api error")
case testAWSDynamoNoValueTable:
assert.NoError(t, err, "dont expect error when returning empty metric list from cloudwatch")
assert.NoError(t, err, "dont expect error when returning empty result from dynamodb")
case testAWSDynamoIndexTable:
assert.EqualValues(t, int64(2), value)
default:
assert.EqualValues(t, int64(4), value)
}
Expand All @@ -345,9 +427,11 @@ func TestDynamoIsActive(t *testing.T) {
_, value, err := scaler.GetMetricsAndActivity(context.Background(), "aws-dynamodb")
switch meta.tableName {
case testAWSDynamoErrorTable:
assert.Error(t, err, "expect error because of cloudwatch api error")
assert.EqualError(t, err, "error", "expect error because of dynamodb api error")
case testAWSDynamoNoValueTable:
assert.NoError(t, err, "dont expect error when returning empty result from dynamodb")
case testAWSDynamoIndexTable:
assert.EqualValues(t, false, value)
default:
assert.EqualValues(t, true, value)
}
Expand Down