Skip to content

Commit

Permalink
Azure Application Insights: Add option to ignore null values (#4329)
Browse files Browse the repository at this point in the history
Signed-off-by: Yarden Shoham <hrsi88@gmail.com>
  • Loading branch information
yardenshoham committed Mar 8, 2023
1 parent 8524f98 commit 6d0d60c
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Here is an overview of all new **experimental** features:
- **General**: Add a warning when KEDA run outside supported k8s versions ([#4130](https://github.com/kedacore/keda/issues/4130))
- **General**: Use (self-signed) certificates for all the communications (internals and externals) ([#3931](https://github.com/kedacore/keda/issues/3931))
- **General**: Use TLS1.2 as minimum TLS version ([#4193](https://github.com/kedacore/keda/issues/4193))
- **Azure Application Insights Scaler**: Add ignoreNullValues to ignore errors when the data returned has null in its values ([#4316](https://github.com/kedacore/keda/issues/4316))
- **Azure Pipelines Scaler**: Improve error logging for `validatePoolID` ([#3996](https://github.com/kedacore/keda/issues/3996))
- **Azure Pipelines Scaler**: New configuration parameter `requireAllDemands` to scale only if jobs request all demands provided by the scaling definition ([#4138](https://github.com/kedacore/keda/issues/4138))
- **Hashicorp Vault**: Add support to secrets backend version 1 ([#2645](https://github.com/kedacore/keda/issues/2645))
Expand Down
10 changes: 7 additions & 3 deletions pkg/scalers/azure/azure_app_insights.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func extractAppInsightValue(info AppInsightsInfo, metric ApplicationInsightsMetr
}
floatVal = val.(float64)
} else {
return -1, fmt.Errorf("metric %s did not containe aggregation type %s", info.MetricID, info.AggregationType)
return -1, fmt.Errorf("metric %s did not contain aggregation type %s", info.MetricID, info.AggregationType)
}

azureAppInsightsLog.V(2).Info("value extracted from metric request", "metric type", info.AggregationType, "metric value", floatVal)
Expand All @@ -115,7 +115,7 @@ func queryParamsForAppInsightsRequest(info AppInsightsInfo) (map[string]interfac
}

// GetAzureAppInsightsMetricValue returns the value of an Azure App Insights metric, rounded to the nearest int
func GetAzureAppInsightsMetricValue(ctx context.Context, info AppInsightsInfo, podIdentity kedav1alpha1.AuthPodIdentity) (float64, error) {
func GetAzureAppInsightsMetricValue(ctx context.Context, info AppInsightsInfo, podIdentity kedav1alpha1.AuthPodIdentity, ignoreNullValues bool) (float64, error) {
config := getAuthConfig(ctx, info, podIdentity)
authorizer, err := config.Authorizer()
if err != nil {
Expand Down Expand Up @@ -154,5 +154,9 @@ func GetAzureAppInsightsMetricValue(ctx context.Context, info AppInsightsInfo, p
return -1, err
}

return extractAppInsightValue(info, *metric)
val, err := extractAppInsightValue(info, *metric)
if err != nil && ignoreNullValues {
return 0.0, nil
}
return val, err
}
22 changes: 21 additions & 1 deletion pkg/scalers/azure_app_insights_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,23 @@ const (
azureAppInsightsMetricAggregationTypeName = "metricAggregationType"
azureAppInsightsMetricFilterName = "metricFilter"
azureAppInsightsTenantIDName = "tenantId"
azureAppInsightsIgnoreNullValues = "ignoreNullValues"
)

var (
azureAppInsightsDefaultIgnoreNullValues = false
)

type azureAppInsightsMetadata struct {
azureAppInsightsInfo azure.AppInsightsInfo
targetValue float64
activationTargetValue float64
scalerIndex int
// sometimes we should consider there is an error we can accept
// default value is true/t, to ignore the null value returned from prometheus
// change to false/f if you can not accept prometheus returning null values
// https://github.com/kedacore/keda/issues/4316
ignoreNullValues bool
}

type azureAppInsightsScaler struct {
Expand Down Expand Up @@ -139,6 +149,16 @@ func parseAzureAppInsightsMetadata(config *ScalerConfig, logger logr.Logger) (*a
}
meta.azureAppInsightsInfo.ActiveDirectoryEndpoint = activeDirectoryEndpoint

meta.ignoreNullValues = azureAppInsightsDefaultIgnoreNullValues
if val, ok := config.TriggerMetadata[azureAppInsightsIgnoreNullValues]; ok && val != "" {
azureAppInsightsIgnoreNullValues, err := strconv.ParseBool(val)
if err != nil {
return nil, fmt.Errorf("err incorrect value for azureAppInsightsIgnoreNullValues given: %s, "+
"please use true or false", val)
}
meta.ignoreNullValues = azureAppInsightsIgnoreNullValues
}

// Required authentication parameters below

val, err = getParameterFromConfig(config, azureAppInsightsAppIDName, true)
Expand Down Expand Up @@ -182,7 +202,7 @@ func (s *azureAppInsightsScaler) GetMetricSpecForScaling(context.Context) []v2.M

// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric
func (s *azureAppInsightsScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) {
val, err := azure.GetAzureAppInsightsMetricValue(ctx, s.metadata.azureAppInsightsInfo, s.podIdentity)
val, err := azure.GetAzureAppInsightsMetricValue(ctx, s.metadata.azureAppInsightsInfo, s.podIdentity, s.metadata.ignoreNullValues)
if err != nil {
s.logger.Error(err, "error getting azure app insights metric")
return []external_metrics.ExternalMetricValue{}, false, err
Expand Down
18 changes: 18 additions & 0 deletions pkg/scalers/azure_app_insights_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,24 @@ var azureAppInsightsScalerData = []azureAppInsightsScalerTestData{
},
PodIdentity: kedav1alpha1.AuthPodIdentity{Provider: kedav1alpha1.PodIdentityProvider("notAzureWorkload")},
}},
{name: "correct ignoreNullValues (true)", isError: false, config: ScalerConfig{
TriggerMetadata: map[string]string{
"targetValue": "11", "applicationInsightsId": "1234", "metricId": "unittest/test", "metricAggregationTimespan": "01:02", "metricAggregationType": "max", "metricFilter": "cloud/roleName eq 'test'", "tenantId": "1234", "ignoreNullValues": "true",
},
PodIdentity: kedav1alpha1.AuthPodIdentity{Provider: kedav1alpha1.PodIdentityProviderAzureWorkload},
}},
{name: "correct ignoreNullValues (false)", isError: false, config: ScalerConfig{
TriggerMetadata: map[string]string{
"targetValue": "11", "applicationInsightsId": "1234", "metricId": "unittest/test", "metricAggregationTimespan": "01:02", "metricAggregationType": "max", "metricFilter": "cloud/roleName eq 'test'", "tenantId": "1234", "ignoreNullValues": "false",
},
PodIdentity: kedav1alpha1.AuthPodIdentity{Provider: kedav1alpha1.PodIdentityProviderAzureWorkload},
}},
{name: "incorrect ignoreNullValues", isError: true, config: ScalerConfig{
TriggerMetadata: map[string]string{
"targetValue": "11", "applicationInsightsId": "1234", "metricId": "unittest/test", "metricAggregationTimespan": "01:02", "metricAggregationType": "max", "metricFilter": "cloud/roleName eq 'test'", "tenantId": "1234", "ignoreNullValues": "not a boolean",
},
PodIdentity: kedav1alpha1.AuthPodIdentity{Provider: kedav1alpha1.PodIdentityProviderAzureWorkload},
}},
{name: "app insights id in auth", isError: false, config: ScalerConfig{
TriggerMetadata: map[string]string{
"targetValue": "11", "metricId": "unittest/test", "metricAggregationTimespan": "01:02", "metricAggregationType": "max", "metricFilter": "cloud/roleName eq 'test'", "tenantId": "1234",
Expand Down

0 comments on commit 6d0d60c

Please sign in to comment.