From dc764d44048e393b5809cc40d1f6e9bd9871195b Mon Sep 17 00:00:00 2001 From: Zbynek Roubalik Date: Fri, 2 Dec 2022 11:02:12 +0100 Subject: [PATCH] cache metrics in polling interval call Signed-off-by: Zbynek Roubalik --- apis/keda/v1alpha1/scaledobject_types.go | 12 +- apis/keda/v1alpha1/zz_generated.deepcopy.go | 5 + config/crd/bases/keda.sh_scaledjobs.yaml | 5 + config/crd/bases/keda.sh_scaledobjects.yaml | 5 + controllers/keda/hpa_test.go | 1 - pkg/fallback/fallback_test.go | 34 ++-- pkg/metricsservice/server.go | 6 +- pkg/mock/mock_scaler/mock_scaler.go | 64 ++---- pkg/mock/mock_scaling/mock_interface.go | 12 +- pkg/prommetrics/prommetrics.go | 4 +- pkg/scalers/activemq_scaler.go | 16 +- pkg/scalers/artemis_scaler.go | 19 +- pkg/scalers/aws_cloudwatch_scaler.go | 16 +- pkg/scalers/aws_cloudwatch_scaler_test.go | 2 +- pkg/scalers/aws_dynamodb_scaler.go | 15 +- pkg/scalers/aws_dynamodb_scaler_test.go | 4 +- pkg/scalers/aws_dynamodb_streams_scaler.go | 25 +-- .../aws_dynamodb_streams_scaler_test.go | 4 +- pkg/scalers/aws_kinesis_stream_scaler.go | 19 +- pkg/scalers/aws_kinesis_stream_scaler_test.go | 2 +- pkg/scalers/aws_sqs_queue_scaler.go | 19 +- pkg/scalers/aws_sqs_queue_scaler_test.go | 2 +- pkg/scalers/azure_app_insights_scaler.go | 17 +- pkg/scalers/azure_blob_scaler.go | 25 +-- pkg/scalers/azure_data_explorer_scaler.go | 16 +- pkg/scalers/azure_eventhub_scaler.go | 17 ++ pkg/scalers/azure_log_analytics_scaler.go | 53 +---- .../azure_log_analytics_scaler_test.go | 2 - pkg/scalers/azure_monitor_scaler.go | 19 +- pkg/scalers/azure_pipelines_scaler.go | 23 +-- pkg/scalers/azure_queue_scaler.go | 28 +-- pkg/scalers/azure_servicebus_scaler.go | 19 +- pkg/scalers/cassandra_scaler.go | 18 +- pkg/scalers/cpu_memory_scaler.go | 11 +- pkg/scalers/cron_scaler.go | 19 +- pkg/scalers/cron_scaler_test.go | 8 +- pkg/scalers/datadog_scaler.go | 17 +- pkg/scalers/elasticsearch_scaler.go | 16 +- pkg/scalers/external_mock_scaler.go | 15 +- pkg/scalers/external_scaler.go | 17 ++ pkg/scalers/gcp_pubsub_scaler.go | 30 +-- pkg/scalers/gcp_stackdriver_scaler.go | 15 +- pkg/scalers/gcp_storage_scaler.go | 18 +- pkg/scalers/graphite_scaler.go | 16 +- pkg/scalers/huawei_cloudeye_scaler.go | 16 +- pkg/scalers/ibmmq_scaler.go | 15 +- pkg/scalers/influxdb_scaler.go | 18 +- pkg/scalers/kafka_scaler.go | 16 +- pkg/scalers/kubernetes_workload_scaler.go | 19 +- .../kubernetes_workload_scaler_test.go | 4 +- pkg/scalers/liiklus/LiiklusService.pb.go | 22 ++- pkg/scalers/liiklus/LiiklusService_grpc.pb.go | 12 +- pkg/scalers/liiklus_scaler.go | 19 +- pkg/scalers/liiklus_scaler_test.go | 8 +- pkg/scalers/loki_scaler.go | 19 +- pkg/scalers/metrics_api_scaler.go | 17 +- pkg/scalers/metrics_api_scaler_test.go | 2 +- pkg/scalers/mongo_scaler.go | 17 +- pkg/scalers/mssql_scaler.go | 18 +- pkg/scalers/mysql_scaler.go | 18 +- pkg/scalers/nats_jetstream_scaler.go | 52 +---- pkg/scalers/newrelic_scaler.go | 15 +- pkg/scalers/openstack_metrics_scaler.go | 16 +- pkg/scalers/openstack_swift_scaler.go | 16 +- pkg/scalers/postgresql_scaler.go | 18 +- pkg/scalers/predictkube_scaler.go | 17 ++ pkg/scalers/prometheus_scaler.go | 16 +- pkg/scalers/pulsar_scaler.go | 25 +-- pkg/scalers/pulsar_scaler_test.go | 12 +- pkg/scalers/rabbitmq_scaler.go | 24 +-- pkg/scalers/rabbitmq_scaler_test.go | 8 +- pkg/scalers/redis_scaler.go | 20 +- pkg/scalers/redis_streams_scaler.go | 21 +- pkg/scalers/scaler.go | 12 +- pkg/scalers/selenium_grid_scaler.go | 15 +- pkg/scalers/solace_scaler.go | 21 +- pkg/scalers/stan_scaler.go | 72 +++---- .../cache/metricscache/metrics_record.go | 45 +++++ pkg/scaling/cache/scalers_cache.go | 108 ++++++----- pkg/scaling/cache/scalers_cache_test.go | 16 +- pkg/scaling/scale_handler.go | 182 ++++++++++-------- pkg/scaling/scale_handler_test.go | 38 ++-- 82 files changed, 631 insertions(+), 1088 deletions(-) create mode 100644 pkg/scaling/cache/metricscache/metrics_record.go diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index e06d49f0425..96f4162ca41 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -123,7 +123,12 @@ type ScaleTarget struct { type ScaleTriggers struct { Type string `json:"type"` // +optional - Name string `json:"name,omitempty"` + Name string `json:"name,omitempty"` + // +optional + CacheDuration *int32 `json:"cacheDuration,omitempty"` + + EnableCache bool `json:"enableCache,omitempty"` + Metadata map[string]string `json:"metadata"` // +optional AuthenticationRef *ScaledObjectAuthRef `json:"authenticationRef,omitempty"` @@ -179,3 +184,8 @@ type ScaledObjectAuthRef struct { func init() { SchemeBuilder.Register(&ScaledObject{}, &ScaledObjectList{}) } + +// GenerateIdentifier returns identifier for the object in for "kind.namespace.name" +func (s *ScaledObject) GenerateIdentifier() string { + return GenerateIdentifier(s.Kind, s.Namespace, s.Name) +} diff --git a/apis/keda/v1alpha1/zz_generated.deepcopy.go b/apis/keda/v1alpha1/zz_generated.deepcopy.go index 0d5a81c8a00..353af765dd5 100644 --- a/apis/keda/v1alpha1/zz_generated.deepcopy.go +++ b/apis/keda/v1alpha1/zz_generated.deepcopy.go @@ -428,6 +428,11 @@ func (in *ScaleTarget) DeepCopy() *ScaleTarget { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ScaleTriggers) DeepCopyInto(out *ScaleTriggers) { *out = *in + if in.CacheDuration != nil { + in, out := &in.CacheDuration, &out.CacheDuration + *out = new(int32) + **out = **in + } if in.Metadata != nil { in, out := &in.Metadata, &out.Metadata *out = make(map[string]string, len(*in)) diff --git a/config/crd/bases/keda.sh_scaledjobs.yaml b/config/crd/bases/keda.sh_scaledjobs.yaml index 4e091d1f888..f6122e601f3 100644 --- a/config/crd/bases/keda.sh_scaledjobs.yaml +++ b/config/crd/bases/keda.sh_scaledjobs.yaml @@ -7861,6 +7861,11 @@ spec: required: - name type: object + cacheDuration: + format: int32 + type: integer + enableCache: + type: boolean metadata: additionalProperties: type: string diff --git a/config/crd/bases/keda.sh_scaledobjects.yaml b/config/crd/bases/keda.sh_scaledobjects.yaml index 22925dcd715..ee694b7092d 100644 --- a/config/crd/bases/keda.sh_scaledobjects.yaml +++ b/config/crd/bases/keda.sh_scaledobjects.yaml @@ -263,6 +263,11 @@ spec: required: - name type: object + cacheDuration: + format: int32 + type: integer + enableCache: + type: boolean metadata: additionalProperties: type: string diff --git a/controllers/keda/hpa_test.go b/controllers/keda/hpa_test.go index 7cef70233db..a6655cae034 100644 --- a/controllers/keda/hpa_test.go +++ b/controllers/keda/hpa_test.go @@ -137,7 +137,6 @@ func setupTest(health map[string]v1alpha1.HealthStatus, scaler *mock_scalers.Moc return scaler, &scalers.ScalerConfig{}, nil }, }}, - Logger: logr.Discard(), Recorder: nil, } metricSpec := v2.MetricSpec{ diff --git a/pkg/fallback/fallback_test.go b/pkg/fallback/fallback_test.go index 2103d441ead..c4124b3217b 100644 --- a/pkg/fallback/fallback_test.go +++ b/pkg/fallback/fallback_test.go @@ -76,7 +76,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(3) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) metrics, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) @@ -107,7 +107,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(3) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) metrics, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) @@ -117,13 +117,13 @@ var _ = Describe("fallback", func() { }) It("should propagate the error when fallback is disabled", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) so := buildScaledObject(nil, nil) metricSpec := createMetricSpec(3) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) _, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ShouldNot(BeNil()) @@ -131,7 +131,7 @@ var _ = Describe("fallback", func() { }) It("should bump the number of failures when metrics call fails", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(0) so := buildScaledObject( @@ -152,7 +152,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(10) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) _, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ShouldNot(BeNil()) @@ -161,7 +161,7 @@ var _ = Describe("fallback", func() { }) It("should return a normalised metric when number of failures are beyond threshold", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) expectedMetricValue := int64(100) @@ -182,7 +182,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(10) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) metrics, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) @@ -214,7 +214,7 @@ var _ = Describe("fallback", func() { }) It("should ignore error if we fail to update kubernetes status", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) expectedMetricValue := int64(100) @@ -238,7 +238,7 @@ var _ = Describe("fallback", func() { statusWriter.EXPECT().Patch(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("Some error")) client.EXPECT().Status().Return(statusWriter) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) metrics, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) @@ -248,7 +248,7 @@ var _ = Describe("fallback", func() { }) It("should return error when fallback is enabled but scaledobject has invalid parameter", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) so := buildScaledObject( @@ -268,7 +268,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(10) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) _, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ShouldNot(BeNil()) @@ -276,7 +276,7 @@ var _ = Describe("fallback", func() { }) It("should set the fallback condition when a fallback exists in the scaled object", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) failingNumberOfFailures := int32(6) anotherMetricName := "another metric name" @@ -302,7 +302,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(10) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) _, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) condition := so.Status.Conditions.GetFallbackCondition() @@ -310,7 +310,7 @@ var _ = Describe("fallback", func() { }) It("should set the fallback condition to false if the config is invalid", func() { - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return(nil, errors.New("Some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) failingNumberOfFailures := int32(6) anotherMetricName := "another metric name" @@ -336,7 +336,7 @@ var _ = Describe("fallback", func() { metricSpec := createMetricSpec(10) expectStatusPatch(ctrl, client) - metrics, err := scaler.GetMetrics(context.Background(), metricName) + metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName) _, err = GetMetricsWithFallback(context.Background(), client, logger, metrics, err, metricName, so, metricSpec) Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(Equal("Some error")) @@ -425,7 +425,7 @@ func primeGetMetrics(scaler *mock_scalers.MockScaler, value int64) { Timestamp: metav1.Now(), } - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Eq(metricName)).Return([]external_metrics.ExternalMetricValue{expectedMetric}, nil) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return([]external_metrics.ExternalMetricValue{expectedMetric}, true, nil) } func createMetricSpec(averageValue int) v2.MetricSpec { diff --git a/pkg/metricsservice/server.go b/pkg/metricsservice/server.go index 2d6f6325ee0..9a6d6d9c2c7 100644 --- a/pkg/metricsservice/server.go +++ b/pkg/metricsservice/server.go @@ -23,13 +23,13 @@ import ( "google.golang.org/grpc" "k8s.io/metrics/pkg/apis/external_metrics/v1beta1" - ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/kedacore/keda/v2/pkg/metricsservice/api" "github.com/kedacore/keda/v2/pkg/scaling" ) -var log = ctrl.Log.WithName("grpc_server") +var log = logf.Log.WithName("grpc_server") type GrpcServer struct { server *grpc.Server @@ -41,7 +41,7 @@ type GrpcServer struct { // GetMetrics returns metrics values in form of ExternalMetricValueList for specified ScaledObject reference func (s *GrpcServer) GetMetrics(ctx context.Context, in *api.ScaledObjectRef) (*api.Response, error) { v1beta1ExtMetrics := &v1beta1.ExternalMetricValueList{} - extMetrics, exportedMetrics, err := (*s.scalerHandler).GetExternalMetrics(ctx, in.Name, in.Namespace, in.MetricName) + extMetrics, exportedMetrics, err := (*s.scalerHandler).GetScaledObjectMetrics(ctx, in.Name, in.Namespace, in.MetricName) if err != nil { return nil, fmt.Errorf("error when getting metric values %s", err) } diff --git a/pkg/mock/mock_scaler/mock_scaler.go b/pkg/mock/mock_scaler/mock_scaler.go index 7615887ae30..db30ab8403a 100644 --- a/pkg/mock/mock_scaler/mock_scaler.go +++ b/pkg/mock/mock_scaler/mock_scaler.go @@ -64,34 +64,20 @@ func (mr *MockScalerMockRecorder) GetMetricSpecForScaling(ctx interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetricSpecForScaling", reflect.TypeOf((*MockScaler)(nil).GetMetricSpecForScaling), ctx) } -// GetMetrics mocks base method. -func (m *MockScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity mocks base method. +func (m *MockScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetMetrics", ctx, metricName) + ret := m.ctrl.Call(m, "GetMetricsAndActivity", ctx, metricName) ret0, _ := ret[0].([]external_metrics.ExternalMetricValue) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } -// GetMetrics indicates an expected call of GetMetrics. -func (mr *MockScalerMockRecorder) GetMetrics(ctx, metricName interface{}) *gomock.Call { +// GetMetricsAndActivity indicates an expected call of GetMetricsAndActivity. +func (mr *MockScalerMockRecorder) GetMetricsAndActivity(ctx, metricName interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetrics", reflect.TypeOf((*MockScaler)(nil).GetMetrics), ctx, metricName) -} - -// IsActive mocks base method. -func (m *MockScaler) IsActive(ctx context.Context) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsActive", ctx) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// IsActive indicates an expected call of IsActive. -func (mr *MockScalerMockRecorder) IsActive(ctx interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsActive", reflect.TypeOf((*MockScaler)(nil).IsActive), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetricsAndActivity", reflect.TypeOf((*MockScaler)(nil).GetMetricsAndActivity), ctx, metricName) } // MockPushScaler is a mock of PushScaler interface. @@ -145,34 +131,20 @@ func (mr *MockPushScalerMockRecorder) GetMetricSpecForScaling(ctx interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetricSpecForScaling", reflect.TypeOf((*MockPushScaler)(nil).GetMetricSpecForScaling), ctx) } -// GetMetrics mocks base method. -func (m *MockPushScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity mocks base method. +func (m *MockPushScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetMetrics", ctx, metricName) + ret := m.ctrl.Call(m, "GetMetricsAndActivity", ctx, metricName) ret0, _ := ret[0].([]external_metrics.ExternalMetricValue) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetMetrics indicates an expected call of GetMetrics. -func (mr *MockPushScalerMockRecorder) GetMetrics(ctx, metricName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetrics", reflect.TypeOf((*MockPushScaler)(nil).GetMetrics), ctx, metricName) -} - -// IsActive mocks base method. -func (m *MockPushScaler) IsActive(ctx context.Context) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsActive", ctx) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } -// IsActive indicates an expected call of IsActive. -func (mr *MockPushScalerMockRecorder) IsActive(ctx interface{}) *gomock.Call { +// GetMetricsAndActivity indicates an expected call of GetMetricsAndActivity. +func (mr *MockPushScalerMockRecorder) GetMetricsAndActivity(ctx, metricName interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsActive", reflect.TypeOf((*MockPushScaler)(nil).IsActive), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetricsAndActivity", reflect.TypeOf((*MockPushScaler)(nil).GetMetricsAndActivity), ctx, metricName) } // Run mocks base method. diff --git a/pkg/mock/mock_scaling/mock_interface.go b/pkg/mock/mock_scaling/mock_interface.go index 8cc1a70b93b..d0579a12bac 100644 --- a/pkg/mock/mock_scaling/mock_interface.go +++ b/pkg/mock/mock_scaling/mock_interface.go @@ -65,20 +65,20 @@ func (mr *MockScaleHandlerMockRecorder) DeleteScalableObject(ctx, scalableObject return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteScalableObject", reflect.TypeOf((*MockScaleHandler)(nil).DeleteScalableObject), ctx, scalableObject) } -// GetExternalMetrics mocks base method. -func (m *MockScaleHandler) GetExternalMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, *api.PromMetricsMsg, error) { +// GetScaledObjectMetrics mocks base method. +func (m *MockScaleHandler) GetScaledObjectMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, *api.PromMetricsMsg, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetExternalMetrics", ctx, scaledObjectName, scaledObjectNamespace, metricName) + ret := m.ctrl.Call(m, "GetScaledObjectMetrics", ctx, scaledObjectName, scaledObjectNamespace, metricName) ret0, _ := ret[0].(*external_metrics.ExternalMetricValueList) ret1, _ := ret[1].(*api.PromMetricsMsg) ret2, _ := ret[2].(error) return ret0, ret1, ret2 } -// GetExternalMetrics indicates an expected call of GetExternalMetrics. -func (mr *MockScaleHandlerMockRecorder) GetExternalMetrics(ctx, scaledObjectName, scaledObjectNamespace, metricName interface{}) *gomock.Call { +// GetScaledObjectMetrics indicates an expected call of GetScaledObjectMetrics. +func (mr *MockScaleHandlerMockRecorder) GetScaledObjectMetrics(ctx, scaledObjectName, scaledObjectNamespace, metricName interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetExternalMetrics", reflect.TypeOf((*MockScaleHandler)(nil).GetExternalMetrics), ctx, scaledObjectName, scaledObjectNamespace, metricName) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetScaledObjectMetrics", reflect.TypeOf((*MockScaleHandler)(nil).GetScaledObjectMetrics), ctx, scaledObjectName, scaledObjectNamespace, metricName) } // GetScalersCache mocks base method. diff --git a/pkg/prommetrics/prommetrics.go b/pkg/prommetrics/prommetrics.go index 925f7728173..193128ef29c 100644 --- a/pkg/prommetrics/prommetrics.go +++ b/pkg/prommetrics/prommetrics.go @@ -20,11 +20,11 @@ import ( "strconv" "github.com/prometheus/client_golang/prometheus" - ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/metrics" ) -var log = ctrl.Log.WithName("prometheus_server") +var log = logf.Log.WithName("prometheus_server") const ( ClusterTriggerAuthenticationResource = "cluster_trigger_authentication" diff --git a/pkg/scalers/activemq_scaler.go b/pkg/scalers/activemq_scaler.go index 6bd22c72d25..70e95fcb409 100644 --- a/pkg/scalers/activemq_scaler.go +++ b/pkg/scalers/activemq_scaler.go @@ -166,16 +166,6 @@ func parseActiveMQMetadata(config *ScalerConfig) (*activeMQMetadata, error) { return &meta, nil } -func (s *activeMQScaler) IsActive(ctx context.Context) (bool, error) { - queueSize, err := s.getQueueMessageCount(ctx) - if err != nil { - s.logger.Error(err, "Unable to access activeMQ management endpoint", "managementEndpoint", s.metadata.managementEndpoint) - return false, err - } - - return queueSize > s.metadata.activationTargetQueueSize, nil -} - // getRestAPIParameters parse restAPITemplate to provide managementEndpoint, brokerName, destinationName func getRestAPIParameters(meta activeMQMetadata) (activeMQMetadata, error) { u, err := url.ParseRequestURI(meta.restAPITemplate) @@ -278,15 +268,15 @@ func (s *activeMQScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpe return []v2.MetricSpec{metricSpec} } -func (s *activeMQScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *activeMQScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { queueSize, err := s.getQueueMessageCount(ctx) if err != nil { - return nil, fmt.Errorf("error inspecting ActiveMQ queue size: %s", err) + return nil, false, fmt.Errorf("error inspecting ActiveMQ queue size: %s", err) } metric := GenerateMetricInMili(metricName, float64(queueSize)) - return []external_metrics.ExternalMetricValue{metric}, nil + return []external_metrics.ExternalMetricValue{metric}, queueSize > s.metadata.activationTargetQueueSize, nil } func (s *activeMQScaler) Close(context.Context) error { diff --git a/pkg/scalers/artemis_scaler.go b/pkg/scalers/artemis_scaler.go index df8d9a1480a..273046c68ba 100644 --- a/pkg/scalers/artemis_scaler.go +++ b/pkg/scalers/artemis_scaler.go @@ -176,17 +176,6 @@ func parseArtemisMetadata(config *ScalerConfig) (*artemisMetadata, error) { return &meta, nil } -// IsActive determines if we need to scale from zero -func (s *artemisScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.getQueueMessageCount(ctx) - if err != nil { - s.logger.Error(err, "Unable to access the artemis management endpoint", "managementEndpoint", s.metadata.managementEndpoint) - return false, err - } - - return messages > s.metadata.activationQueueLength, nil -} - // getAPIParameters parse restAPITemplate to provide managementEndpoint , brokerName, brokerAddress, queueName func getAPIParameters(meta artemisMetadata) (artemisMetadata, error) { u, err := url.ParseRequestURI(meta.restAPITemplate) @@ -277,18 +266,18 @@ func (s *artemisScaler) GetMetricSpecForScaling(ctx context.Context) []v2.Metric return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *artemisScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *artemisScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { messages, err := s.getQueueMessageCount(ctx) if err != nil { s.logger.Error(err, "Unable to access the artemis management endpoint", "managementEndpoint", s.metadata.managementEndpoint) - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(messages)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, messages > s.metadata.activationQueueLength, nil } // Nothing to close here. diff --git a/pkg/scalers/aws_cloudwatch_scaler.go b/pkg/scalers/aws_cloudwatch_scaler.go index 646988995eb..5c448ae6b13 100644 --- a/pkg/scalers/aws_cloudwatch_scaler.go +++ b/pkg/scalers/aws_cloudwatch_scaler.go @@ -273,17 +273,17 @@ func computeQueryWindow(current time.Time, metricPeriodSec, metricEndTimeOffsetS return } -func (s *awsCloudwatchScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *awsCloudwatchScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { metricValue, err := s.GetCloudwatchMetrics() if err != nil { s.logger.Error(err, "Error getting metric value") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, metricValue) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, metricValue > s.metadata.activationTargetMetricValue, nil } func (s *awsCloudwatchScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { @@ -305,16 +305,6 @@ func (s *awsCloudwatchScaler) GetMetricSpecForScaling(context.Context) []v2.Metr return []v2.MetricSpec{metricSpec} } -func (s *awsCloudwatchScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.GetCloudwatchMetrics() - - if err != nil { - return false, err - } - - return val > s.metadata.activationTargetMetricValue, nil -} - func (s *awsCloudwatchScaler) Close(context.Context) error { return nil } diff --git a/pkg/scalers/aws_cloudwatch_scaler_test.go b/pkg/scalers/aws_cloudwatch_scaler_test.go index e07153cdff0..9fb09287327 100644 --- a/pkg/scalers/aws_cloudwatch_scaler_test.go +++ b/pkg/scalers/aws_cloudwatch_scaler_test.go @@ -501,7 +501,7 @@ func TestAWSCloudwatchGetMetricSpecForScaling(t *testing.T) { func TestAWSCloudwatchScalerGetMetrics(t *testing.T) { for _, meta := range awsCloudwatchGetMetricTestData { mockAWSCloudwatchScaler := awsCloudwatchScaler{"", &meta, &mockCloudwatch{}, logr.Discard()} - value, err := mockAWSCloudwatchScaler.GetMetrics(context.Background(), meta.metricsName) + value, _, err := mockAWSCloudwatchScaler.GetMetricsAndActivity(context.Background(), meta.metricsName) switch meta.metricsName { case testAWSCloudwatchErrorMetric: assert.Error(t, err, "expect error because of cloudwatch api error") diff --git a/pkg/scalers/aws_dynamodb_scaler.go b/pkg/scalers/aws_dynamodb_scaler.go index 935d4316e23..6a5a8022c40 100644 --- a/pkg/scalers/aws_dynamodb_scaler.go +++ b/pkg/scalers/aws_dynamodb_scaler.go @@ -151,16 +151,16 @@ func createDynamoDBClient(metadata *awsDynamoDBMetadata) *dynamodb.DynamoDB { return dynamodb.New(sess, config) } -func (s *awsDynamoDBScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *awsDynamoDBScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { metricValue, err := s.GetQueryMetrics() if err != nil { s.logger.Error(err, "Error getting metric value") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, metricValue) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, metricValue > float64(s.metadata.activationTargetValue), nil } func (s *awsDynamoDBScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { @@ -177,15 +177,6 @@ func (s *awsDynamoDBScaler) GetMetricSpecForScaling(context.Context) []v2.Metric } } -func (s *awsDynamoDBScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.GetQueryMetrics() - if err != nil { - return false, fmt.Errorf("error inspecting aws-dynamodb: %s", err) - } - - return messages > float64(s.metadata.activationTargetValue), nil -} - func (s *awsDynamoDBScaler) Close(context.Context) error { return nil } diff --git a/pkg/scalers/aws_dynamodb_scaler_test.go b/pkg/scalers/aws_dynamodb_scaler_test.go index 12ebc276544..b07ccad3684 100644 --- a/pkg/scalers/aws_dynamodb_scaler_test.go +++ b/pkg/scalers/aws_dynamodb_scaler_test.go @@ -305,7 +305,7 @@ func TestDynamoGetMetrics(t *testing.T) { t.Run(meta.tableName, func(t *testing.T) { scaler := awsDynamoDBScaler{"", &meta, &mockDynamoDB{}, logr.Discard()} - value, err := scaler.GetMetrics(context.Background(), "aws-dynamodb") + value, _, err := scaler.GetMetricsAndActivity(context.Background(), "aws-dynamodb") switch meta.tableName { case testAWSDynamoErrorTable: assert.Error(t, err, "expect error because of dynamodb api error") @@ -341,7 +341,7 @@ func TestDynamoIsActive(t *testing.T) { t.Run(meta.tableName, func(t *testing.T) { scaler := awsDynamoDBScaler{"", &meta, &mockDynamoDB{}, logr.Discard()} - value, err := scaler.IsActive(context.Background()) + _, value, err := scaler.GetMetricsAndActivity(context.Background(), "aws-dynamodb") switch meta.tableName { case testAWSDynamoErrorTable: assert.Error(t, err, "expect error because of cloudwatch api error") diff --git a/pkg/scalers/aws_dynamodb_streams_scaler.go b/pkg/scalers/aws_dynamodb_streams_scaler.go index 6f6c06d903c..8f2ae7dc525 100644 --- a/pkg/scalers/aws_dynamodb_streams_scaler.go +++ b/pkg/scalers/aws_dynamodb_streams_scaler.go @@ -11,8 +11,6 @@ import ( "github.com/aws/aws-sdk-go/service/dynamodbstreams/dynamodbstreamsiface" "github.com/go-logr/logr" v2 "k8s.io/api/autoscaling/v2" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/metrics/pkg/apis/external_metrics" kedautil "github.com/kedacore/keda/v2/pkg/util" @@ -148,15 +146,6 @@ func getDynamoDBStreamsArn(ctx context.Context, db dynamodbiface.DynamoDBAPI, ta return tableOutput.Table.LatestStreamArn, nil } -// IsActive determines if we need to scale from zero -func (s *awsDynamoDBStreamsScaler) IsActive(ctx context.Context) (bool, error) { - count, err := s.GetDynamoDBStreamShardCount(ctx) - if err != nil { - return false, err - } - return count > s.metadata.activationTargetShardCount, nil -} - func (s *awsDynamoDBStreamsScaler) Close(context.Context) error { return nil } @@ -172,22 +161,18 @@ func (s *awsDynamoDBStreamsScaler) GetMetricSpecForScaling(context.Context) []v2 return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *awsDynamoDBStreamsScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *awsDynamoDBStreamsScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { shardCount, err := s.GetDynamoDBStreamShardCount(ctx) if err != nil { s.logger.Error(err, "error getting shard count") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } - metric := external_metrics.ExternalMetricValue{ - MetricName: metricName, - Value: *resource.NewQuantity(shardCount, resource.DecimalSI), - Timestamp: metav1.Now(), - } + metric := GenerateMetricInMili(metricName, float64(shardCount)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, shardCount > s.metadata.activationTargetShardCount, nil } // Get DynamoDB Stream Shard Count diff --git a/pkg/scalers/aws_dynamodb_streams_scaler_test.go b/pkg/scalers/aws_dynamodb_streams_scaler_test.go index c9cf2fed967..c792c8f5299 100644 --- a/pkg/scalers/aws_dynamodb_streams_scaler_test.go +++ b/pkg/scalers/aws_dynamodb_streams_scaler_test.go @@ -421,7 +421,7 @@ func TestAwsDynamoDBStreamsScalerGetMetrics(t *testing.T) { streamArn, err = getDynamoDBStreamsArn(ctx, &mockAwsDynamoDB{}, &meta.tableName) if err == nil { scaler := awsDynamoDBStreamsScaler{"", meta, streamArn, &mockAwsDynamoDBStreams{}, logr.Discard()} - value, err = scaler.GetMetrics(context.Background(), "MetricName") + value, _, err = scaler.GetMetricsAndActivity(context.Background(), "MetricName") } switch meta.tableName { case testAWSDynamoDBErrorTable: @@ -445,7 +445,7 @@ func TestAwsDynamoDBStreamsScalerIsActive(t *testing.T) { streamArn, err = getDynamoDBStreamsArn(ctx, &mockAwsDynamoDB{}, &meta.tableName) if err == nil { scaler := awsDynamoDBStreamsScaler{"", meta, streamArn, &mockAwsDynamoDBStreams{}, logr.Discard()} - value, err = scaler.IsActive(context.Background()) + _, value, err = scaler.GetMetricsAndActivity(context.Background(), "MetricName") } switch meta.tableName { case testAWSDynamoDBErrorTable: diff --git a/pkg/scalers/aws_kinesis_stream_scaler.go b/pkg/scalers/aws_kinesis_stream_scaler.go index 99f877d4ca4..fdab9daee36 100644 --- a/pkg/scalers/aws_kinesis_stream_scaler.go +++ b/pkg/scalers/aws_kinesis_stream_scaler.go @@ -118,17 +118,6 @@ func createKinesisClient(metadata *awsKinesisStreamMetadata) *kinesis.Kinesis { return kinesis.New(sess, config) } -// IsActive determines if we need to scale from zero -func (s *awsKinesisStreamScaler) IsActive(ctx context.Context) (bool, error) { - count, err := s.GetAwsKinesisOpenShardCount() - - if err != nil { - return false, err - } - - return count > s.metadata.activationTargetShardCount, nil -} - func (s *awsKinesisStreamScaler) Close(context.Context) error { return nil } @@ -144,18 +133,18 @@ func (s *awsKinesisStreamScaler) GetMetricSpecForScaling(context.Context) []v2.M return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *awsKinesisStreamScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *awsKinesisStreamScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { shardCount, err := s.GetAwsKinesisOpenShardCount() if err != nil { s.logger.Error(err, "Error getting shard count") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(shardCount)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, shardCount > s.metadata.activationTargetShardCount, nil } // Get Kinesis open shard count diff --git a/pkg/scalers/aws_kinesis_stream_scaler_test.go b/pkg/scalers/aws_kinesis_stream_scaler_test.go index 2422308c309..f857d6701b8 100644 --- a/pkg/scalers/aws_kinesis_stream_scaler_test.go +++ b/pkg/scalers/aws_kinesis_stream_scaler_test.go @@ -351,7 +351,7 @@ func TestAWSKinesisGetMetricSpecForScaling(t *testing.T) { func TestAWSKinesisStreamScalerGetMetrics(t *testing.T) { for _, meta := range awsKinesisGetMetricTestData { scaler := awsKinesisStreamScaler{"", meta, &mockKinesis{}, logr.Discard()} - value, err := scaler.GetMetrics(context.Background(), "MetricName") + value, _, err := scaler.GetMetricsAndActivity(context.Background(), "MetricName") switch meta.streamName { case testAWSKinesisErrorStream: assert.Error(t, err, "expect error because of kinesis api error") diff --git a/pkg/scalers/aws_sqs_queue_scaler.go b/pkg/scalers/aws_sqs_queue_scaler.go index e585514b034..aa15f458148 100644 --- a/pkg/scalers/aws_sqs_queue_scaler.go +++ b/pkg/scalers/aws_sqs_queue_scaler.go @@ -166,17 +166,6 @@ func createSqsClient(metadata *awsSqsQueueMetadata) *sqs.SQS { return sqs.New(sess, config) } -// IsActive determines if we need to scale from zero -func (s *awsSqsQueueScaler) IsActive(ctx context.Context) (bool, error) { - length, err := s.getAwsSqsQueueLength() - - if err != nil { - return false, err - } - - return length > s.metadata.activationTargetQueueLength, nil -} - func (s *awsSqsQueueScaler) Close(context.Context) error { return nil } @@ -192,18 +181,18 @@ func (s *awsSqsQueueScaler) GetMetricSpecForScaling(context.Context) []v2.Metric return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *awsSqsQueueScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *awsSqsQueueScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { queuelen, err := s.getAwsSqsQueueLength() if err != nil { s.logger.Error(err, "Error getting queue length") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(queuelen)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, queuelen > s.metadata.activationTargetQueueLength, nil } // Get SQS Queue Length diff --git a/pkg/scalers/aws_sqs_queue_scaler_test.go b/pkg/scalers/aws_sqs_queue_scaler_test.go index 35eba831582..8f00bfeed7c 100644 --- a/pkg/scalers/aws_sqs_queue_scaler_test.go +++ b/pkg/scalers/aws_sqs_queue_scaler_test.go @@ -345,7 +345,7 @@ func TestAWSSQSGetMetricSpecForScaling(t *testing.T) { func TestAWSSQSScalerGetMetrics(t *testing.T) { for _, meta := range awsSQSGetMetricTestData { scaler := awsSqsQueueScaler{"", meta, &mockSqs{}, logr.Discard()} - value, err := scaler.GetMetrics(context.Background(), "MetricName") + value, _, err := scaler.GetMetricsAndActivity(context.Background(), "MetricName") switch meta.queueURL { case testAWSSQSErrorQueueURL: assert.Error(t, err, "expect error because of sqs api error") diff --git a/pkg/scalers/azure_app_insights_scaler.go b/pkg/scalers/azure_app_insights_scaler.go index 8d12a565897..b9acc3661e1 100644 --- a/pkg/scalers/azure_app_insights_scaler.go +++ b/pkg/scalers/azure_app_insights_scaler.go @@ -163,17 +163,6 @@ func parseAzureAppInsightsMetadata(config *ScalerConfig, logger logr.Logger) (*a return &meta, nil } -// Returns true if the Azure App Insights metric value is greater than the target value -func (s *azureAppInsightsScaler) IsActive(ctx context.Context) (bool, error) { - val, err := azure.GetAzureAppInsightsMetricValue(ctx, s.metadata.azureAppInsightsInfo, s.podIdentity) - if err != nil { - s.logger.Error(err, "error getting azure app insights metric") - return false, err - } - - return val > s.metadata.activationTargetValue, nil -} - func (s *azureAppInsightsScaler) Close(context.Context) error { return nil } @@ -190,14 +179,14 @@ func (s *azureAppInsightsScaler) GetMetricSpecForScaling(context.Context) []v2.M } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *azureAppInsightsScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *azureAppInsightsScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := azure.GetAzureAppInsightsMetricValue(ctx, s.metadata.azureAppInsightsInfo, s.podIdentity) if err != nil { s.logger.Error(err, "error getting azure app insights metric") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationTargetValue, nil } diff --git a/pkg/scalers/azure_blob_scaler.go b/pkg/scalers/azure_blob_scaler.go index 20913d40389..f67e589994c 100644 --- a/pkg/scalers/azure_blob_scaler.go +++ b/pkg/scalers/azure_blob_scaler.go @@ -180,23 +180,6 @@ func parseAzureBlobMetadata(config *ScalerConfig, logger logr.Logger) (*azure.Bl return &meta, config.PodIdentity, nil } -// GetScaleDecision is a func -func (s *azureBlobScaler) IsActive(ctx context.Context) (bool, error) { - length, err := azure.GetAzureBlobListLength( - ctx, - s.httpClient, - s.podIdentity, - s.metadata, - ) - - if err != nil { - s.logger.Error(err, "error") - return false, err - } - - return length > s.metadata.ActivationTargetBlobCount, nil -} - func (s *azureBlobScaler) Close(context.Context) error { return nil } @@ -212,8 +195,8 @@ func (s *azureBlobScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSp return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *azureBlobScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *azureBlobScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { bloblen, err := azure.GetAzureBlobListLength( ctx, s.httpClient, @@ -223,10 +206,10 @@ func (s *azureBlobScaler) GetMetrics(ctx context.Context, metricName string) ([] if err != nil { s.logger.Error(err, "error getting blob list length") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(bloblen)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, bloblen > s.metadata.ActivationTargetBlobCount, nil } diff --git a/pkg/scalers/azure_data_explorer_scaler.go b/pkg/scalers/azure_data_explorer_scaler.go index a95856b7f88..b75bd005b2b 100644 --- a/pkg/scalers/azure_data_explorer_scaler.go +++ b/pkg/scalers/azure_data_explorer_scaler.go @@ -169,14 +169,15 @@ func parseAzureDataExplorerAuthParams(config *ScalerConfig, logger logr.Logger) return &metadata, nil } -func (s azureDataExplorerScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s azureDataExplorerScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { metricValue, err := azure.GetAzureDataExplorerMetricValue(ctx, s.client, s.metadata.DatabaseName, s.metadata.Query) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("failed to get metrics for scaled object %s in namespace %s: %v", s.name, s.namespace, err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("failed to get metrics for scaled object %s in namespace %s: %v", s.name, s.namespace, err) } metric := GenerateMetricInMili(metricName, metricValue) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + + return []external_metrics.ExternalMetricValue{metric}, metricValue > s.metadata.ActivationThreshold, nil } func (s azureDataExplorerScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { @@ -190,15 +191,6 @@ func (s azureDataExplorerScaler) GetMetricSpecForScaling(context.Context) []v2.M return []v2.MetricSpec{metricSpec} } -func (s azureDataExplorerScaler) IsActive(ctx context.Context) (bool, error) { - metricValue, err := azure.GetAzureDataExplorerMetricValue(ctx, s.client, s.metadata.DatabaseName, s.metadata.Query) - if err != nil { - return false, fmt.Errorf("failed to get azure data explorer metric value: %s", err) - } - - return metricValue > s.metadata.ActivationThreshold, nil -} - func (s azureDataExplorerScaler) Close(context.Context) error { return nil } diff --git a/pkg/scalers/azure_eventhub_scaler.go b/pkg/scalers/azure_eventhub_scaler.go index 8c98835b0fe..3b137fefcc8 100644 --- a/pkg/scalers/azure_eventhub_scaler.go +++ b/pkg/scalers/azure_eventhub_scaler.go @@ -431,3 +431,20 @@ func (s *azureEventHubScaler) Close(ctx context.Context) error { return nil } + +// TODO merge isActive() and GetMetrics() +func (s *azureEventHubScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { + metrics, err := s.GetMetrics(ctx, metricName) + if err != nil { + s.logger.Error(err, "error getting metrics") + return []external_metrics.ExternalMetricValue{}, false, err + } + + isActive, err := s.IsActive(ctx) + if err != nil { + s.logger.Error(err, "error getting activity status") + return []external_metrics.ExternalMetricValue{}, false, err + } + + return metrics, isActive, nil +} diff --git a/pkg/scalers/azure_log_analytics_scaler.go b/pkg/scalers/azure_log_analytics_scaler.go index e055e470feb..91cdb9d9cfa 100644 --- a/pkg/scalers/azure_log_analytics_scaler.go +++ b/pkg/scalers/azure_log_analytics_scaler.go @@ -50,7 +50,6 @@ const ( type azureLogAnalyticsScaler struct { metricType v2.MetricTargetType metadata *azureLogAnalyticsMetadata - cache *sessionCache name string namespace string httpClient *http.Client @@ -72,11 +71,6 @@ type azureLogAnalyticsMetadata struct { activeDirectoryEndpoint string } -type sessionCache struct { - metricValue float64 - metricThreshold float64 -} - type tokenData struct { TokenType string `json:"token_type"` ExpiresIn int `json:"expires_in,string"` @@ -130,7 +124,6 @@ func NewAzureLogAnalyticsScaler(config *ScalerConfig) (Scaler, error) { return &azureLogAnalyticsScaler{ metricType: metricType, metadata: azureLogAnalyticsMetadata, - cache: &sessionCache{metricValue: -1, metricThreshold: -1}, name: config.ScalableObjectName, namespace: config.ScalableObjectNamespace, httpClient: kedautil.CreateHTTPClient(config.GlobalHTTPTimeout, false), @@ -252,72 +245,34 @@ func getParameterFromConfig(config *ScalerConfig, parameter string, checkAuthPar return "", fmt.Errorf("error parsing metadata. Details: %s was not found in metadata. Check your ScaledObject configuration", parameter) } -// IsActive determines if we need to scale from zero -func (s *azureLogAnalyticsScaler) IsActive(ctx context.Context) (bool, error) { - err := s.updateCache(ctx) - - if err != nil { - return false, fmt.Errorf("failed to execute IsActive function. Scaled object: %s. Namespace: %s. Inner Error: %v", s.name, s.namespace, err) - } - - return s.cache.metricValue > s.metadata.activationThreshold, nil -} - func (s *azureLogAnalyticsScaler) GetMetricSpecForScaling(ctx context.Context) []v2.MetricSpec { - err := s.updateCache(ctx) - - if err != nil { - s.logger.V(1).Info("failed to get metric spec.", "Scaled object", s.name, "Namespace", s.namespace, "Inner Error", err) - return nil - } - externalMetric := &v2.ExternalMetricSource{ Metric: v2.MetricIdentifier{ Name: GenerateMetricNameWithIndex(s.metadata.scalerIndex, s.metadata.metricName), }, - Target: GetMetricTargetMili(s.metricType, s.cache.metricThreshold), + Target: GetMetricTargetMili(s.metricType, s.metadata.threshold), } metricSpec := v2.MetricSpec{External: externalMetric, Type: externalMetricType} return []v2.MetricSpec{metricSpec} } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *azureLogAnalyticsScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *azureLogAnalyticsScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { receivedMetric, err := s.getMetricData(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("failed to get metrics. Scaled object: %s. Namespace: %s. Inner Error: %v", s.name, s.namespace, err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("failed to get metrics. Scaled object: %s. Namespace: %s. Inner Error: %v", s.name, s.namespace, err) } metric := GenerateMetricInMili(metricName, receivedMetric.value) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, receivedMetric.value > s.metadata.activationThreshold, nil } func (s *azureLogAnalyticsScaler) Close(context.Context) error { return nil } -func (s *azureLogAnalyticsScaler) updateCache(ctx context.Context) error { - if s.cache.metricValue < 0 { - receivedMetric, err := s.getMetricData(ctx) - - if err != nil { - return err - } - - s.cache.metricValue = receivedMetric.value - - if receivedMetric.threshold > 0 { - s.cache.metricThreshold = receivedMetric.threshold - } else { - s.cache.metricThreshold = s.metadata.threshold - } - } - - return nil -} - func (s *azureLogAnalyticsScaler) getMetricData(ctx context.Context) (metricsData, error) { tokenInfo, err := s.getAccessToken(ctx) if err != nil { diff --git a/pkg/scalers/azure_log_analytics_scaler_test.go b/pkg/scalers/azure_log_analytics_scaler_test.go index c17be86921b..4ae0cf6224a 100644 --- a/pkg/scalers/azure_log_analytics_scaler_test.go +++ b/pkg/scalers/azure_log_analytics_scaler_test.go @@ -198,10 +198,8 @@ func TestLogAnalyticsGetMetricSpecForScaling(t *testing.T) { if err != nil { t.Fatal("Could not parse metadata:", err) } - cache := &sessionCache{metricValue: 1, metricThreshold: 2} mockLogAnalyticsScaler := azureLogAnalyticsScaler{ metadata: meta, - cache: cache, name: "test-so", namespace: "test-ns", httpClient: http.DefaultClient, diff --git a/pkg/scalers/azure_monitor_scaler.go b/pkg/scalers/azure_monitor_scaler.go index fdb75607a16..1de83010d6a 100644 --- a/pkg/scalers/azure_monitor_scaler.go +++ b/pkg/scalers/azure_monitor_scaler.go @@ -213,17 +213,6 @@ func parseAzurePodIdentityParams(config *ScalerConfig) (clientID string, clientP return clientID, clientPassword, nil } -// Returns true if the Azure Monitor metric value is greater than zero -func (s *azureMonitorScaler) IsActive(ctx context.Context) (bool, error) { - val, err := azure.GetAzureMetricValue(ctx, s.metadata.azureMonitorInfo, s.podIdentity) - if err != nil { - s.logger.Error(err, "error getting azure monitor metric") - return false, err - } - - return val > s.metadata.activationTargetValue, nil -} - func (s *azureMonitorScaler) Close(context.Context) error { return nil } @@ -239,15 +228,15 @@ func (s *azureMonitorScaler) GetMetricSpecForScaling(context.Context) []v2.Metri return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *azureMonitorScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *azureMonitorScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := azure.GetAzureMetricValue(ctx, s.metadata.azureMonitorInfo, s.podIdentity) if err != nil { s.logger.Error(err, "error getting azure monitor metric") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationTargetValue, nil } diff --git a/pkg/scalers/azure_pipelines_scaler.go b/pkg/scalers/azure_pipelines_scaler.go index 7ec6eed649f..3112f6e8236 100644 --- a/pkg/scalers/azure_pipelines_scaler.go +++ b/pkg/scalers/azure_pipelines_scaler.go @@ -309,19 +309,6 @@ func getAzurePipelineRequest(ctx context.Context, url string, metadata *azurePip return b, nil } -func (s *azurePipelinesScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { - queuelen, err := s.GetAzurePipelinesQueueLength(ctx) - - if err != nil { - s.logger.Error(err, "error getting pipelines queue length") - return []external_metrics.ExternalMetricValue{}, err - } - - metric := GenerateMetricInMili(metricName, float64(queuelen)) - - return append([]external_metrics.ExternalMetricValue{}, metric), nil -} - func (s *azurePipelinesScaler) GetAzurePipelinesQueueLength(ctx context.Context) (int64, error) { url := fmt.Sprintf("%s/_apis/distributedtask/pools/%d/jobrequests", s.metadata.organizationURL, s.metadata.poolID) body, err := getAzurePipelineRequest(ctx, url, s.metadata, s.httpClient) @@ -416,15 +403,17 @@ func (s *azurePipelinesScaler) GetMetricSpecForScaling(context.Context) []v2.Met return []v2.MetricSpec{metricSpec} } -func (s *azurePipelinesScaler) IsActive(ctx context.Context) (bool, error) { +func (s *azurePipelinesScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { queuelen, err := s.GetAzurePipelinesQueueLength(ctx) if err != nil { - s.logger.Error(err, "error)") - return false, err + s.logger.Error(err, "error getting pipelines queue length") + return []external_metrics.ExternalMetricValue{}, false, err } - return queuelen > s.metadata.activationTargetPipelinesQueueLength, nil + metric := GenerateMetricInMili(metricName, float64(queuelen)) + + return []external_metrics.ExternalMetricValue{metric}, queuelen > s.metadata.activationTargetPipelinesQueueLength, nil } func (s *azurePipelinesScaler) Close(context.Context) error { diff --git a/pkg/scalers/azure_queue_scaler.go b/pkg/scalers/azure_queue_scaler.go index 307836edaa6..6b208750761 100644 --- a/pkg/scalers/azure_queue_scaler.go +++ b/pkg/scalers/azure_queue_scaler.go @@ -158,26 +158,6 @@ func parseAzureQueueMetadata(config *ScalerConfig, logger logr.Logger) (*azureQu return &meta, config.PodIdentity, nil } -// IsActive determines whether this scaler is currently active -func (s *azureQueueScaler) IsActive(ctx context.Context) (bool, error) { - length, err := azure.GetAzureQueueLength( - ctx, - s.httpClient, - s.podIdentity, - s.metadata.connection, - s.metadata.queueName, - s.metadata.accountName, - s.metadata.endpointSuffix, - ) - - if err != nil { - s.logger.Error(err, "error)") - return false, err - } - - return length > s.metadata.activationTargetQueueLength, nil -} - func (s *azureQueueScaler) Close(context.Context) error { return nil } @@ -193,8 +173,8 @@ func (s *azureQueueScaler) GetMetricSpecForScaling(context.Context) []v2.MetricS return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *azureQueueScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *azureQueueScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { queuelen, err := azure.GetAzureQueueLength( ctx, s.httpClient, @@ -207,10 +187,10 @@ func (s *azureQueueScaler) GetMetrics(ctx context.Context, metricName string) ([ if err != nil { s.logger.Error(err, "error getting queue length") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(queuelen)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, queuelen > s.metadata.activationTargetQueueLength, nil } diff --git a/pkg/scalers/azure_servicebus_scaler.go b/pkg/scalers/azure_servicebus_scaler.go index 590624de52a..b8da096a39c 100755 --- a/pkg/scalers/azure_servicebus_scaler.go +++ b/pkg/scalers/azure_servicebus_scaler.go @@ -226,17 +226,6 @@ func parseAzureServiceBusMetadata(config *ScalerConfig, logger logr.Logger) (*az return &meta, nil } -// Returns true if the scaler's queue has messages in it, false otherwise -func (s *azureServiceBusScaler) IsActive(ctx context.Context) (bool, error) { - length, err := s.getAzureServiceBusLength(ctx) - if err != nil { - s.logger.Error(err, "error") - return false, err - } - - return length > s.metadata.activationTargetLength, nil -} - // Close - nothing to close for SB func (s *azureServiceBusScaler) Close(context.Context) error { return nil @@ -269,18 +258,18 @@ func (s *azureServiceBusScaler) GetMetricSpecForScaling(context.Context) []v2.Me return []v2.MetricSpec{metricSpec} } -// Returns the current metrics to be served to the HPA -func (s *azureServiceBusScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns the current metrics to be served to the HPA +func (s *azureServiceBusScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { queuelen, err := s.getAzureServiceBusLength(ctx) if err != nil { s.logger.Error(err, "error getting service bus entity length") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(queuelen)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, queuelen > s.metadata.activationTargetLength, nil } // Returns the length of the queue or subscription diff --git a/pkg/scalers/cassandra_scaler.go b/pkg/scalers/cassandra_scaler.go index 88f4776c749..2ca237366c9 100644 --- a/pkg/scalers/cassandra_scaler.go +++ b/pkg/scalers/cassandra_scaler.go @@ -179,16 +179,6 @@ func newCassandraSession(meta *CassandraMetadata, logger logr.Logger) (*gocql.Se return session, nil } -// IsActive returns true if there are pending events to be processed. -func (s *cassandraScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.GetQueryResult(ctx) - if err != nil { - return false, fmt.Errorf("error inspecting cassandra: %s", err) - } - - return messages > s.metadata.activationTargetQueryValue, nil -} - // GetMetricSpecForScaling returns the MetricSpec for the Horizontal Pod Autoscaler. func (s *cassandraScaler) GetMetricSpecForScaling(ctx context.Context) []v2.MetricSpec { externalMetric := &v2.ExternalMetricSource{ @@ -204,16 +194,16 @@ func (s *cassandraScaler) GetMetricSpecForScaling(ctx context.Context) []v2.Metr return []v2.MetricSpec{metricSpec} } -// GetMetrics returns a value for a supported metric or an error if there is a problem getting the metric. -func (s *cassandraScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns a value for a supported metric or an error if there is a problem getting the metric. +func (s *cassandraScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.GetQueryResult(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting cassandra: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting cassandra: %s", err) } metric := GenerateMetricInMili(metricName, float64(num)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationTargetQueryValue, nil } // GetQueryResult returns the result of the scaler query. diff --git a/pkg/scalers/cpu_memory_scaler.go b/pkg/scalers/cpu_memory_scaler.go index cfd7db7c84b..fef14738aeb 100644 --- a/pkg/scalers/cpu_memory_scaler.go +++ b/pkg/scalers/cpu_memory_scaler.go @@ -81,11 +81,6 @@ func parseResourceMetadata(config *ScalerConfig, logger logr.Logger) (*cpuMemory return meta, nil } -// IsActive always return true for cpu/memory scaler -func (s *cpuMemoryScaler) IsActive(ctx context.Context) (bool, error) { - return true, nil -} - // Close no need for cpuMemory scaler func (s *cpuMemoryScaler) Close(context.Context) error { return nil @@ -121,7 +116,7 @@ func (s *cpuMemoryScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSp return []v2.MetricSpec{metricSpec} } -// GetMetrics no need for cpu/memory scaler -func (s *cpuMemoryScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { - return nil, nil +// GetMetrics no need for cpu/memory scaler and always active for cpu/memory scaler +func (s *cpuMemoryScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { + return nil, true, nil } diff --git a/pkg/scalers/cron_scaler.go b/pkg/scalers/cron_scaler.go index df48185b923..9c508e68862 100644 --- a/pkg/scalers/cron_scaler.go +++ b/pkg/scalers/cron_scaler.go @@ -183,5 +183,22 @@ func (s *cronScaler) GetMetrics(ctx context.Context, metricName string) ([]exter /*******************************************************************************/ metric := GenerateMetricInMili(metricName, float64(currentReplicas)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, nil +} + +// TODO merge isActive() and GetMetrics() +func (s *cronScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { + metrics, err := s.GetMetrics(ctx, metricName) + if err != nil { + s.logger.Error(err, "error getting metrics") + return []external_metrics.ExternalMetricValue{}, false, err + } + + isActive, err := s.IsActive(ctx) + if err != nil { + s.logger.Error(err, "error getting activity status") + return []external_metrics.ExternalMetricValue{}, false, err + } + + return metrics, isActive, nil } diff --git a/pkg/scalers/cron_scaler_test.go b/pkg/scalers/cron_scaler_test.go index 33123800afb..b0643186d65 100644 --- a/pkg/scalers/cron_scaler_test.go +++ b/pkg/scalers/cron_scaler_test.go @@ -73,7 +73,7 @@ func TestCronParseMetadata(t *testing.T) { func TestIsActive(t *testing.T) { scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata}) - isActive, _ := scaler.IsActive(context.TODO()) + _, isActive, _ := scaler.GetMetricsAndActivity(context.TODO(), "ReplicaCount") if currentDay == "Thursday" { assert.Equal(t, isActive, true) } else { @@ -83,7 +83,7 @@ func TestIsActive(t *testing.T) { func TestIsActiveRange(t *testing.T) { scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata2}) - isActive, _ := scaler.IsActive(context.TODO()) + _, isActive, _ := scaler.GetMetricsAndActivity(context.TODO(), "ReplicaCount") if currentHour%2 == 0 { assert.Equal(t, isActive, true) } else { @@ -93,7 +93,7 @@ func TestIsActiveRange(t *testing.T) { func TestGetMetrics(t *testing.T) { scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata}) - metrics, _ := scaler.GetMetrics(context.TODO(), "ReplicaCount") + metrics, _, _ := scaler.GetMetricsAndActivity(context.TODO(), "ReplicaCount") assert.Equal(t, metrics[0].MetricName, "ReplicaCount") if currentDay == "Thursday" { assert.Equal(t, metrics[0].Value.Value(), int64(10)) @@ -104,7 +104,7 @@ func TestGetMetrics(t *testing.T) { func TestGetMetricsRange(t *testing.T) { scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata2}) - metrics, _ := scaler.GetMetrics(context.TODO(), "ReplicaCount") + metrics, _, _ := scaler.GetMetricsAndActivity(context.TODO(), "ReplicaCount") assert.Equal(t, metrics[0].MetricName, "ReplicaCount") if currentHour%2 == 0 { assert.Equal(t, metrics[0].Value.Value(), int64(10)) diff --git a/pkg/scalers/datadog_scaler.go b/pkg/scalers/datadog_scaler.go index ffb6940f10c..1e5ccd7d8ca 100644 --- a/pkg/scalers/datadog_scaler.go +++ b/pkg/scalers/datadog_scaler.go @@ -230,17 +230,6 @@ func (s *datadogScaler) Close(context.Context) error { return nil } -// IsActive checks whether the scaler is active -func (s *datadogScaler) IsActive(ctx context.Context) (bool, error) { - num, err := s.getQueryResult(ctx) - - if err != nil { - return false, err - } - - return num > s.metadata.activationQueryValue, nil -} - // getQueryResult returns result of the scaler query func (s *datadogScaler) getQueryResult(ctx context.Context) (float64, error) { ctx = context.WithValue( @@ -338,16 +327,16 @@ func (s *datadogScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *datadogScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *datadogScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.getQueryResult(ctx) if err != nil { s.logger.Error(err, "error getting metrics from Datadog") - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error getting metrics from Datadog: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error getting metrics from Datadog: %s", err) } metric := GenerateMetricInMili(metricName, num) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationQueryValue, nil } // Find the largest value in a slice of floats diff --git a/pkg/scalers/elasticsearch_scaler.go b/pkg/scalers/elasticsearch_scaler.go index c2aeb57af3e..e8949bfe5d1 100644 --- a/pkg/scalers/elasticsearch_scaler.go +++ b/pkg/scalers/elasticsearch_scaler.go @@ -251,16 +251,6 @@ func (s *elasticsearchScaler) Close(ctx context.Context) error { return nil } -// IsActive returns true if there are pending messages to be processed -func (s *elasticsearchScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.getQueryResult(ctx) - if err != nil { - s.logger.Error(err, fmt.Sprintf("Error inspecting elasticsearch: %s", err)) - return false, err - } - return messages > s.metadata.activationTargetValue, nil -} - // getQueryResult returns result of the scaler query func (s *elasticsearchScaler) getQueryResult(ctx context.Context) (float64, error) { // Build the request body. @@ -340,15 +330,15 @@ func (s *elasticsearchScaler) GetMetricSpecForScaling(context.Context) []v2.Metr } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *elasticsearchScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *elasticsearchScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.getQueryResult(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting elasticsearch: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting elasticsearch: %s", err) } metric := GenerateMetricInMili(metricName, num) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationTargetValue, nil } // Splits a string separated by a specified separator and trims space from all the elements. diff --git a/pkg/scalers/external_mock_scaler.go b/pkg/scalers/external_mock_scaler.go index 2e880e1b103..02f8091479b 100644 --- a/pkg/scalers/external_mock_scaler.go +++ b/pkg/scalers/external_mock_scaler.go @@ -30,15 +30,6 @@ func NewExternalMockScaler(config *ScalerConfig) (Scaler, error) { return &externalMockScaler{}, nil } -// IsActive implements Scaler -func (*externalMockScaler) IsActive(ctx context.Context) (bool, error) { - if atomic.LoadInt32(&MockExternalServerStatus) != MockExternalServerStatusOnline { - return false, ErrMock - } - - return true, nil -} - // Close implements Scaler func (*externalMockScaler) Close(ctx context.Context) error { return nil @@ -54,12 +45,12 @@ func (*externalMockScaler) GetMetricSpecForScaling(ctx context.Context) []v2.Met } // GetMetrics implements Scaler -func (*externalMockScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (*externalMockScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { if atomic.LoadInt32(&MockExternalServerStatus) != MockExternalServerStatusOnline { - return nil, ErrMock + return nil, false, ErrMock } - return getMockExternalMetricsValue(), nil + return getMockExternalMetricsValue(), true, nil } func getMockMetricsSpecs() []v2.MetricSpec { diff --git a/pkg/scalers/external_scaler.go b/pkg/scalers/external_scaler.go index 2b83ee26cac..b9fb43988c2 100644 --- a/pkg/scalers/external_scaler.go +++ b/pkg/scalers/external_scaler.go @@ -217,6 +217,23 @@ func (s *externalScaler) GetMetrics(ctx context.Context, metricName string) ([]e return metrics, nil } +// TODO merge isActive() and GetMetrics() +func (s *externalScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { + metrics, err := s.GetMetrics(ctx, metricName) + if err != nil { + s.logger.Error(err, "error getting metrics") + return []external_metrics.ExternalMetricValue{}, false, err + } + + isActive, err := s.IsActive(ctx) + if err != nil { + s.logger.Error(err, "error getting activity status") + return []external_metrics.ExternalMetricValue{}, false, err + } + + return metrics, isActive, nil +} + // handleIsActiveStream is the only writer to the active channel and will close it on return. func (s *externalPushScaler) Run(ctx context.Context, active chan<- bool) { defer close(active) diff --git a/pkg/scalers/gcp_pubsub_scaler.go b/pkg/scalers/gcp_pubsub_scaler.go index 1e9571f092c..bbdf6d469a2 100644 --- a/pkg/scalers/gcp_pubsub_scaler.go +++ b/pkg/scalers/gcp_pubsub_scaler.go @@ -135,28 +135,6 @@ func parsePubSubMetadata(config *ScalerConfig, logger logr.Logger) (*pubsubMetad return &meta, nil } -// IsActive checks if there are any messages in the subscription -func (s *pubsubScaler) IsActive(ctx context.Context) (bool, error) { - switch s.metadata.mode { - case pubsubModeSubscriptionSize: - size, err := s.getMetrics(ctx, pubSubStackDriverSubscriptionSizeMetricName) - if err != nil { - s.logger.Error(err, "error getting Active Status") - return false, err - } - return size > s.metadata.activationValue, nil - case pubsubModeOldestUnackedMessageAge: - delay, err := s.getMetrics(ctx, pubSubStackDriverOldestUnackedMessageAgeMetricName) - if err != nil { - s.logger.Error(err, "error getting Active Status") - return false, err - } - return delay > s.metadata.activationValue, nil - default: - return false, errors.New("unknown mode") - } -} - func (s *pubsubScaler) Close(context.Context) error { if s.client != nil { err := s.client.metricsClient.Close() @@ -188,7 +166,7 @@ func (s *pubsubScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec } // GetMetrics connects to Stack Driver and finds the size of the pub sub subscription -func (s *pubsubScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *pubsubScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { var value int64 var err error @@ -197,19 +175,19 @@ func (s *pubsubScaler) GetMetrics(ctx context.Context, metricName string) ([]ext value, err = s.getMetrics(ctx, pubSubStackDriverSubscriptionSizeMetricName) if err != nil { s.logger.Error(err, "error getting subscription size") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } case pubsubModeOldestUnackedMessageAge: value, err = s.getMetrics(ctx, pubSubStackDriverOldestUnackedMessageAgeMetricName) if err != nil { s.logger.Error(err, "error getting oldest unacked message age") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } } metric := GenerateMetricInMili(metricName, float64(value)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, value > s.metadata.activationValue, nil } func (s *pubsubScaler) setStackdriverClient(ctx context.Context) error { diff --git a/pkg/scalers/gcp_stackdriver_scaler.go b/pkg/scalers/gcp_stackdriver_scaler.go index efb6e70593e..1567a06454c 100644 --- a/pkg/scalers/gcp_stackdriver_scaler.go +++ b/pkg/scalers/gcp_stackdriver_scaler.go @@ -161,15 +161,6 @@ func initializeStackdriverClient(ctx context.Context, gcpAuthorization *gcpAutho return client, nil } -func (s *stackdriverScaler) IsActive(ctx context.Context) (bool, error) { - value, err := s.getMetrics(ctx) - if err != nil { - s.logger.Error(err, "error getting metric value") - return false, err - } - return value > s.metadata.activationTargetValue, nil -} - func (s *stackdriverScaler) Close(context.Context) error { if s.client != nil { err := s.client.metricsClient.Close() @@ -201,16 +192,16 @@ func (s *stackdriverScaler) GetMetricSpecForScaling(context.Context) []v2.Metric } // GetMetrics connects to Stack Driver and retrieves the metric -func (s *stackdriverScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *stackdriverScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { value, err := s.getMetrics(ctx) if err != nil { s.logger.Error(err, "error getting metric value") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(value)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, value > s.metadata.activationTargetValue, nil } // getMetrics gets metric type value from stackdriver api diff --git a/pkg/scalers/gcp_storage_scaler.go b/pkg/scalers/gcp_storage_scaler.go index 5600fc9a2ac..5efe3e77c6a 100644 --- a/pkg/scalers/gcp_storage_scaler.go +++ b/pkg/scalers/gcp_storage_scaler.go @@ -157,16 +157,6 @@ func parseGcsMetadata(config *ScalerConfig, logger logr.Logger) (*gcsMetadata, e return &meta, nil } -// IsActive checks if there are any messages in the subscription -func (s *gcsScaler) IsActive(ctx context.Context) (bool, error) { - items, err := s.getItemCount(ctx, s.metadata.activationTargetObjectCount+1) - if err != nil { - return false, err - } - - return items > s.metadata.activationTargetObjectCount, nil -} - func (s *gcsScaler) Close(context.Context) error { if s.client != nil { return s.client.Close() @@ -186,16 +176,16 @@ func (s *gcsScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { return []v2.MetricSpec{metricSpec} } -// GetMetrics returns the number of items in the bucket (up to s.metadata.maxBucketItemsToScan) -func (s *gcsScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns the number of items in the bucket (up to s.metadata.maxBucketItemsToScan) +func (s *gcsScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { items, err := s.getItemCount(ctx, s.metadata.maxBucketItemsToScan) if err != nil { - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(items)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, items > s.metadata.activationTargetObjectCount, nil } // getItemCount gets the number of items in the bucket, up to maxCount diff --git a/pkg/scalers/graphite_scaler.go b/pkg/scalers/graphite_scaler.go index 0c14982016f..f9e88620c7e 100644 --- a/pkg/scalers/graphite_scaler.go +++ b/pkg/scalers/graphite_scaler.go @@ -148,16 +148,6 @@ func parseGraphiteMetadata(config *ScalerConfig) (*graphiteMetadata, error) { return &meta, nil } -func (s *graphiteScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.executeGrapQuery(ctx) - if err != nil { - s.logger.Error(err, "error executing graphite query") - return false, err - } - - return val > s.metadata.activationThreshold, nil -} - func (s *graphiteScaler) Close(context.Context) error { return nil } @@ -223,14 +213,14 @@ func (s *graphiteScaler) executeGrapQuery(ctx context.Context) (float64, error) return -1, fmt.Errorf("no valid non-null response in query %s, try increasing your queryTime or check your query", s.metadata.query) } -func (s *graphiteScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *graphiteScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := s.executeGrapQuery(ctx) if err != nil { s.logger.Error(err, "error executing graphite query") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil } diff --git a/pkg/scalers/huawei_cloudeye_scaler.go b/pkg/scalers/huawei_cloudeye_scaler.go index f80bcc629cc..be9c4810b78 100644 --- a/pkg/scalers/huawei_cloudeye_scaler.go +++ b/pkg/scalers/huawei_cloudeye_scaler.go @@ -240,16 +240,16 @@ func gethuaweiAuthorization(authParams map[string]string) (huaweiAuthorizationMe return meta, nil } -func (s *huaweiCloudeyeScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *huaweiCloudeyeScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { metricValue, err := s.GetCloudeyeMetrics() if err != nil { s.logger.Error(err, "Error getting metric value") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, metricValue) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, metricValue > s.metadata.activationTargetMetricValue, nil } func (s *huaweiCloudeyeScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { @@ -263,16 +263,6 @@ func (s *huaweiCloudeyeScaler) GetMetricSpecForScaling(context.Context) []v2.Met return []v2.MetricSpec{metricSpec} } -func (s *huaweiCloudeyeScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.GetCloudeyeMetrics() - - if err != nil { - return false, err - } - - return val > s.metadata.activationTargetMetricValue, nil -} - func (s *huaweiCloudeyeScaler) Close(context.Context) error { return nil } diff --git a/pkg/scalers/ibmmq_scaler.go b/pkg/scalers/ibmmq_scaler.go index c6767d31b59..9176e27fa18 100644 --- a/pkg/scalers/ibmmq_scaler.go +++ b/pkg/scalers/ibmmq_scaler.go @@ -164,15 +164,6 @@ func parseIBMMQMetadata(config *ScalerConfig) (*IBMMQMetadata, error) { return &meta, nil } -// IsActive returns true if there are messages to be processed/if we need to scale from zero -func (s *IBMMQScaler) IsActive(ctx context.Context) (bool, error) { - queueDepth, err := s.getQueueDepthViaHTTP(ctx) - if err != nil { - return false, fmt.Errorf("error inspecting IBM MQ queue depth: %s", err) - } - return queueDepth > s.metadata.activationQueueDepth, nil -} - // getQueueDepthViaHTTP returns the depth of the MQ Queue from the Admin endpoint func (s *IBMMQScaler) getQueueDepthViaHTTP(ctx context.Context) (int64, error) { queue := s.metadata.queueName @@ -229,13 +220,13 @@ func (s *IBMMQScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *IBMMQScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *IBMMQScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { queueDepth, err := s.getQueueDepthViaHTTP(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting IBM MQ queue depth: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting IBM MQ queue depth: %s", err) } metric := GenerateMetricInMili(metricName, float64(queueDepth)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, queueDepth > s.metadata.activationQueueDepth, nil } diff --git a/pkg/scalers/influxdb_scaler.go b/pkg/scalers/influxdb_scaler.go index cd1f48b8c5d..bb0c2d6dbd0 100644 --- a/pkg/scalers/influxdb_scaler.go +++ b/pkg/scalers/influxdb_scaler.go @@ -164,18 +164,6 @@ func parseInfluxDBMetadata(config *ScalerConfig) (*influxDBMetadata, error) { }, nil } -// IsActive returns true if queried value is above the minimum value -func (s *influxDBScaler) IsActive(ctx context.Context) (bool, error) { - queryAPI := s.client.QueryAPI(s.metadata.organizationName) - - value, err := queryInfluxDB(ctx, queryAPI, s.metadata.query) - if err != nil { - return false, err - } - - return value > s.metadata.activationThresholdValue, nil -} - // Close closes the connection of the client to the server func (s *influxDBScaler) Close(context.Context) error { s.client.Close() @@ -207,18 +195,18 @@ func queryInfluxDB(ctx context.Context, queryAPI api.QueryAPI, query string) (fl } // GetMetrics connects to influxdb via the client and returns a value based on the query -func (s *influxDBScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *influxDBScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { // Grab QueryAPI to make queries to influxdb instance queryAPI := s.client.QueryAPI(s.metadata.organizationName) value, err := queryInfluxDB(ctx, queryAPI, s.metadata.query) if err != nil { - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, value) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, value > s.metadata.activationThresholdValue, nil } // GetMetricSpecForScaling returns the metric spec for the Horizontal Pod Autoscaler diff --git a/pkg/scalers/kafka_scaler.go b/pkg/scalers/kafka_scaler.go index 09fdb684030..fb8ee261871 100644 --- a/pkg/scalers/kafka_scaler.go +++ b/pkg/scalers/kafka_scaler.go @@ -276,16 +276,6 @@ func parseKafkaMetadata(config *ScalerConfig, logger logr.Logger) (kafkaMetadata return meta, nil } -// IsActive determines if we need to scale from zero -func (s *kafkaScaler) IsActive(ctx context.Context) (bool, error) { - totalLag, err := s.getTotalLag() - if err != nil { - return false, err - } - - return totalLag > s.metadata.activationLagThreshold, nil -} - func getKafkaClients(metadata kafkaMetadata) (sarama.Client, sarama.ClusterAdmin, error) { config := sarama.NewConfig() config.Version = metadata.version @@ -500,14 +490,14 @@ func (s *kafkaScaler) getConsumerAndProducerOffsets(topicPartitions map[string][ } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *kafkaScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *kafkaScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { totalLag, err := s.getTotalLag() if err != nil { - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(totalLag)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, totalLag > s.metadata.activationLagThreshold, nil } func (s *kafkaScaler) getTotalLag() (int64, error) { diff --git a/pkg/scalers/kubernetes_workload_scaler.go b/pkg/scalers/kubernetes_workload_scaler.go index 07a7812655c..a60f6a2eca3 100644 --- a/pkg/scalers/kubernetes_workload_scaler.go +++ b/pkg/scalers/kubernetes_workload_scaler.go @@ -87,17 +87,6 @@ func parseWorkloadMetadata(config *ScalerConfig) (*kubernetesWorkloadMetadata, e return meta, nil } -// IsActive determines if we need to scale from zero -func (s *kubernetesWorkloadScaler) IsActive(ctx context.Context) (bool, error) { - pods, err := s.getMetricValue(ctx) - - if err != nil { - return false, err - } - - return float64(pods) > s.metadata.activationValue, nil -} - // Close no need for kubernetes workload scaler func (s *kubernetesWorkloadScaler) Close(context.Context) error { return nil @@ -115,16 +104,16 @@ func (s *kubernetesWorkloadScaler) GetMetricSpecForScaling(context.Context) []v2 return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric -func (s *kubernetesWorkloadScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric +func (s *kubernetesWorkloadScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { pods, err := s.getMetricValue(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting kubernetes workload: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting kubernetes workload: %s", err) } metric := GenerateMetricInMili(metricName, float64(pods)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, float64(pods) > s.metadata.activationValue, nil } func (s *kubernetesWorkloadScaler) getMetricValue(ctx context.Context) (int64, error) { diff --git a/pkg/scalers/kubernetes_workload_scaler_test.go b/pkg/scalers/kubernetes_workload_scaler_test.go index d7e26e4da05..a015a77c3ba 100644 --- a/pkg/scalers/kubernetes_workload_scaler_test.go +++ b/pkg/scalers/kubernetes_workload_scaler_test.go @@ -75,7 +75,7 @@ func TestWorkloadIsActive(t *testing.T) { ScalableObjectNamespace: testData.namespace, }, ) - isActive, _ := s.IsActive(context.TODO()) + _, isActive, _ := s.GetMetricsAndActivity(context.TODO(), "Metric") if testData.active && !isActive { t.Error("Expected active but got inactive") } @@ -183,7 +183,7 @@ func TestWorkloadPhase(t *testing.T) { if err != nil { t.Errorf("Failed to create test scaler -- %v", err) } - isActive, err := s.IsActive(context.TODO()) + _, isActive, err := s.GetMetricsAndActivity(context.TODO(), "Metric") if err != nil { t.Errorf("Failed to count active -- %v", err) } diff --git a/pkg/scalers/liiklus/LiiklusService.pb.go b/pkg/scalers/liiklus/LiiklusService.pb.go index 18a9b752e4a..b101d7cc6fc 100644 --- a/pkg/scalers/liiklus/LiiklusService.pb.go +++ b/pkg/scalers/liiklus/LiiklusService.pb.go @@ -7,10 +7,10 @@ package liiklus import ( - empty "github.com/golang/protobuf/ptypes/empty" - timestamp "github.com/golang/protobuf/ptypes/timestamp" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" + emptypb "google.golang.org/protobuf/types/known/emptypb" + timestamppb "google.golang.org/protobuf/types/known/timestamppb" reflect "reflect" sync "sync" ) @@ -326,6 +326,7 @@ type SubscribeReply struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Reply: + // // *SubscribeReply_Assignment Reply isSubscribeReply_Reply `protobuf_oneof:"reply"` } @@ -536,6 +537,7 @@ type ReceiveReply struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Reply: + // // *ReceiveReply_Record_ Reply isReceiveReply_Reply `protobuf_oneof:"reply"` } @@ -805,11 +807,11 @@ type ReceiveReply_Record struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Offset uint64 `protobuf:"varint,1,opt,name=offset,proto3" json:"offset,omitempty"` - Key []byte `protobuf:"bytes,2,opt,name=key,proto3" json:"key,omitempty"` - Value []byte `protobuf:"bytes,3,opt,name=value,proto3" json:"value,omitempty"` - Timestamp *timestamp.Timestamp `protobuf:"bytes,4,opt,name=timestamp,proto3" json:"timestamp,omitempty"` - Replay bool `protobuf:"varint,5,opt,name=replay,proto3" json:"replay,omitempty"` + Offset uint64 `protobuf:"varint,1,opt,name=offset,proto3" json:"offset,omitempty"` + Key []byte `protobuf:"bytes,2,opt,name=key,proto3" json:"key,omitempty"` + Value []byte `protobuf:"bytes,3,opt,name=value,proto3" json:"value,omitempty"` + Timestamp *timestamppb.Timestamp `protobuf:"bytes,4,opt,name=timestamp,proto3" json:"timestamp,omitempty"` + Replay bool `protobuf:"varint,5,opt,name=replay,proto3" json:"replay,omitempty"` } func (x *ReceiveReply_Record) Reset() { @@ -865,7 +867,7 @@ func (x *ReceiveReply_Record) GetValue() []byte { return nil } -func (x *ReceiveReply_Record) GetTimestamp() *timestamp.Timestamp { +func (x *ReceiveReply_Record) GetTimestamp() *timestamppb.Timestamp { if x != nil { return x.Timestamp } @@ -1071,8 +1073,8 @@ var file_LiiklusService_proto_goTypes = []interface{}{ (*ReceiveReply_Record)(nil), // 13: com.github.bsideup.liiklus.ReceiveReply.Record nil, // 14: com.github.bsideup.liiklus.GetOffsetsReply.OffsetsEntry nil, // 15: com.github.bsideup.liiklus.GetEndOffsetsReply.OffsetsEntry - (*timestamp.Timestamp)(nil), // 16: google.protobuf.Timestamp - (*empty.Empty)(nil), // 17: google.protobuf.Empty + (*timestamppb.Timestamp)(nil), // 16: google.protobuf.Timestamp + (*emptypb.Empty)(nil), // 17: google.protobuf.Empty } var file_LiiklusService_proto_depIdxs = []int32{ 0, // 0: com.github.bsideup.liiklus.SubscribeRequest.autoOffsetReset:type_name -> com.github.bsideup.liiklus.SubscribeRequest.AutoOffsetReset diff --git a/pkg/scalers/liiklus/LiiklusService_grpc.pb.go b/pkg/scalers/liiklus/LiiklusService_grpc.pb.go index 6fe70e2b323..d05170338af 100644 --- a/pkg/scalers/liiklus/LiiklusService_grpc.pb.go +++ b/pkg/scalers/liiklus/LiiklusService_grpc.pb.go @@ -8,10 +8,10 @@ package liiklus import ( context "context" - empty "github.com/golang/protobuf/ptypes/empty" grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status" + emptypb "google.golang.org/protobuf/types/known/emptypb" ) // This is a compile-time assertion to ensure that this generated file @@ -26,7 +26,7 @@ type LiiklusServiceClient interface { Publish(ctx context.Context, in *PublishRequest, opts ...grpc.CallOption) (*PublishReply, error) Subscribe(ctx context.Context, in *SubscribeRequest, opts ...grpc.CallOption) (LiiklusService_SubscribeClient, error) Receive(ctx context.Context, in *ReceiveRequest, opts ...grpc.CallOption) (LiiklusService_ReceiveClient, error) - Ack(ctx context.Context, in *AckRequest, opts ...grpc.CallOption) (*empty.Empty, error) + Ack(ctx context.Context, in *AckRequest, opts ...grpc.CallOption) (*emptypb.Empty, error) GetOffsets(ctx context.Context, in *GetOffsetsRequest, opts ...grpc.CallOption) (*GetOffsetsReply, error) GetEndOffsets(ctx context.Context, in *GetEndOffsetsRequest, opts ...grpc.CallOption) (*GetEndOffsetsReply, error) } @@ -112,8 +112,8 @@ func (x *liiklusServiceReceiveClient) Recv() (*ReceiveReply, error) { return m, nil } -func (c *liiklusServiceClient) Ack(ctx context.Context, in *AckRequest, opts ...grpc.CallOption) (*empty.Empty, error) { - out := new(empty.Empty) +func (c *liiklusServiceClient) Ack(ctx context.Context, in *AckRequest, opts ...grpc.CallOption) (*emptypb.Empty, error) { + out := new(emptypb.Empty) err := c.cc.Invoke(ctx, "/com.github.bsideup.liiklus.LiiklusService/Ack", in, out, opts...) if err != nil { return nil, err @@ -146,7 +146,7 @@ type LiiklusServiceServer interface { Publish(context.Context, *PublishRequest) (*PublishReply, error) Subscribe(*SubscribeRequest, LiiklusService_SubscribeServer) error Receive(*ReceiveRequest, LiiklusService_ReceiveServer) error - Ack(context.Context, *AckRequest) (*empty.Empty, error) + Ack(context.Context, *AckRequest) (*emptypb.Empty, error) GetOffsets(context.Context, *GetOffsetsRequest) (*GetOffsetsReply, error) GetEndOffsets(context.Context, *GetEndOffsetsRequest) (*GetEndOffsetsReply, error) mustEmbedUnimplementedLiiklusServiceServer() @@ -165,7 +165,7 @@ func (UnimplementedLiiklusServiceServer) Subscribe(*SubscribeRequest, LiiklusSer func (UnimplementedLiiklusServiceServer) Receive(*ReceiveRequest, LiiklusService_ReceiveServer) error { return status.Errorf(codes.Unimplemented, "method Receive not implemented") } -func (UnimplementedLiiklusServiceServer) Ack(context.Context, *AckRequest) (*empty.Empty, error) { +func (UnimplementedLiiklusServiceServer) Ack(context.Context, *AckRequest) (*emptypb.Empty, error) { return nil, status.Errorf(codes.Unimplemented, "method Ack not implemented") } func (UnimplementedLiiklusServiceServer) GetOffsets(context.Context, *GetOffsetsRequest) (*GetOffsetsReply, error) { diff --git a/pkg/scalers/liiklus_scaler.go b/pkg/scalers/liiklus_scaler.go index 04198828e0d..f3f8996b945 100644 --- a/pkg/scalers/liiklus_scaler.go +++ b/pkg/scalers/liiklus_scaler.go @@ -74,19 +74,19 @@ func NewLiiklusScaler(config *ScalerConfig) (Scaler, error) { return &scaler, nil } -func (s *liiklusScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *liiklusScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { totalLag, lags, err := s.getLag(ctx) if err != nil { - return nil, err + return nil, false, err } if totalLag/uint64(s.metadata.lagThreshold) > uint64(len(lags)) { totalLag = uint64(s.metadata.lagThreshold) * uint64(len(lags)) } - return []external_metrics.ExternalMetricValue{ - GenerateMetricInMili(metricName, float64(totalLag)), - }, nil + metric := GenerateMetricInMili(metricName, float64(totalLag)) + + return []external_metrics.ExternalMetricValue{metric}, totalLag > uint64(s.metadata.activationLagThreshold), nil } func (s *liiklusScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { @@ -108,15 +108,6 @@ func (s *liiklusScaler) Close(context.Context) error { return nil } -// IsActive returns true if there is any lag on any partition. -func (s *liiklusScaler) IsActive(ctx context.Context) (bool, error) { - lag, _, err := s.getLag(ctx) - if err != nil { - return false, err - } - return lag > uint64(s.metadata.activationLagThreshold), nil -} - // getLag returns the total lag, as well as per-partition lag for this scaler. That is, the difference between the // latest offset available on this scaler topic, and the position of the consumer group this scaler is configured for. func (s *liiklusScaler) getLag(ctx context.Context) (uint64, map[uint32]uint64, error) { diff --git a/pkg/scalers/liiklus_scaler_test.go b/pkg/scalers/liiklus_scaler_test.go index 16d95e65fca..b929300a44a 100644 --- a/pkg/scalers/liiklus_scaler_test.go +++ b/pkg/scalers/liiklus_scaler_test.go @@ -96,7 +96,7 @@ func TestLiiklusScalerActiveBehavior(t *testing.T) { GetEndOffsets(gomock.Any(), gomock.Any()). Return(&liiklus.GetEndOffsetsReply{Offsets: map[uint32]uint64{0: 2}}, nil) - active, err := scaler.IsActive(context.Background()) + _, active, err := scaler.GetMetricsAndActivity(context.Background(), "m") if err != nil { t.Errorf("error calling IsActive: %v", err) return @@ -112,7 +112,7 @@ func TestLiiklusScalerActiveBehavior(t *testing.T) { GetEndOffsets(gomock.Any(), gomock.Any()). Return(&liiklus.GetEndOffsetsReply{Offsets: map[uint32]uint64{0: 2}}, nil) - active, err = scaler.IsActive(context.Background()) + _, active, err = scaler.GetMetricsAndActivity(context.Background(), "m") if err != nil { t.Errorf("error calling IsActive: %v", err) return @@ -140,7 +140,7 @@ func TestLiiklusScalerGetMetricsBehavior(t *testing.T) { GetEndOffsets(gomock.Any(), gomock.Any()). Return(&liiklus.GetEndOffsetsReply{Offsets: map[uint32]uint64{0: 20, 1: 30}}, nil) - values, err := scaler.GetMetrics(context.Background(), "m") + values, _, err := scaler.GetMetricsAndActivity(context.Background(), "m") if err != nil { t.Errorf("error calling IsActive: %v", err) return @@ -157,7 +157,7 @@ func TestLiiklusScalerGetMetricsBehavior(t *testing.T) { mockClient.EXPECT(). GetEndOffsets(gomock.Any(), gomock.Any()). Return(&liiklus.GetEndOffsetsReply{Offsets: map[uint32]uint64{0: 20, 1: 30}}, nil) - values, err = scaler.GetMetrics(context.Background(), "m") + values, _, err = scaler.GetMetricsAndActivity(context.Background(), "m") if err != nil { t.Errorf("error calling IsActive: %v", err) return diff --git a/pkg/scalers/loki_scaler.go b/pkg/scalers/loki_scaler.go index d5301ee58bc..631ff08c446 100644 --- a/pkg/scalers/loki_scaler.go +++ b/pkg/scalers/loki_scaler.go @@ -158,17 +158,6 @@ func parseLokiMetadata(config *ScalerConfig) (meta *lokiMetadata, err error) { return meta, nil } -// IsActive validates the loki query result against the activation threshold -func (s *lokiScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.ExecuteLokiQuery(ctx) - if err != nil { - s.logger.Error(err, "error executing loki query") - return false, err - } - - return val > s.metadata.activationThreshold, nil -} - // Close returns a nil error func (s *lokiScaler) Close(context.Context) error { return nil @@ -273,15 +262,15 @@ func (s *lokiScaler) ExecuteLokiQuery(ctx context.Context) (float64, error) { return v, nil } -// GetMetrics returns an external metric value for the loki -func (s *lokiScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns an external metric value for the loki +func (s *lokiScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := s.ExecuteLokiQuery(ctx) if err != nil { s.logger.Error(err, "error executing loki query") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil } diff --git a/pkg/scalers/metrics_api_scaler.go b/pkg/scalers/metrics_api_scaler.go index 08433269e89..257bf4c0a1f 100644 --- a/pkg/scalers/metrics_api_scaler.go +++ b/pkg/scalers/metrics_api_scaler.go @@ -256,17 +256,6 @@ func (s *metricsAPIScaler) Close(context.Context) error { return nil } -// IsActive returns true if there are pending messages to be processed -func (s *metricsAPIScaler) IsActive(ctx context.Context) (bool, error) { - v, err := s.getMetricValue(ctx) - if err != nil { - s.logger.Error(err, fmt.Sprintf("Error when checking metric value: %s", err)) - return false, err - } - - return v > s.metadata.activationTargetValue, nil -} - // GetMetricSpecForScaling returns the MetricSpec for the Horizontal Pod Autoscaler func (s *metricsAPIScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { externalMetric := &v2.ExternalMetricSource{ @@ -282,15 +271,15 @@ func (s *metricsAPIScaler) GetMetricSpecForScaling(context.Context) []v2.MetricS } // GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *metricsAPIScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *metricsAPIScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := s.getMetricValue(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error requesting metrics endpoint: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error requesting metrics endpoint: %s", err) } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationTargetValue, nil } func getMetricAPIServerRequest(ctx context.Context, meta *metricsAPIScalerMetadata) (*http.Request, error) { diff --git a/pkg/scalers/metrics_api_scaler_test.go b/pkg/scalers/metrics_api_scaler_test.go index 7dca8cac2a4..430218d864f 100644 --- a/pkg/scalers/metrics_api_scaler_test.go +++ b/pkg/scalers/metrics_api_scaler_test.go @@ -219,7 +219,7 @@ func TestBearerAuth(t *testing.T) { t.Errorf("Error creating the Scaler") } - _, err = s.GetMetrics(context.TODO(), "test-metric") + _, _, err = s.GetMetricsAndActivity(context.TODO(), "test-metric") if err != nil { t.Errorf("Error getting the metric") } diff --git a/pkg/scalers/mongo_scaler.go b/pkg/scalers/mongo_scaler.go index 1fb0b63a044..e902a0ef018 100644 --- a/pkg/scalers/mongo_scaler.go +++ b/pkg/scalers/mongo_scaler.go @@ -201,15 +201,6 @@ func parseMongoDBMetadata(config *ScalerConfig) (*mongoDBMetadata, string, error return &meta, connStr, nil } -func (s *mongoDBScaler) IsActive(ctx context.Context) (bool, error) { - result, err := s.getQueryResult(ctx) - if err != nil { - s.logger.Error(err, fmt.Sprintf("failed to get query result by mongoDB, because of %v", err)) - return false, err - } - return result > s.metadata.activationQueryValue, nil -} - // Close disposes of mongoDB connections func (s *mongoDBScaler) Close(ctx context.Context) error { if s.client != nil { @@ -243,16 +234,16 @@ func (s *mongoDBScaler) getQueryResult(ctx context.Context) (int64, error) { return docsNum, nil } -// GetMetrics query from mongoDB,and return to external metrics -func (s *mongoDBScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity query from mongoDB,and return to external metrics +func (s *mongoDBScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.getQueryResult(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("failed to inspect momgoDB, because of %v", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("failed to inspect momgoDB, because of %v", err) } metric := GenerateMetricInMili(metricName, float64(num)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationQueryValue, nil } // GetMetricSpecForScaling get the query value for scaling diff --git a/pkg/scalers/mssql_scaler.go b/pkg/scalers/mssql_scaler.go index 9bebcf3c521..f9c487aac2f 100644 --- a/pkg/scalers/mssql_scaler.go +++ b/pkg/scalers/mssql_scaler.go @@ -243,16 +243,16 @@ func (s *mssqlScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { return []v2.MetricSpec{metricSpec} } -// GetMetrics returns a value for a supported metric or an error if there is a problem getting the metric -func (s *mssqlScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns a value for a supported metric or an error if there is a problem getting the metric +func (s *mssqlScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.getQueryResult(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting mssql: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting mssql: %s", err) } metric := GenerateMetricInMili(metricName, num) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationTargetValue, nil } // getQueryResult returns the result of the scaler query @@ -270,16 +270,6 @@ func (s *mssqlScaler) getQueryResult(ctx context.Context) (float64, error) { return value, nil } -// IsActive returns true if there are pending events to be processed -func (s *mssqlScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.getQueryResult(ctx) - if err != nil { - return false, fmt.Errorf("error inspecting mssql: %s", err) - } - - return messages > s.metadata.activationTargetValue, nil -} - // Close closes the mssql database connections func (s *mssqlScaler) Close(context.Context) error { err := s.connection.Close() diff --git a/pkg/scalers/mysql_scaler.go b/pkg/scalers/mysql_scaler.go index 019e2bf3db3..476ecfffd25 100644 --- a/pkg/scalers/mysql_scaler.go +++ b/pkg/scalers/mysql_scaler.go @@ -192,16 +192,6 @@ func (s *mySQLScaler) Close(context.Context) error { return nil } -// IsActive returns true if there are pending messages to be processed -func (s *mySQLScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.getQueryResult(ctx) - if err != nil { - s.logger.Error(err, fmt.Sprintf("Error inspecting MySQL: %s", err)) - return false, err - } - return messages > s.metadata.activationQueryValue, nil -} - // getQueryResult returns result of the scaler query func (s *mySQLScaler) getQueryResult(ctx context.Context) (float64, error) { var value float64 @@ -227,14 +217,14 @@ func (s *mySQLScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *mySQLScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *mySQLScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.getQueryResult(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting MySQL: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting MySQL: %s", err) } metric := GenerateMetricInMili(metricName, num) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationQueryValue, nil } diff --git a/pkg/scalers/nats_jetstream_scaler.go b/pkg/scalers/nats_jetstream_scaler.go index df25fe3a239..0f9ac376607 100644 --- a/pkg/scalers/nats_jetstream_scaler.go +++ b/pkg/scalers/nats_jetstream_scaler.go @@ -10,8 +10,6 @@ import ( "github.com/go-logr/logr" v2 "k8s.io/api/autoscaling/v2" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/metrics/pkg/apis/external_metrics" kedautil "github.com/kedacore/keda/v2/pkg/util" @@ -171,38 +169,6 @@ func getNATSJetStreamEndpoint(useHTTPS bool, natsServerEndpoint string, account return fmt.Sprintf("%s://%s/jsz?acc=%s&consumers=true&config=true", protocol, natsServerEndpoint, account) } -func (s *natsJetStreamScaler) IsActive(ctx context.Context) (bool, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.metadata.monitoringEndpoint, nil) - if err != nil { - return false, err - } - - resp, err := s.httpClient.Do(req) - if err != nil { - s.logger.Error(err, "unable to access NATS JetStream monitoring endpoint", "natsServerMonitoringEndpoint", s.metadata.monitoringEndpoint) - return false, err - } - - defer resp.Body.Close() - var jsAccountResp jetStreamEndpointResponse - if err = json.NewDecoder(resp.Body).Decode(&jsAccountResp); err != nil { - s.logger.Error(err, "unable to decode JetStream account response") - return false, err - } - - // Find and assign the stream that we are looking for. - for _, account := range jsAccountResp.Accounts { - if account.Name == s.metadata.account { - for _, stream := range account.Streams { - if stream.Name == s.metadata.stream { - s.stream = stream - } - } - } - } - return s.getMaxMsgLag() > s.metadata.activationLagThreshold, nil -} - func (s *natsJetStreamScaler) getMaxMsgLag() int64 { consumerName := s.metadata.consumer @@ -228,23 +194,23 @@ func (s *natsJetStreamScaler) GetMetricSpecForScaling(context.Context) []v2.Metr return []v2.MetricSpec{metricSpec} } -func (s *natsJetStreamScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *natsJetStreamScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.metadata.monitoringEndpoint, nil) if err != nil { - return nil, err + return []external_metrics.ExternalMetricValue{}, false, err } resp, err := s.httpClient.Do(req) if err != nil { s.logger.Error(err, "unable to access NATS JetStream monitoring endpoint", "natsServerMonitoringEndpoint", s.metadata.monitoringEndpoint) - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } defer resp.Body.Close() var jsAccountResp jetStreamEndpointResponse if err = json.NewDecoder(resp.Body).Decode(&jsAccountResp); err != nil { - s.logger.Error(err, "unable to decode JetStream account details") - return []external_metrics.ExternalMetricValue{}, err + s.logger.Error(err, "unable to decode JetStream account response") + return []external_metrics.ExternalMetricValue{}, false, err } // Find and assign the stream that we are looking for. @@ -261,13 +227,9 @@ func (s *natsJetStreamScaler) GetMetrics(ctx context.Context, metricName string) totalLag := s.getMaxMsgLag() s.logger.V(1).Info("NATS JetStream Scaler: Providing metrics based on totalLag, threshold", "totalLag", totalLag, "lagThreshold", s.metadata.lagThreshold) - metric := external_metrics.ExternalMetricValue{ - MetricName: metricName, - Value: *resource.NewQuantity(totalLag, resource.DecimalSI), - Timestamp: metav1.Now(), - } + metric := GenerateMetricInMili(metricName, float64(totalLag)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, totalLag > s.metadata.activationLagThreshold, nil } func (s *natsJetStreamScaler) Close(context.Context) error { diff --git a/pkg/scalers/newrelic_scaler.go b/pkg/scalers/newrelic_scaler.go index 7e5fa565625..11813fea29f 100644 --- a/pkg/scalers/newrelic_scaler.go +++ b/pkg/scalers/newrelic_scaler.go @@ -141,15 +141,6 @@ func parseNewRelicMetadata(config *ScalerConfig, logger logr.Logger) (*newrelicM return &meta, nil } -func (s *newrelicScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.executeNewRelicQuery(ctx) - if err != nil { - s.logger.Error(err, "error executing NRQL") - return false, err - } - return val > s.metadata.activationThreshold, nil -} - func (s *newrelicScaler) Close(context.Context) error { return nil } @@ -173,16 +164,16 @@ func (s *newrelicScaler) executeNewRelicQuery(ctx context.Context) (float64, err return 0, nil } -func (s *newrelicScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *newrelicScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := s.executeNewRelicQuery(ctx) if err != nil { s.logger.Error(err, "error executing NRQL query") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil } func (s *newrelicScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { diff --git a/pkg/scalers/openstack_metrics_scaler.go b/pkg/scalers/openstack_metrics_scaler.go index e026948a445..039c9446d03 100644 --- a/pkg/scalers/openstack_metrics_scaler.go +++ b/pkg/scalers/openstack_metrics_scaler.go @@ -232,27 +232,17 @@ func (s *openstackMetricScaler) GetMetricSpecForScaling(context.Context) []v2.Me return []v2.MetricSpec{metricSpec} } -func (s *openstackMetricScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *openstackMetricScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := s.readOpenstackMetrics(ctx) if err != nil { s.logger.Error(err, "Error collecting metric value") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil -} - -func (s *openstackMetricScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.readOpenstackMetrics(ctx) - - if err != nil { - return false, err - } - - return val > s.metadata.activationThreshold, nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil } func (s *openstackMetricScaler) Close(context.Context) error { diff --git a/pkg/scalers/openstack_swift_scaler.go b/pkg/scalers/openstack_swift_scaler.go index 3f0927e75ab..3c1e3ba2905 100644 --- a/pkg/scalers/openstack_swift_scaler.go +++ b/pkg/scalers/openstack_swift_scaler.go @@ -368,31 +368,21 @@ func parseOpenstackSwiftAuthenticationMetadata(config *ScalerConfig) (*openstack return &authMeta, nil } -func (s *openstackSwiftScaler) IsActive(ctx context.Context) (bool, error) { - objectCount, err := s.getOpenstackSwiftContainerObjectCount(ctx) - - if err != nil { - return false, err - } - - return objectCount > s.metadata.activationObjectCount, nil -} - func (s *openstackSwiftScaler) Close(context.Context) error { return nil } -func (s *openstackSwiftScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *openstackSwiftScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { objectCount, err := s.getOpenstackSwiftContainerObjectCount(ctx) if err != nil { s.logger.Error(err, "error getting objectCount") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(objectCount)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, objectCount > s.metadata.activationObjectCount, nil } func (s *openstackSwiftScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { diff --git a/pkg/scalers/postgresql_scaler.go b/pkg/scalers/postgresql_scaler.go index 6bd4203b05d..00f2b2048d3 100644 --- a/pkg/scalers/postgresql_scaler.go +++ b/pkg/scalers/postgresql_scaler.go @@ -167,16 +167,6 @@ func (s *postgreSQLScaler) Close(context.Context) error { return nil } -// IsActive returns true if there are pending messages to be processed -func (s *postgreSQLScaler) IsActive(ctx context.Context) (bool, error) { - messages, err := s.getActiveNumber(ctx) - if err != nil { - return false, fmt.Errorf("error inspecting postgreSQL: %s", err) - } - - return messages > s.metadata.activationTargetQueryValue, nil -} - func (s *postgreSQLScaler) getActiveNumber(ctx context.Context) (float64, error) { var id float64 err := s.connection.QueryRowContext(ctx, s.metadata.query).Scan(&id) @@ -201,14 +191,14 @@ func (s *postgreSQLScaler) GetMetricSpecForScaling(context.Context) []v2.MetricS return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *postgreSQLScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *postgreSQLScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { num, err := s.getActiveNumber(ctx) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error inspecting postgreSQL: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error inspecting postgreSQL: %s", err) } metric := GenerateMetricInMili(metricName, num) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, num > s.metadata.activationTargetQueryValue, nil } diff --git a/pkg/scalers/predictkube_scaler.go b/pkg/scalers/predictkube_scaler.go index 1436185aeb6..4f0286ad1df 100644 --- a/pkg/scalers/predictkube_scaler.go +++ b/pkg/scalers/predictkube_scaler.go @@ -239,6 +239,23 @@ func (s *PredictKubeScaler) GetMetrics(ctx context.Context, metricName string) ( return append([]external_metrics.ExternalMetricValue{}, metric), nil } +// TODO merge isActive() and GetMetrics() +func (s *PredictKubeScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { + metrics, err := s.GetMetrics(ctx, metricName) + if err != nil { + s.logger.Error(err, "error getting metrics") + return []external_metrics.ExternalMetricValue{}, false, err + } + + isActive, err := s.IsActive(ctx) + if err != nil { + s.logger.Error(err, "error getting activity status") + return []external_metrics.ExternalMetricValue{}, false, err + } + + return metrics, isActive, nil +} + func (s *PredictKubeScaler) doPredictRequest(ctx context.Context) (float64, error) { results, err := s.doQuery(ctx) if err != nil { diff --git a/pkg/scalers/prometheus_scaler.go b/pkg/scalers/prometheus_scaler.go index c44acc5f812..6a455bcda3b 100644 --- a/pkg/scalers/prometheus_scaler.go +++ b/pkg/scalers/prometheus_scaler.go @@ -189,16 +189,6 @@ func parsePrometheusMetadata(config *ScalerConfig) (meta *prometheusMetadata, er return meta, nil } -func (s *prometheusScaler) IsActive(ctx context.Context) (bool, error) { - val, err := s.ExecutePromQuery(ctx) - if err != nil { - s.logger.Error(err, "error executing prometheus query") - return false, err - } - - return val > s.metadata.activationThreshold, nil -} - func (s *prometheusScaler) Close(context.Context) error { return nil } @@ -309,14 +299,14 @@ func (s *prometheusScaler) ExecutePromQuery(ctx context.Context) (float64, error return v, nil } -func (s *prometheusScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *prometheusScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { val, err := s.ExecutePromQuery(ctx) if err != nil { s.logger.Error(err, "error executing prometheus query") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, val) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil } diff --git a/pkg/scalers/pulsar_scaler.go b/pkg/scalers/pulsar_scaler.go index cd68b58d344..f814f6db991 100644 --- a/pkg/scalers/pulsar_scaler.go +++ b/pkg/scalers/pulsar_scaler.go @@ -254,35 +254,20 @@ func (s *pulsarScaler) getMsgBackLog(ctx context.Context) (int64, bool, error) { return v.Msgbacklog, found, nil } -// IsActive determines if we need to scale from zero -func (s *pulsarScaler) IsActive(ctx context.Context) (bool, error) { - msgBackLog, found, err := s.getMsgBackLog(ctx) - if err != nil { - return false, err - } - - if !found { - s.logger.Info("Pulsar subscription is not active, either no subscription found or no backlog detected") - return false, nil - } - - return msgBackLog > s.metadata.activationMsgBacklogThreshold, nil -} - -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *pulsarScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetGetMetricsAndActivityMetrics returns value for a supported metric and an error if there is a problem getting the metric +func (s *pulsarScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { msgBacklog, found, err := s.getMsgBackLog(ctx) if err != nil { - return nil, fmt.Errorf("error requesting stats from url: %s", err) + return nil, false, fmt.Errorf("error requesting stats from url: %s", err) } if !found { - return nil, fmt.Errorf("have not subscription found! %s", s.metadata.subscription) + return nil, false, fmt.Errorf("have not subscription found! %s", s.metadata.subscription) } metric := GenerateMetricInMili(metricName, float64(msgBacklog)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, msgBacklog > s.metadata.activationMsgBacklogThreshold, nil } func (s *pulsarScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { diff --git a/pkg/scalers/pulsar_scaler_test.go b/pkg/scalers/pulsar_scaler_test.go index 2146c85e9e5..549546ef795 100644 --- a/pkg/scalers/pulsar_scaler_test.go +++ b/pkg/scalers/pulsar_scaler_test.go @@ -231,7 +231,10 @@ func TestPulsarIsActive(t *testing.T) { t.Fatal("Failed:", err) } - active, err := mockPulsarScaler.IsActive(context.TODO()) + metricSpec := mockPulsarScaler.GetMetricSpecForScaling(context.TODO()) + metricName := metricSpec[0].External.Metric.Name + + _, active, err := mockPulsarScaler.GetMetricsAndActivity(context.TODO(), metricName) if err != nil { t.Fatal("Failed:", err) } @@ -252,7 +255,10 @@ func TestPulsarIsActiveWithAuthParams(t *testing.T) { t.Fatal("Failed:", err) } - active, err := mockPulsarScaler.IsActive(context.TODO()) + metricSpec := mockPulsarScaler.GetMetricSpecForScaling(context.TODO()) + metricName := metricSpec[0].External.Metric.Name + + _, active, err := mockPulsarScaler.GetMetricsAndActivity(context.TODO(), metricName) if err != nil && !testData.metadataTestData.isError { t.Fatal("Failed:", err) } @@ -276,7 +282,7 @@ func TestPulsarGetMetric(t *testing.T) { metricSpec := mockPulsarScaler.GetMetricSpecForScaling(context.TODO()) metricName := metricSpec[0].External.Metric.Name - metric, err := mockPulsarScaler.GetMetrics(context.TODO(), metricName) + metric, _, err := mockPulsarScaler.GetMetricsAndActivity(context.TODO(), metricName) if err != nil { t.Fatal("Failed:", err) } diff --git a/pkg/scalers/rabbitmq_scaler.go b/pkg/scalers/rabbitmq_scaler.go index a449f060ae4..eb937516343 100644 --- a/pkg/scalers/rabbitmq_scaler.go +++ b/pkg/scalers/rabbitmq_scaler.go @@ -380,19 +380,6 @@ func (s *rabbitMQScaler) Close(context.Context) error { return nil } -// IsActive returns true if there are pending messages to be processed -func (s *rabbitMQScaler) IsActive(ctx context.Context) (bool, error) { - messages, publishRate, err := s.getQueueStatus() - if err != nil { - return false, s.anonimizeRabbitMQError(err) - } - - if s.metadata.mode == rabbitModeQueueLength { - return float64(messages) > s.metadata.activationValue, nil - } - return publishRate > s.metadata.activationValue || float64(messages) > s.metadata.activationValue, nil -} - func (s *rabbitMQScaler) getQueueStatus() (int64, float64, error) { if s.metadata.protocol == httpProtocol { info, err := s.getQueueInfoViaHTTP() @@ -499,21 +486,24 @@ func (s *rabbitMQScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpe return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *rabbitMQScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *rabbitMQScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { messages, publishRate, err := s.getQueueStatus() if err != nil { - return []external_metrics.ExternalMetricValue{}, s.anonimizeRabbitMQError(err) + return []external_metrics.ExternalMetricValue{}, false, s.anonimizeRabbitMQError(err) } var metric external_metrics.ExternalMetricValue + var isActive bool if s.metadata.mode == rabbitModeQueueLength { metric = GenerateMetricInMili(metricName, float64(messages)) + isActive = float64(messages) > s.metadata.activationValue } else { metric = GenerateMetricInMili(metricName, publishRate) + isActive = publishRate > s.metadata.activationValue || float64(messages) > s.metadata.activationValue } - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, isActive, nil } func getComposedQueue(s *rabbitMQScaler, q []queueInfo) (queueInfo, error) { diff --git a/pkg/scalers/rabbitmq_scaler_test.go b/pkg/scalers/rabbitmq_scaler_test.go index 7bc7f74d8eb..3cb8a8b491e 100644 --- a/pkg/scalers/rabbitmq_scaler_test.go +++ b/pkg/scalers/rabbitmq_scaler_test.go @@ -263,7 +263,7 @@ func TestGetQueueInfo(t *testing.T) { } ctx := context.TODO() - active, err := s.IsActive(ctx) + _, active, err := s.GetMetricsAndActivity(ctx, "Metric") if testData.responseStatus == http.StatusOK { if err != nil { @@ -400,7 +400,7 @@ func TestGetQueueInfoWithRegex(t *testing.T) { } ctx := context.TODO() - active, err := s.IsActive(ctx) + _, active, err := s.GetMetricsAndActivity(ctx, "Metric") if testData.responseStatus == http.StatusOK { if err != nil { @@ -479,7 +479,7 @@ func TestGetPageSizeWithRegex(t *testing.T) { } ctx := context.TODO() - active, err := s.IsActive(ctx) + _, active, err := s.GetMetricsAndActivity(ctx, "Metric") if err != nil { t.Error("Expect success", err) @@ -600,7 +600,7 @@ func TestRegexQueueMissingError(t *testing.T) { } ctx := context.TODO() - _, err = s.IsActive(ctx) + _, _, err = s.GetMetricsAndActivity(ctx, "Metric") if err != nil && !testData.isError { t.Error("Expected success but got error", err) } diff --git a/pkg/scalers/redis_scaler.go b/pkg/scalers/redis_scaler.go index 50a740da864..e08630490e3 100644 --- a/pkg/scalers/redis_scaler.go +++ b/pkg/scalers/redis_scaler.go @@ -218,18 +218,6 @@ func parseRedisMetadata(config *ScalerConfig, parserFn redisAddressParser) (*red return &meta, nil } -// IsActive checks if there is any element in the Redis list -func (s *redisScaler) IsActive(ctx context.Context) (bool, error) { - length, err := s.getListLengthFn(ctx) - - if err != nil { - s.logger.Error(err, "error") - return false, err - } - - return length > s.metadata.activationListLength, nil -} - func (s *redisScaler) Close(context.Context) error { return s.closeFn() } @@ -249,18 +237,18 @@ func (s *redisScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { return []v2.MetricSpec{metricSpec} } -// GetMetrics connects to Redis and finds the length of the list -func (s *redisScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity connects to Redis and finds the length of the list +func (s *redisScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { listLen, err := s.getListLengthFn(ctx) if err != nil { s.logger.Error(err, "error getting list length") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(listLen)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, listLen > s.metadata.activationListLength, nil } func parseRedisAddress(metadata, resolvedEnv, authParams map[string]string) (redisConnectionInfo, error) { diff --git a/pkg/scalers/redis_streams_scaler.go b/pkg/scalers/redis_streams_scaler.go index 349a731cc9e..fad5782c1ae 100644 --- a/pkg/scalers/redis_streams_scaler.go +++ b/pkg/scalers/redis_streams_scaler.go @@ -193,18 +193,6 @@ func parseRedisStreamsMetadata(config *ScalerConfig, parseFn redisAddressParser) return &meta, nil } -// IsActive checks if there are pending entries in the 'Pending Entries List' for consumer group of a stream -func (s *redisStreamsScaler) IsActive(ctx context.Context) (bool, error) { - count, err := s.getPendingEntriesCountFn(ctx) - - if err != nil { - s.logger.Error(err, "error") - return false, err - } - - return count > 0, nil -} - func (s *redisStreamsScaler) Close(context.Context) error { return s.closeFn() } @@ -221,15 +209,16 @@ func (s *redisStreamsScaler) GetMetricSpecForScaling(context.Context) []v2.Metri return []v2.MetricSpec{metricSpec} } -// GetMetrics fetches the number of pending entries for a consumer group in a stream -func (s *redisStreamsScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity fetches the number of pending entries for a consumer group in a stream +func (s *redisStreamsScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { pendingEntriesCount, err := s.getPendingEntriesCountFn(ctx) if err != nil { s.logger.Error(err, "error fetching pending entries count") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } metric := GenerateMetricInMili(metricName, float64(pendingEntriesCount)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + + return []external_metrics.ExternalMetricValue{metric}, pendingEntriesCount > 0, nil } diff --git a/pkg/scalers/scaler.go b/pkg/scalers/scaler.go index 8f8554df7c5..0ca8d153963 100644 --- a/pkg/scalers/scaler.go +++ b/pkg/scalers/scaler.go @@ -41,16 +41,13 @@ func init() { // Scaler interface type Scaler interface { - - // The scaler returns the metric values for a metric Name and criteria matching the selector - GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) + // The scaler returns the metric values and activity for a metric Name + GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) // Returns the metrics based on which this scaler determines that the ScaleTarget scales. This is used to construct the HPA spec that is created for // this scaled object. The labels used should match the selectors used in GetMetrics GetMetricSpecForScaling(ctx context.Context) []v2.MetricSpec - IsActive(ctx context.Context) (bool, error) - // Close any resources that need disposing when scaler is no longer used or destroyed Close(ctx context.Context) error } @@ -80,6 +77,11 @@ type ScalerConfig struct { // Name of the trigger TriggerName string + // Cache duration for this trigger + TriggerCacheDuration *int32 + + TriggerEnableCache bool + // TriggerMetadata TriggerMetadata map[string]string diff --git a/pkg/scalers/selenium_grid_scaler.go b/pkg/scalers/selenium_grid_scaler.go index 7273d572ac2..32a71398e0d 100644 --- a/pkg/scalers/selenium_grid_scaler.go +++ b/pkg/scalers/selenium_grid_scaler.go @@ -152,15 +152,15 @@ func (s *seleniumGridScaler) Close(context.Context) error { return nil } -func (s *seleniumGridScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +func (s *seleniumGridScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { sessions, err := s.getSessionsCount(ctx, s.logger) if err != nil { - return []external_metrics.ExternalMetricValue{}, fmt.Errorf("error requesting selenium grid endpoint: %s", err) + return []external_metrics.ExternalMetricValue{}, false, fmt.Errorf("error requesting selenium grid endpoint: %s", err) } metric := GenerateMetricInMili(metricName, float64(sessions)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + return []external_metrics.ExternalMetricValue{metric}, sessions > s.metadata.activationThreshold, nil } func (s *seleniumGridScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { @@ -177,15 +177,6 @@ func (s *seleniumGridScaler) GetMetricSpecForScaling(context.Context) []v2.Metri return []v2.MetricSpec{metricSpec} } -func (s *seleniumGridScaler) IsActive(ctx context.Context) (bool, error) { - v, err := s.getSessionsCount(ctx, s.logger) - if err != nil { - return false, err - } - - return v > s.metadata.activationThreshold, nil -} - func (s *seleniumGridScaler) getSessionsCount(ctx context.Context, logger logr.Logger) (int64, error) { body, err := json.Marshal(map[string]string{ "query": "{ grid { maxSession, nodeCount }, sessionsInfo { sessionQueueRequests, sessions { id, capabilities, nodeId } } }", diff --git a/pkg/scalers/solace_scaler.go b/pkg/scalers/solace_scaler.go index 91e67744a13..36a7c0207e8 100644 --- a/pkg/scalers/solace_scaler.go +++ b/pkg/scalers/solace_scaler.go @@ -347,12 +347,13 @@ func (s *SolaceScaler) getSolaceQueueMetricsFromSEMP(ctx context.Context) (Solac // INTERFACE METHOD // Call SEMP API to retrieve metrics // returns value for named metric -func (s *SolaceScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// returns true if queue messageCount > 0 || msgSpoolUsage > 0 +func (s *SolaceScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { var metricValues, mv SolaceMetricValues var mve error if mv, mve = s.getSolaceQueueMetricsFromSEMP(ctx); mve != nil { s.logger.Error(mve, "call to semp endpoint failed") - return []external_metrics.ExternalMetricValue{}, mve + return []external_metrics.ExternalMetricValue{}, false, mve } metricValues = mv @@ -366,21 +367,9 @@ func (s *SolaceScaler) GetMetrics(ctx context.Context, metricName string) ([]ext // Should never end up here err := fmt.Errorf("unidentified metric: %s", metricName) s.logger.Error(err, "returning error to calling app") - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } - return append([]external_metrics.ExternalMetricValue{}, metric), nil -} - -// INTERFACE METHOD -// Call SEMP API to retrieve metrics -// IsActive returns true if queue messageCount > 0 || msgSpoolUsage > 0 -func (s *SolaceScaler) IsActive(ctx context.Context) (bool, error) { - metricValues, err := s.getSolaceQueueMetricsFromSEMP(ctx) - if err != nil { - s.logger.Error(err, "call to semp endpoint failed") - return false, err - } - return (metricValues.msgCount > s.metadata.activationMsgCountTarget || metricValues.msgSpoolUsage > s.metadata.activationMsgSpoolUsageTarget), nil + return []external_metrics.ExternalMetricValue{metric}, (metricValues.msgCount > s.metadata.activationMsgCountTarget || metricValues.msgSpoolUsage > s.metadata.activationMsgSpoolUsageTarget), nil } // Do Nothing - Satisfies Interface diff --git a/pkg/scalers/stan_scaler.go b/pkg/scalers/stan_scaler.go index 5dae5fe33d0..a4f9b0c7787 100644 --- a/pkg/scalers/stan_scaler.go +++ b/pkg/scalers/stan_scaler.go @@ -139,45 +139,6 @@ func parseStanMetadata(config *ScalerConfig) (stanMetadata, error) { return meta, nil } -// IsActive determines if we need to scale from zero -func (s *stanScaler) IsActive(ctx context.Context) (bool, error) { - req, err := http.NewRequestWithContext(ctx, "GET", s.metadata.monitoringEndpoint, nil) - if err != nil { - return false, err - } - resp, err := s.httpClient.Do(req) - if err != nil { - s.logger.Error(err, "Unable to access the nats streaming broker monitoring endpoint", "natsServerMonitoringEndpoint", s.metadata.monitoringEndpoint) - return false, err - } - - if resp.StatusCode == 404 { - req, err := http.NewRequestWithContext(ctx, "GET", s.metadata.stanChannelsEndpoint, nil) - if err != nil { - return false, err - } - baseResp, err := s.httpClient.Do(req) - if err != nil { - return false, err - } - defer baseResp.Body.Close() - if baseResp.StatusCode == 404 { - s.logger.Info("Streaming broker endpoint returned 404. Please ensure it has been created", "url", s.metadata.monitoringEndpoint, "channelName", s.metadata.subject) - } else { - s.logger.Info("Unable to connect to STAN. Please ensure you have configured the ScaledObject with the correct endpoint.", "baseResp.StatusCode", baseResp.StatusCode, "monitoringEndpoint", s.metadata.monitoringEndpoint) - } - - return false, err - } - - defer resp.Body.Close() - if err := json.NewDecoder(resp.Body).Decode(&s.channelInfo); err != nil { - s.logger.Error(err, "Unable to decode channel info as %v", err) - return false, err - } - return s.hasPendingMessage() || s.getMaxMsgLag() > s.metadata.activationLagThreshold, nil -} - func getSTANChannelsEndpoint(useHTTPS bool, natsServerEndpoint string) string { protocol := natsStreamingHTTPProtocol if useHTTPS { @@ -240,28 +201,49 @@ func (s *stanScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { return []v2.MetricSpec{metricSpec} } -// GetMetrics returns value for a supported metric and an error if there is a problem getting the metric -func (s *stanScaler) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { +// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric +func (s *stanScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) { req, err := http.NewRequestWithContext(ctx, "GET", s.metadata.monitoringEndpoint, nil) if err != nil { - return nil, err + return []external_metrics.ExternalMetricValue{}, false, err } resp, err := s.httpClient.Do(req) if err != nil { s.logger.Error(err, "Unable to access the nats streaming broker monitoring endpoint", "monitoringEndpoint", s.metadata.monitoringEndpoint) - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err + } + + if resp.StatusCode == 404 { + req, err := http.NewRequestWithContext(ctx, "GET", s.metadata.stanChannelsEndpoint, nil) + if err != nil { + return []external_metrics.ExternalMetricValue{}, false, err + } + baseResp, err := s.httpClient.Do(req) + if err != nil { + return []external_metrics.ExternalMetricValue{}, false, err + } + defer baseResp.Body.Close() + if baseResp.StatusCode == 404 { + s.logger.Info("Streaming broker endpoint returned 404. Please ensure it has been created", "url", s.metadata.monitoringEndpoint, "channelName", s.metadata.subject) + } else { + s.logger.Info("Unable to connect to STAN. Please ensure you have configured the ScaledObject with the correct endpoint.", "baseResp.StatusCode", baseResp.StatusCode, "monitoringEndpoint", s.metadata.monitoringEndpoint) + } + + return []external_metrics.ExternalMetricValue{}, false, err } defer resp.Body.Close() if err := json.NewDecoder(resp.Body).Decode(&s.channelInfo); err != nil { s.logger.Error(err, "Unable to decode channel info as %v", err) - return []external_metrics.ExternalMetricValue{}, err + return []external_metrics.ExternalMetricValue{}, false, err } totalLag := s.getMaxMsgLag() s.logger.V(1).Info("Stan scaler: Providing metrics based on totalLag, threshold", "totalLag", totalLag, "lagThreshold", s.metadata.lagThreshold) + metric := GenerateMetricInMili(metricName, float64(totalLag)) - return append([]external_metrics.ExternalMetricValue{}, metric), nil + + return []external_metrics.ExternalMetricValue{metric}, s.hasPendingMessage() || totalLag > s.metadata.activationLagThreshold, nil } // Nothing to close here. diff --git a/pkg/scaling/cache/metricscache/metrics_record.go b/pkg/scaling/cache/metricscache/metrics_record.go new file mode 100644 index 00000000000..d640b8281a3 --- /dev/null +++ b/pkg/scaling/cache/metricscache/metrics_record.go @@ -0,0 +1,45 @@ +package metricscache + +import ( + "sync" + + "k8s.io/metrics/pkg/apis/external_metrics" +) + +type MetricsRecord struct { + IsActive bool + Metric []external_metrics.ExternalMetricValue + ScalerError error +} + +type MetricsCache struct { + metricRecords map[string]map[string]MetricsRecord + lock *sync.RWMutex +} + +func NewMetricsCache() MetricsCache { + return MetricsCache{ + metricRecords: map[string]map[string]MetricsRecord{}, + lock: &sync.RWMutex{}, + } +} + +func (mc *MetricsCache) ReadRecord(scaledObjectIdentifier, metricName string) (MetricsRecord, bool) { + mc.lock.RLock() + defer mc.lock.RUnlock() + record, ok := mc.metricRecords[scaledObjectIdentifier][metricName] + + return record, ok +} + +func (mc *MetricsCache) StoreRecords(scaledObjectIdentifier string, metricsRecords map[string]MetricsRecord) { + mc.lock.Lock() + defer mc.lock.Unlock() + mc.metricRecords[scaledObjectIdentifier] = metricsRecords +} + +func (mc *MetricsCache) Delete(scaledObjectIdentifier string) { + mc.lock.Lock() + defer mc.lock.Unlock() + delete(mc.metricRecords, scaledObjectIdentifier) +} diff --git a/pkg/scaling/cache/scalers_cache.go b/pkg/scaling/cache/scalers_cache.go index d7d3b33a275..c0f994486db 100644 --- a/pkg/scaling/cache/scalers_cache.go +++ b/pkg/scaling/cache/scalers_cache.go @@ -21,7 +21,6 @@ import ( "fmt" "math" - "github.com/go-logr/logr" v2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" @@ -31,13 +30,14 @@ import ( kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1" "github.com/kedacore/keda/v2/pkg/eventreason" "github.com/kedacore/keda/v2/pkg/scalers" + "github.com/kedacore/keda/v2/pkg/scaling/cache/metricscache" ) +var log = logf.Log.WithName("scalers_cache") + type ScalersCache struct { ScaledObject *kedav1alpha1.ScaledObject - Generation int64 Scalers []ScalerBuilder - Logger logr.Logger Recorder record.EventRecorder } @@ -72,7 +72,7 @@ func (c *ScalersCache) GetMetricsForScaler(ctx context.Context, id int, metricNa if id < 0 || id >= len(c.Scalers) { return nil, fmt.Errorf("scaler with id %d not found. Len = %d", id, len(c.Scalers)) } - m, err := c.Scalers[id].Scaler.GetMetrics(ctx, metricName) + m, _, err := c.Scalers[id].Scaler.GetMetricsAndActivity(ctx, metricName) if err == nil { return m, nil } @@ -82,42 +82,63 @@ func (c *ScalersCache) GetMetricsForScaler(ctx context.Context, id int, metricNa return nil, err } - return ns.GetMetrics(ctx, metricName) + m, _, err = ns.GetMetricsAndActivity(ctx, metricName) + return m, err } -func (c *ScalersCache) IsScaledObjectActive(ctx context.Context, scaledObject *kedav1alpha1.ScaledObject) (bool, bool, []external_metrics.ExternalMetricValue) { - isActive := false +func (c *ScalersCache) GetScaledObjectState(ctx context.Context, scaledObject *kedav1alpha1.ScaledObject) (bool, bool, map[string]metricscache.MetricsRecord) { + logger := log.WithValues("scaledobject.Name", scaledObject.Name, "scaledObject.Namespace", scaledObject.Namespace, "scaleTarget.Name", scaledObject.Spec.ScaleTargetRef.Name) + + isScaledObjectActive := false isError := false + metricsRecord := map[string]metricscache.MetricsRecord{} + // Let's collect status of all scalers, no matter if any scaler raises error or is active for i, s := range c.Scalers { - isTriggerActive, err := s.Scaler.IsActive(ctx) - if err != nil { - var ns scalers.Scaler - ns, err = c.refreshScaler(ctx, i) - if err == nil { - isTriggerActive, err = ns.IsActive(ctx) + metricSpec := s.Scaler.GetMetricSpecForScaling(ctx) + + for _, spec := range metricSpec { + logger.V(1).Info("Looping metrics spec", "metricSpec", spec) + // skip cpu/memory resource scaler, these scalers are also always Active + if spec.External == nil { + isScaledObjectActive = true + continue } - } - logger := c.Logger.WithValues("scaledobject.Name", scaledObject.Name, "scaledObject.Namespace", scaledObject.Namespace, - "scaleTarget.Name", scaledObject.Spec.ScaleTargetRef.Name) + metric, isMetricActive, err := s.Scaler.GetMetricsAndActivity(ctx, spec.External.Metric.Name) + if err != nil { + var ns scalers.Scaler + ns, err = c.refreshScaler(ctx, i) + if err == nil { + metric, isMetricActive, err = ns.GetMetricsAndActivity(ctx, spec.External.Metric.Name) + } + } - if err != nil { - isError = true - logger.Error(err, "Error getting scale decision") - c.Recorder.Event(scaledObject, corev1.EventTypeWarning, eventreason.KEDAScalerFailed, err.Error()) - } else if isTriggerActive { - isActive = true - if externalMetricsSpec := s.Scaler.GetMetricSpecForScaling(ctx)[0].External; externalMetricsSpec != nil { - logger.V(1).Info("Scaler for scaledObject is active", "Metrics Name", externalMetricsSpec.Metric.Name) + if s.ScalerConfig.TriggerEnableCache { + metricsRecord[spec.External.Metric.Name] = metricscache.MetricsRecord{ + IsActive: isMetricActive, + Metric: metric, + ScalerError: err, + } } - if resourceMetricsSpec := s.Scaler.GetMetricSpecForScaling(ctx)[0].Resource; resourceMetricsSpec != nil { - logger.V(1).Info("Scaler for scaledObject is active", "Metrics Name", resourceMetricsSpec.Name) + + if err != nil { + isError = true + logger.Error(err, "Error getting scale decision") + c.Recorder.Event(scaledObject, corev1.EventTypeWarning, eventreason.KEDAScalerFailed, err.Error()) + } else if isMetricActive { + isScaledObjectActive = true + if spec.External != nil { + logger.V(1).Info("Scaler for scaledObject is active", "Metrics Name", spec.External.Metric.Name) + } + if spec.Resource != nil { + logger.V(1).Info("Scaler for scaledObject is active", "Metrics Name", spec.Resource.Name) + } } } } - return isActive, isError, []external_metrics.ExternalMetricValue{} + return isScaledObjectActive, isError, metricsRecord } func (c *ScalersCache) IsScaledJobActive(ctx context.Context, scaledJob *kedav1alpha1.ScaledJob) (bool, int64, int64) { @@ -180,27 +201,6 @@ func (c *ScalersCache) IsScaledJobActive(ctx context.Context, scaledJob *kedav1a return isActive, ceilToInt64(queueLength), ceilToInt64(maxValue) } -// TODO this is probably not needed, revisit whole package -func (c *ScalersCache) GetMetrics(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, error) { - var metrics []external_metrics.ExternalMetricValue - for i, s := range c.Scalers { - m, err := s.Scaler.GetMetrics(ctx, metricName) - if err != nil { - ns, err := c.refreshScaler(ctx, i) - if err != nil { - return metrics, err - } - m, err = ns.GetMetrics(ctx, metricName) - if err != nil { - return metrics, err - } - } - metrics = append(metrics, m...) - } - - return metrics, nil -} - func (c *ScalersCache) refreshScaler(ctx context.Context, id int) (scalers.Scaler, error) { if id < 0 || id >= len(c.Scalers) { return nil, fmt.Errorf("scaler with id %d not found. Len = %d", id, len(c.Scalers)) @@ -236,7 +236,7 @@ func (c *ScalersCache) Close(ctx context.Context) { for _, s := range scalers { err := s.Scaler.Close(ctx) if err != nil { - c.Logger.Error(err, "error closing scaler", "scaler", s) + log.Error(err, "error closing scaler", "scaler", s) } } } @@ -258,7 +258,7 @@ func (c *ScalersCache) getScaledJobMetrics(ctx context.Context, scaledJob *kedav maxValue := float64(0) scalerType := fmt.Sprintf("%T:", s) - scalerLogger := c.Logger.WithValues("ScaledJob", scaledJob.Name, "Scaler", scalerType) + scalerLogger := log.WithValues("ScaledJob", scaledJob.Name, "Scaler", scalerType) metricSpecs := s.Scaler.GetMetricSpecForScaling(ctx) @@ -268,12 +268,14 @@ func (c *ScalersCache) getScaledJobMetrics(ctx context.Context, scaledJob *kedav continue } - isTriggerActive, err := s.Scaler.IsActive(ctx) + // TODO fix metric name + _, isTriggerActive, err := s.Scaler.GetMetricsAndActivity(ctx, "") if err != nil { var ns scalers.Scaler ns, err = c.refreshScaler(ctx, i) if err == nil { - isTriggerActive, err = ns.IsActive(ctx) + // TODO fix metric name + _, isTriggerActive, err = ns.GetMetricsAndActivity(ctx, "") } } @@ -286,7 +288,7 @@ func (c *ScalersCache) getScaledJobMetrics(ctx context.Context, scaledJob *kedav targetAverageValue = getTargetAverageValue(metricSpecs) // TODO this should probably be `cache.GetMetricsForScaler(ctx, scalerIndex, metricName)` - metrics, err := s.Scaler.GetMetrics(ctx, metricSpecs[0].External.Metric.Name) + metrics, _, err := s.Scaler.GetMetricsAndActivity(ctx, metricSpecs[0].External.Metric.Name) if err != nil { scalerLogger.V(1).Info("Error getting scaler metrics, but continue", "Error", err) c.Recorder.Event(scaledJob, corev1.EventTypeWarning, eventreason.KEDAScalerFailed, err.Error()) diff --git a/pkg/scaling/cache/scalers_cache_test.go b/pkg/scaling/cache/scalers_cache_test.go index fd73a8a01ab..cc8c223b5b1 100644 --- a/pkg/scaling/cache/scalers_cache_test.go +++ b/pkg/scaling/cache/scalers_cache_test.go @@ -5,7 +5,6 @@ import ( "fmt" "testing" - "github.com/go-logr/logr" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" v2 "k8s.io/api/autoscaling/v2" @@ -70,7 +69,7 @@ func TestIsScaledJobActive(t *testing.T) { recorder := record.NewFakeRecorder(1) // Keep the current behavior // Assme 1 trigger only - scaledJobSingle := createScaledObject(0, 100, "") // testing default = max + scaledJobSingle := createScaledJob(0, 100, "") // testing default = max scalerSingle := []ScalerBuilder{{ Scaler: createScaler(ctrl, int64(20), int64(2), true, metricName), Factory: func() (scalers.Scaler, *scalers.ScalerConfig, error) { @@ -80,7 +79,6 @@ func TestIsScaledJobActive(t *testing.T) { cache := ScalersCache{ Scalers: scalerSingle, - Logger: logr.Discard(), Recorder: recorder, } @@ -100,7 +98,6 @@ func TestIsScaledJobActive(t *testing.T) { cache = ScalersCache{ Scalers: scalerSingle, - Logger: logr.Discard(), Recorder: recorder, } @@ -120,7 +117,7 @@ func TestIsScaledJobActive(t *testing.T) { } for index, scalerTestData := range scalerTestDatam { - scaledJob := createScaledObject(scalerTestData.MinReplicaCount, scalerTestData.MaxReplicaCount, scalerTestData.MultipleScalersCalculation) + scaledJob := createScaledJob(scalerTestData.MinReplicaCount, scalerTestData.MaxReplicaCount, scalerTestData.MultipleScalersCalculation) scalersToTest := []ScalerBuilder{{ Scaler: createScaler(ctrl, scalerTestData.Scaler1QueueLength, scalerTestData.Scaler1AverageValue, scalerTestData.Scaler1IsActive, scalerTestData.MetricName), Factory: func() (scalers.Scaler, *scalers.ScalerConfig, error) { @@ -145,7 +142,6 @@ func TestIsScaledJobActive(t *testing.T) { cache = ScalersCache{ Scalers: scalersToTest, - Logger: logr.Discard(), Recorder: recorder, } fmt.Printf("index: %d", index) @@ -164,7 +160,7 @@ func TestIsScaledJobActiveIfQueueEmptyButMinReplicaCountGreaterZero(t *testing.T recorder := record.NewFakeRecorder(1) // Keep the current behavior // Assme 1 trigger only - scaledJobSingle := createScaledObject(1, 100, "") // testing default = max + scaledJobSingle := createScaledJob(1, 100, "") // testing default = max scalerSingle := []ScalerBuilder{{ Scaler: createScaler(ctrl, int64(0), int64(1), true, metricName), Factory: func() (scalers.Scaler, *scalers.ScalerConfig, error) { @@ -174,7 +170,6 @@ func TestIsScaledJobActiveIfQueueEmptyButMinReplicaCountGreaterZero(t *testing.T cache := ScalersCache{ Scalers: scalerSingle, - Logger: logr.Discard(), Recorder: recorder, } @@ -248,7 +243,7 @@ type scalerTestData struct { MinReplicaCount int32 } -func createScaledObject(minReplicaCount int32, maxReplicaCount int32, multipleScalersCalculation string) *kedav1alpha1.ScaledJob { +func createScaledJob(minReplicaCount int32, maxReplicaCount int32, multipleScalersCalculation string) *kedav1alpha1.ScaledJob { if multipleScalersCalculation != "" { return &kedav1alpha1.ScaledJob{ Spec: kedav1alpha1.ScaledJobSpec{ @@ -278,9 +273,8 @@ func createScaler(ctrl *gomock.Controller, queueLength int64, averageValue int64 Value: *resource.NewQuantity(queueLength, resource.DecimalSI), }, } - scaler.EXPECT().IsActive(gomock.Any()).Return(isActive, nil) scaler.EXPECT().GetMetricSpecForScaling(gomock.Any()).Return(metricsSpecs) - scaler.EXPECT().GetMetrics(gomock.Any(), gomock.Any()).Return(metrics, nil) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Any()).Return(metrics, isActive, nil).Times(2) scaler.EXPECT().Close(gomock.Any()) return scaler } diff --git a/pkg/scaling/scale_handler.go b/pkg/scaling/scale_handler.go index cb057644346..de60184daeb 100644 --- a/pkg/scaling/scale_handler.go +++ b/pkg/scaling/scale_handler.go @@ -40,6 +40,7 @@ import ( "github.com/kedacore/keda/v2/pkg/prommetrics" "github.com/kedacore/keda/v2/pkg/scalers" "github.com/kedacore/keda/v2/pkg/scaling/cache" + "github.com/kedacore/keda/v2/pkg/scaling/cache/metricscache" "github.com/kedacore/keda/v2/pkg/scaling/executor" "github.com/kedacore/keda/v2/pkg/scaling/resolver" ) @@ -52,31 +53,33 @@ type ScaleHandler interface { GetScalersCache(ctx context.Context, scalableObject interface{}) (*cache.ScalersCache, error) ClearScalersCache(ctx context.Context, scalableObject interface{}) error - GetExternalMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, *metricsserviceapi.PromMetricsMsg, error) + GetScaledObjectMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, *metricsserviceapi.PromMetricsMsg, error) } type scaleHandler struct { - client client.Client - logger logr.Logger - scaleLoopContexts *sync.Map - scaleExecutor executor.ScaleExecutor - globalHTTPTimeout time.Duration - recorder record.EventRecorder - scalerCaches map[string]*cache.ScalersCache - lock *sync.RWMutex + client client.Client + logger logr.Logger + scaleLoopContexts *sync.Map + scaleExecutor executor.ScaleExecutor + globalHTTPTimeout time.Duration + recorder record.EventRecorder + scalerCaches map[string]*cache.ScalersCache + scalerCachesLock *sync.RWMutex + scaledObjectsMetricCache metricscache.MetricsCache } // NewScaleHandler creates a ScaleHandler object func NewScaleHandler(client client.Client, scaleClient scale.ScalesGetter, reconcilerScheme *runtime.Scheme, globalHTTPTimeout time.Duration, recorder record.EventRecorder) ScaleHandler { return &scaleHandler{ - client: client, - logger: logf.Log.WithName("scalehandler"), - scaleLoopContexts: &sync.Map{}, - scaleExecutor: executor.NewScaleExecutor(client, scaleClient, reconcilerScheme, recorder), - globalHTTPTimeout: globalHTTPTimeout, - recorder: recorder, - scalerCaches: map[string]*cache.ScalersCache{}, - lock: &sync.RWMutex{}, + client: client, + logger: logf.Log.WithName("scalehandler"), + scaleLoopContexts: &sync.Map{}, + scaleExecutor: executor.NewScaleExecutor(client, scaleClient, reconcilerScheme, recorder), + globalHTTPTimeout: globalHTTPTimeout, + recorder: recorder, + scalerCaches: map[string]*cache.ScalersCache{}, + scalerCachesLock: &sync.RWMutex{}, + scaledObjectsMetricCache: metricscache.NewMetricsCache(), } } @@ -170,78 +173,67 @@ func (h *scaleHandler) startScaleLoop(ctx context.Context, withTriggers *kedav1a } } +// TODO refactor this together with GetScalersCacheForScaledObject() +func (h *scaleHandler) GetScalersCache(ctx context.Context, scalableObject interface{}) (*cache.ScalersCache, error) { + withTriggers, err := asDuckWithTriggers(scalableObject) + if err != nil { + return nil, err + } + key := withTriggers.GenerateIdentifier() + + return h.performGetScalersCache(ctx, key, scalableObject, "", "", "") +} + func (h *scaleHandler) getScalersCacheForScaledObject(ctx context.Context, scaledObjectName, scaledObjectNamespace string) (*cache.ScalersCache, error) { key := kedav1alpha1.GenerateIdentifier("ScaledObject", scaledObjectNamespace, scaledObjectName) - h.lock.RLock() + return h.performGetScalersCache(ctx, key, nil, "ScaledObject", scaledObjectNamespace, scaledObjectName) +} + +func (h *scaleHandler) performGetScalersCache(ctx context.Context, key string, scalableObject interface{}, scalableObjectKind, scalableObjectNamespace, scalableObjectName string) (*cache.ScalersCache, error) { + h.scalerCachesLock.RLock() if cache, ok := h.scalerCaches[key]; ok { - h.lock.RUnlock() + h.scalerCachesLock.RUnlock() return cache, nil } - h.lock.RUnlock() + h.scalerCachesLock.RUnlock() - h.lock.Lock() - defer h.lock.Unlock() + h.scalerCachesLock.Lock() + defer h.scalerCachesLock.Unlock() if cache, ok := h.scalerCaches[key]; ok { return cache, nil } - scaledObject := &kedav1alpha1.ScaledObject{} - err := h.client.Get(ctx, types.NamespacedName{Name: scaledObjectName, Namespace: scaledObjectNamespace}, scaledObject) - if err != nil { - h.logger.Error(err, "failed to get ScaledObject", "name", scaledObjectName, "namespace", scaledObjectNamespace) - return nil, err - } - - podTemplateSpec, containerName, err := resolver.ResolveScaleTargetPodSpec(ctx, h.client, h.logger, scaledObject) - if err != nil { - return nil, err - } - - withTriggers, err := asDuckWithTriggers(scaledObject) - if err != nil { - return nil, err - } - - scalers, err := h.buildScalers(ctx, withTriggers, podTemplateSpec, containerName) - if err != nil { - return nil, err - } - - h.scalerCaches[key] = &cache.ScalersCache{ - ScaledObject: scaledObject, - Scalers: scalers, - Logger: h.logger, - Recorder: h.recorder, + if scalableObject == nil { + switch scalableObjectKind { + case "ScaledObject": + scaledObject := &kedav1alpha1.ScaledObject{} + err := h.client.Get(ctx, types.NamespacedName{Name: scalableObjectName, Namespace: scalableObjectNamespace}, scaledObject) + if err != nil { + h.logger.Error(err, "failed to get ScaledObject", "name", scalableObjectName, "namespace", scalableObjectNamespace) + return nil, err + } + scalableObject = scaledObject + case "ScaledJob": + scaledJob := &kedav1alpha1.ScaledJob{} + err := h.client.Get(ctx, types.NamespacedName{Name: scalableObjectName, Namespace: scalableObjectNamespace}, scaledJob) + if err != nil { + h.logger.Error(err, "failed to get ScaledJob", "name", scalableObjectName, "namespace", scalableObjectNamespace) + return nil, err + } + scalableObject = scaledJob + default: + err := fmt.Errorf("unknown ScalableObjectKind, got=%q", scalableObjectKind) + h.logger.Error(err, "unknown kind", "name", scalableObjectName, "namespace", scalableObjectNamespace) + return nil, err + } } - return h.scalerCaches[key], nil -} - -// TODO refactor this together with GetScalersCacheForScaledObject() -func (h *scaleHandler) GetScalersCache(ctx context.Context, scalableObject interface{}) (*cache.ScalersCache, error) { withTriggers, err := asDuckWithTriggers(scalableObject) if err != nil { return nil, err } - key := withTriggers.GenerateIdentifier() - - h.lock.RLock() - if cache, ok := h.scalerCaches[key]; ok && cache.Generation == withTriggers.Generation { - h.lock.RUnlock() - return cache, nil - } - h.lock.RUnlock() - - h.lock.Lock() - defer h.lock.Unlock() - if cache, ok := h.scalerCaches[key]; ok && cache.Generation == withTriggers.Generation { - return cache, nil - } else if ok { - cache.Close(ctx) - } - podTemplateSpec, containerName, err := resolver.ResolveScaleTargetPodSpec(ctx, h.client, h.logger, scalableObject) if err != nil { return nil, err @@ -253,10 +245,8 @@ func (h *scaleHandler) GetScalersCache(ctx context.Context, scalableObject inter } newCache := &cache.ScalersCache{ - Generation: withTriggers.Generation, - Scalers: scalers, - Logger: h.logger, - Recorder: h.recorder, + Scalers: scalers, + Recorder: h.recorder, } switch obj := scalableObject.(type) { case *kedav1alpha1.ScaledObject: @@ -277,8 +267,10 @@ func (h *scaleHandler) ClearScalersCache(ctx context.Context, scalableObject int key := withTriggers.GenerateIdentifier() - h.lock.Lock() - defer h.lock.Unlock() + go h.scaledObjectsMetricCache.Delete(key) + + h.scalerCachesLock.Lock() + defer h.scalerCachesLock.Unlock() if cache, ok := h.scalerCaches[key]; ok { h.logger.V(1).WithValues("key", key).Info("Removing entry from ScalersCache") cache.Close(ctx) @@ -338,8 +330,12 @@ func (h *scaleHandler) checkScalers(ctx context.Context, scalableObject interfac h.logger.Error(err, "Error getting scaledObject", "object", scalableObject) return } - isActive, isError, _ := cache.IsScaledObjectActive(ctx, obj) + isActive, isError, metricsRecords := cache.GetScaledObjectState(ctx, obj) h.scaleExecutor.RequestScale(ctx, obj, isActive, isError) + if len(metricsRecords) > 0 { + h.logger.Info("Storing metrics to cache", "scaledObject.Namespace", obj.Namespace, "scaledObject.Name", obj.Name, "metricsRecords", metricsRecords) + go h.scaledObjectsMetricCache.StoreRecords(obj.GenerateIdentifier(), metricsRecords) + } case *kedav1alpha1.ScaledJob: err = h.client.Get(ctx, types.NamespacedName{Name: obj.Name, Namespace: obj.Namespace}, obj) if err != nil { @@ -351,7 +347,7 @@ func (h *scaleHandler) checkScalers(ctx context.Context, scalableObject interfac } } -func (h *scaleHandler) GetExternalMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, *metricsserviceapi.PromMetricsMsg, error) { +func (h *scaleHandler) GetScaledObjectMetrics(ctx context.Context, scaledObjectName, scaledObjectNamespace, metricName string) (*external_metrics.ExternalMetricValueList, *metricsserviceapi.PromMetricsMsg, error) { var matchingMetrics []external_metrics.ExternalMetricValue exportedPromMetrics := metricsserviceapi.PromMetricsMsg{ @@ -379,14 +375,14 @@ func (h *scaleHandler) GetExternalMetrics(ctx context.Context, scaledObjectName, return nil, &exportedPromMetrics, err } - // let's check metrics for all scalers in a ScaledObject scalerError := false - scalers, scalerConfigs := cache.GetScalers() - - h.logger.V(1).WithValues("name", scaledObjectName, "namespace", scaledObjectNamespace, "metricName", metricName, "scalers", scalers).Info("Getting metric value") + scaledObjectIdentifier := scaledObject.GenerateIdentifier() + // let's check metrics for all scalers in a ScaledObject + scalers, scalerConfigs := cache.GetScalers() for scalerIndex := 0; scalerIndex < len(scalers); scalerIndex++ { metricSpecs := scalers[scalerIndex].GetMetricSpecForScaling(ctx) + scalerName := strings.Replace(fmt.Sprintf("%T", scalers[scalerIndex]), "*scalers.", "", 1) if scalerConfigs[scalerIndex].TriggerName != "" { scalerName = scalerConfigs[scalerIndex].TriggerName @@ -400,7 +396,23 @@ func (h *scaleHandler) GetExternalMetrics(ctx context.Context, scaledObjectName, // Filter only the desired metric if strings.EqualFold(metricSpec.External.Metric.Name, metricName) { - metrics, err := cache.GetMetricsForScaler(ctx, scalerIndex, metricName) + var metrics []external_metrics.ExternalMetricValue + + // if cache is defined for this scaler/metric, let's try to hit it first + metricsFoundInCache := false + if scalerConfigs[scalerIndex].TriggerEnableCache { + var metricsRecord metricscache.MetricsRecord + if metricsRecord, metricsFoundInCache = h.scaledObjectsMetricCache.ReadRecord(scaledObjectIdentifier, metricSpec.External.Metric.Name); metricsFoundInCache { + h.logger.V(1).Info("Reading metrics from cache", "scaledObject.Namespace", scaledObjectNamespace, "scaledObject.Name", scaledObjectName, "scaler", scalerName, "metricName", metricSpec.External.Metric.Name, "metricsRecord", metricsRecord) + metrics = metricsRecord.Metric + err = metricsRecord.ScalerError + } + } + + if !metricsFoundInCache { + metrics, err = cache.GetMetricsForScaler(ctx, scalerIndex, metricName) + h.logger.V(1).Info("Getting metrics from scaler", "scaledObject.Namespace", scaledObjectNamespace, "scaledObject.Name", scaledObjectName, "scaler", scalerName, "metricName", metricSpec.External.Metric.Name, "metrics", metrics, "scalerError", err) + } metrics, err = fallback.GetMetricsWithFallback(ctx, h.client, h.logger, metrics, err, metricName, scaledObject, metricSpec) if err != nil { scalerError = true @@ -476,7 +488,9 @@ func (h *scaleHandler) buildScalers(ctx context.Context, withTriggers *kedav1alp ScalableObjectNamespace: withTriggers.Namespace, ScalableObjectType: withTriggers.Kind, TriggerName: trigger.Name, + TriggerCacheDuration: trigger.CacheDuration, TriggerMetadata: trigger.Metadata, + TriggerEnableCache: trigger.EnableCache, ResolvedEnv: resolvedEnv, AuthParams: make(map[string]string), GlobalHTTPTimeout: h.globalHTTPTimeout, diff --git a/pkg/scaling/scale_handler_test.go b/pkg/scaling/scale_handler_test.go index 2acc0e2903b..848c1b74cf9 100644 --- a/pkg/scaling/scale_handler_test.go +++ b/pkg/scaling/scale_handler_test.go @@ -27,7 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" - logf "sigs.k8s.io/controller-runtime/pkg/log" + "k8s.io/metrics/pkg/apis/external_metrics" kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1" mock_scalers "github.com/kedacore/keda/v2/pkg/mock/mock_scaler" @@ -39,14 +39,19 @@ func TestCheckScaledObjectScalersWithError(t *testing.T) { ctrl := gomock.NewController(t) recorder := record.NewFakeRecorder(1) + metricsSpecs := []v2.MetricSpec{createMetricSpec(1, "metric-name")} + + scaler := mock_scalers.NewMockScaler(ctrl) + scaler.EXPECT().GetMetricSpecForScaling(gomock.Any()).Return(metricsSpecs) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Any()).Return([]external_metrics.ExternalMetricValue{}, false, errors.New("some error")) + scaler.EXPECT().Close(gomock.Any()) + factory := func() (scalers.Scaler, *scalers.ScalerConfig, error) { scaler := mock_scalers.NewMockScaler(ctrl) - scaler.EXPECT().IsActive(gomock.Any()).Return(false, errors.New("some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Any()).Return([]external_metrics.ExternalMetricValue{}, false, errors.New("some error")) scaler.EXPECT().Close(gomock.Any()) return scaler, &scalers.ScalerConfig{}, nil } - scaler, _, err := factory() - assert.Nil(t, err) scaledObject := kedav1alpha1.ScaledObject{ ObjectMeta: metav1.ObjectMeta{ @@ -65,11 +70,10 @@ func TestCheckScaledObjectScalersWithError(t *testing.T) { Scaler: scaler, Factory: factory, }}, - Logger: logf.Log.WithName("scalehandler"), Recorder: recorder, } - isActive, isError, _ := cache.IsScaledObjectActive(context.TODO(), &scaledObject) + isActive, isError, _ := cache.GetScaledObjectState(context.TODO(), &scaledObject) cache.Close(context.Background()) assert.Equal(t, false, isActive) @@ -80,12 +84,12 @@ func TestCheckScaledObjectFindFirstActiveNotIgnoreOthers(t *testing.T) { ctrl := gomock.NewController(t) recorder := record.NewFakeRecorder(1) - metricsSpecs := []v2.MetricSpec{createMetricSpec(1)} + metricsSpecs := []v2.MetricSpec{createMetricSpec(1, "metric-name")} activeFactory := func() (scalers.Scaler, *scalers.ScalerConfig, error) { scaler := mock_scalers.NewMockScaler(ctrl) - scaler.EXPECT().IsActive(gomock.Any()).Return(true, nil) - scaler.EXPECT().GetMetricSpecForScaling(gomock.Any()).Times(2).Return(metricsSpecs) + scaler.EXPECT().GetMetricSpecForScaling(gomock.Any()).Return(metricsSpecs) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Any()).Return([]external_metrics.ExternalMetricValue{}, true, nil) scaler.EXPECT().Close(gomock.Any()) return scaler, &scalers.ScalerConfig{}, nil } @@ -94,12 +98,14 @@ func TestCheckScaledObjectFindFirstActiveNotIgnoreOthers(t *testing.T) { failingFactory := func() (scalers.Scaler, *scalers.ScalerConfig, error) { scaler := mock_scalers.NewMockScaler(ctrl) - scaler.EXPECT().IsActive(gomock.Any()).Return(false, errors.New("some error")) + scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Any()).Return([]external_metrics.ExternalMetricValue{}, false, errors.New("some error")) scaler.EXPECT().Close(gomock.Any()) return scaler, &scalers.ScalerConfig{}, nil } - failingScaler, _, err := failingFactory() - assert.Nil(t, err) + failingScaler := mock_scalers.NewMockScaler(ctrl) + failingScaler.EXPECT().GetMetricSpecForScaling(gomock.Any()).Return(metricsSpecs) + failingScaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Any()).Return([]external_metrics.ExternalMetricValue{}, false, errors.New("some error")) + failingScaler.EXPECT().Close(gomock.Any()) scaledObject := &kedav1alpha1.ScaledObject{ ObjectMeta: metav1.ObjectMeta{ @@ -123,24 +129,26 @@ func TestCheckScaledObjectFindFirstActiveNotIgnoreOthers(t *testing.T) { scalersCache := cache.ScalersCache{ Scalers: scalers, - Logger: logf.Log.WithName("scalercache"), Recorder: recorder, } - isActive, isError, _ := scalersCache.IsScaledObjectActive(context.TODO(), scaledObject) + isActive, isError, _ := scalersCache.GetScaledObjectState(context.TODO(), scaledObject) scalersCache.Close(context.Background()) assert.Equal(t, true, isActive) assert.Equal(t, true, isError) } -func createMetricSpec(averageValue int64) v2.MetricSpec { +func createMetricSpec(averageValue int64, metricName string) v2.MetricSpec { qty := resource.NewQuantity(averageValue, resource.DecimalSI) return v2.MetricSpec{ External: &v2.ExternalMetricSource{ Target: v2.MetricTarget{ AverageValue: qty, }, + Metric: v2.MetricIdentifier{ + Name: metricName, + }, }, } }