feat(CustomMetrics): Allow Different App to Send Custom Metrics #3211

asalan316 · 2024-09-17T07:12:35Z

Previously, only the application sending custom metrics to the autoscaler could be scaled.

The new feature enables users to scale an application based on custom metrics sent by another app (Custom Metrics Producer), This is achieve if:

Both applications are bound to same application autoscaler instance
The application to scale has the "metric_submission_strategy.allow_from" set to bound_app during the binding process

How It Works

To enable this feature, specify the metric_submission_strategy.allow_from : "bound_app" parameter in the configuration object during the binding process with the application to be scaled (as shown below)

Binding Configuration (aka.scaling policy)

{
  "configuration": {
    "custom_metrics": {
      "metric_submission_strategy": {
        "allow_from": "bound_app"
      }
    }
  },
  "instance_max_count":4,
  "instance_min_count":1,
  "scaling_rules":[
    {
      "metric_type":"test_metric",
      "threshold":500,
      "operator":">",
      "adjustment":"+1"
    }, {
      "metric_type":"test_metric",
      "threshold":100,
      "operator":"<",
      "adjustment":"-1"
    }
  ]
}

Bind to Applications

# Bind autoscaler to application to scale

cf bind-service  app-to-scale  autosocaler -c <binding Configuration.json>

# Bind autoscaler to custom metrics producer app

cf bind-service  custom-metrics-producer-app  autosocaler

Other Changes

Introduced additional database bindingDB in metricsforwarder service to retrieve binding information
Added New Database schema to validate custom metrics strategy - allowed values are bound_app and same_app (default)
Introduced a configuration structure to differentiate between policy and binding configurations
Binding parameters are retrieved as follows:
- For existing bindings (without configuration) - only policy json is displayed (existing behavior)
- For new bindings - both the configuration and policy json are displayed in the response

src/autoscaler/eventgenerator/metric/fetcher_test.go

src/autoscaler/metricsforwarder/server/auth/authenticator.go

fix api->broker tests fix acceptance test add some logging fix test fix db add acceptance tests fix db test by setting default to same_app custom metric strategy should be at the scaling side add some tests and refactor fix linters refactor naming

• [FAILED] [0.063 seconds] BindingSqldb CreateServiceBinding When service instance exists When service binding is being created first time [It] should succeed /__w/app-autoscaler-release/app-autoscaler-release/src/autoscaler/db/sqldb/binding_sqldb_test.go:374 Timeline >> {"timestamp":"1729248292.991818190","source":"binding-sqldb-test","message":"binding-sqldb-test.Cleaning service binding test-binding-id-4 failed:doesn't exist","log_level":1,"data":{}} {"timestamp":"1729248292.992060423","source":"binding-sqldb-test","message":"binding-sqldb-test.Cleaning service binding test-binding-id-42 failed:doesn't exist","log_level":1,"data":{}} {"timestamp":"1729248292.992296696","source":"binding-sqldb-test","message":"binding-sqldb-test.Cleaning service binding test-binding-id-43 failed:doesn't exist","log_level":1,"data":{}} {"timestamp":"1729248293.024851084","source":"binding-sqldb-test","message":"binding-sqldb-test.Cleaning service instance test-instance-id-42 failed:doesn't exist","log_level":1,"data":{}} {"timestamp":"1729248293.025138617","source":"binding-sqldb-test","message":"binding-sqldb-test.Cleaning service instance test-instance-id-43 failed:doesn't exist","log_level":1,"data":{}} {"timestamp":"1729248293.026132822","source":"binding-sqldb-test","message":"binding-sqldb-test.get-service-instance","log_level":2,"data":{"error":"sql: no rows in result set","query":"SELECT * FROM service_instance WHERE service_instance_id = $1","serviceInstanceId":"test-instance-id-4"}} {"timestamp":"1729248293.028556824","source":"binding-sqldb-test","message":"binding-sqldb-test.create-service-binding","log_level":2,"data":{"appid":"test-app-id-4","bindingid":"test-binding-id-4","customMetricsStrategy":"","error":"ERROR: insert or update on table \"binding\" violates foreign key constraint \"fk_binding_custom_metrics_strategy\" (SQLSTATE 23503)","query":"INSERT INTO binding(binding_id, service_instance_id, app_id, created_at, custom_metrics_strategy) VALUES($1, $2, $3, $4,$5)","serviceinstanceid":"test-instance-id-4"}} [FAILED] in [It] - /__w/app-autoscaler-release/app-autoscaler-release/src/autoscaler/db/sqldb/binding_sqldb_test.go:375 @ 10/18/24 11:44:53.029 << Timeline [FAILED] Unexpected error: <*pgconn.PgError | 0xc0003be200>: ERROR: insert or update on table "binding" violates foreign key constraint "fk_binding_custom_metrics_strategy" (SQLSTATE 23503) { Severity: "ERROR", SeverityUnlocalized: "ERROR", Code: "23503", Message: "insert or update on table \"binding\" violates foreign key constraint \"fk_binding_custom_metrics_strategy\"", Detail: "Key (custom_metrics_strategy)=() is not present in table \"metrics_submission\".", Hint: "", Position: 0, InternalPosition: 0, InternalQuery: "", Where: "", SchemaName: "public", TableName: "binding", ColumnName: "", DataTypeName: "", ConstraintName: "fk_binding_custom_metrics_strategy", File: "ri_triggers.c", Line: 2608, Routine: "ri_ReportViolation", } occurred In [It] at: /__w/app-autoscaler-release/app-autoscaler-release/src/autoscaler/db/sqldb/binding_sqldb_test.go:375 @ 10

asalan316 · 2024-10-23T08:03:16Z

src/acceptance/assets/app/go_app/internal/app/custom_metrics.go

@@ -103,7 +113,7 @@ func (*CustomMetricAPIClient) PostCustomMetric(ctx context.Context, appConfig *c
 	}

 	metrics := createSingletonMetric(metricName, metricValue)
-
+	logger.Info("sending metric to autoscaler for app", "appId", appId, "metricName", metricName, "metricValue", metricValue)


FYI: included for better logging from the test side

• Abstracted database configuration logic into configureDb function for reuse • Extended configuration to support BindingDb alongside PolicyDb • Updated tests to reflect additional BindingDb configuration • Added binding_db tag to mta.tpl.yaml resources

sonarcloud · 2024-10-25T09:57:31Z

Quality Gate failed

Failed conditions
1 Security Hotspot
4.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

bonzofenix · 2024-10-23T11:40:24Z

jobs/metricsforwarder/spec

@@ -24,6 +24,10 @@ templates:
  policy_db.crt.erb: config/certs/policy_db/crt
  policy_db.key.erb: config/certs/policy_db/key

+  binding_db_ca.crt.erb: config/certs/binding_db/ca.crt


Considering we have added a new database for the metrics forwarder, we need to ensure that the MTA deployment is functioning correctly: https://github.com/cloudfoundry/app-autoscaler-release/actions/runs/11464983060/job/31903137875?pr=3211. Currently, it seems to be failing due to a different error.

We will need to add the bindingDB tag in the PostgreSQL user-provided service: https://github.com/cloudfoundry/app-autoscaler-release/blob/main/src/autoscaler/mta.tpl.yaml#L43.

Additionally, we must check the metrics forwarder configuration to ensure we are loading the database when running on Cloud Foundry as well: https://github.com/cloudfoundry/app-autoscaler-release/blob/main/src/autoscaler/metricsforwarder/config/config.go#L124.cloudfoundry/app-autoscaler-release/blob/main/src/autoscaler/metricsforwarder/config/config.go#L124

@asalan316 this should fix it: a373e94

bonzofenix · 2024-10-25T08:47:35Z

src/acceptance/app/custom_metric_test.go

+		// Therefore, only mtls connection will be supported for custom metrics in future
+		Context("when scaling by custom metrics", func() {
+			BeforeEach(func() {
+				//instanceName = CreatePolicy(cfg, appToScaleName, appToScaleGUID, policy)


we might want to remove this commented test

bonzofenix · 2024-10-25T08:48:29Z

src/acceptance/app/custom_metric_test.go

+		// Therefore, only mtls connection will be supported for custom metrics in future
+		Context("when scaling by custom metrics", func() {
+			BeforeEach(func() {
+				//instanceName = CreatePolicy(cfg, appToScaleName, appToScaleGUID, policy)


should we remove this comment?

bonzofenix · 2024-10-25T09:18:33Z

src/acceptance/assets/app/go_app/internal/app/custom_metrics.go

The changes in this file are all for logging purposes, right? The app that sends metrics should behave the same, even if it is not scaling itself.

bonzofenix · 2024-10-25T09:58:18Z

src/acceptance/broker/broker_test.go

 			policy, err := os.ReadFile(policyFile)
+			fmt.Println("policy", string(policy)) //FIXME


should we remove this ?

bonzofenix · 2024-10-25T10:01:15Z

src/acceptance/broker/broker_test.go

@@ -192,6 +223,7 @@ var _ = Describe("AutoScaler Service Broker", func() {
 		var instance serviceInstance
 		It("should update a service instance from one plan to another plan", func() {
 			servicePlans := GetServicePlans(cfg)
+			fmt.Println("servicePlans", servicePlans)


mayber we remove this too or turn it into a By statement?

bonzofenix · 2024-10-25T13:06:29Z

src/autoscaler/metricsforwarder/server/auth/auth_test.go

+					testAppId = "app-to-scale-id"
+					vars["appid"] = testAppId
+					req.Header.Add("X-Forwarded-Client-Cert", MustReadXFCCcert(validClientCert2))
+					// ToDO: this should be read via configurations aka scaling policy binding parameters


should we look into this TODO

asalan316 force-pushed the neighbour-app-cutom-metrics branch from fae6122 to 9859b16 Compare September 17, 2024 10:08

asalan316 added the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Sep 17, 2024

asalan316 force-pushed the neighbour-app-cutom-metrics branch 3 times, most recently from 3edfa26 to c217dd7 Compare September 20, 2024 12:59

asalan316 force-pushed the neighbour-app-cutom-metrics branch from a4de8af to f63ba3c Compare September 27, 2024 14:18

asalan316 commented Sep 30, 2024

View reviewed changes

src/autoscaler/eventgenerator/metric/fetcher_test.go Outdated Show resolved Hide resolved

asalan316 commented Sep 30, 2024

View reviewed changes

src/autoscaler/metricsforwarder/server/auth/authenticator.go Outdated Show resolved Hide resolved

asalan316 force-pushed the neighbour-app-cutom-metrics branch 8 times, most recently from b30a9fc to 200be0b Compare October 2, 2024 14:32

asalan316 force-pushed the neighbour-app-cutom-metrics branch 7 times, most recently from 31ee44d to 2e9256a Compare October 17, 2024 13:03

asalan316 changed the title ~~Neighbour app custom metrics~~ Allow Different App to Send Custom Metrics Oct 17, 2024

asalan316 changed the title ~~Allow Different App to Send Custom Metrics~~ feat(CustomMetrics): Allow Different App to Send Custom Metrics Oct 17, 2024

asalan316 force-pushed the neighbour-app-cutom-metrics branch 5 times, most recently from 1fb8dce to 840fa60 Compare October 18, 2024 12:46

asalan316 added 2 commits October 18, 2024 14:52

test for go_app

a554409

asalan316 force-pushed the neighbour-app-cutom-metrics branch 3 times, most recently from c5d8a30 to 18a84ce Compare October 18, 2024 14:07

asalan316 marked this pull request as ready for review October 18, 2024 14:19

asalan316 force-pushed the neighbour-app-cutom-metrics branch from 18a84ce to 8e4bf83 Compare October 18, 2024 14:31

asalan316 added allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. and removed allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. labels Oct 21, 2024

asalan316 force-pushed the neighbour-app-cutom-metrics branch 3 times, most recently from b105a31 to 3b57893 Compare October 22, 2024 11:40

asalan316 added the skip-cleanup label Oct 22, 2024

implement GetBinding to include binding configs

1cd4ac9

asalan316 force-pushed the neighbour-app-cutom-metrics branch from 3b57893 to 1cd4ac9 Compare October 22, 2024 12:49

remove custom metrics authentication details from the binding configs

979581a

asalan316 force-pushed the neighbour-app-cutom-metrics branch from b67cb3c to 979581a Compare October 22, 2024 14:08

adept broker acceptance to display binding configs

dec3f28

asalan316 force-pushed the neighbour-app-cutom-metrics branch from 8d27a10 to dec3f28 Compare October 22, 2024 17:02

asalan316 commented Oct 23, 2024

View reviewed changes

bonzofenix force-pushed the neighbour-app-cutom-metrics branch from 0eee25c to dec3f28 Compare October 23, 2024 15:38

bonzofenix and others added 2 commits October 24, 2024 15:27

Merge branch 'main' into neighbour-app-cutom-metrics

3e161f2

bonzofenix removed the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Oct 24, 2024

Updated Makefile to deploy with version-rule ALL for autoscaler

5b1e98f

bonzofenix added the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Oct 25, 2024

Merge branch 'main' into neighbour-app-cutom-metrics

8e785c2

bonzofenix requested changes Oct 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(CustomMetrics): Allow Different App to Send Custom Metrics #3211

feat(CustomMetrics): Allow Different App to Send Custom Metrics #3211

asalan316 commented Sep 17, 2024 •

edited

Loading

asalan316 Oct 23, 2024

sonarcloud bot commented Oct 25, 2024

bonzofenix Oct 23, 2024

bonzofenix Oct 24, 2024

bonzofenix Oct 25, 2024

bonzofenix Oct 25, 2024

bonzofenix Oct 25, 2024

bonzofenix Oct 25, 2024

bonzofenix Oct 25, 2024

bonzofenix Oct 25, 2024

		policy, err := os.ReadFile(policyFile)
		fmt.Println("policy", string(policy)) //FIXME

feat(CustomMetrics): Allow Different App to Send Custom Metrics #3211

Are you sure you want to change the base?

feat(CustomMetrics): Allow Different App to Send Custom Metrics #3211

Conversation

asalan316 commented Sep 17, 2024 • edited Loading

Other Changes

Choose a reason for hiding this comment

sonarcloud bot commented Oct 25, 2024

Quality Gate failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asalan316 commented Sep 17, 2024 •

edited

Loading