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

Azure Application Insights: Add option to ignore null values #4329

Merged
merged 4 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -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 = true
yardenshoham marked this conversation as resolved.
Show resolved Hide resolved
)

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