Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(metric-architecture)!: remove loggregator support and make log-cache the default metric source #2983

Merged
merged 39 commits into from
Jun 17, 2024

Conversation

geigerj0
Copy link
Contributor

@geigerj0 geigerj0 commented Jun 4, 2024

Some context upfront

Application Autoscaler comes with two different deployment variants:

  1. Retrieving metrics from the Loggregator metric stack (legacy): This variant was initially introduced because Log Cache wasn't available at that time. It requires the metricsserver and metricssgateway jobs to be deployed via the deployment manifest.
  2. Retrieving metrics from the Log Cache metric stack (new): This is the successor of the Loggregator variant that got introduced with Log cache client #659. This variant works without the metricsserver and metricssgateway jobs but requires the feature flag use_log_cache and to be configured and valid Log Cache properties to be set under the metricscollector property of the eventgenerator job.

What is this breaking change about?

This PR introduces a breaking change by removing the possibility to deploy Application Autoscaler with metrics retrieved from the Loggregator metric stack. In case you deploy Application Autoscaler with metricsserver- and metricsgateway-job, your deployment will fail.

foo

Why is this breaking change necessary?

  • There is no need to support two metric retrieval variants (Loggregator and Log Cache).
  • Removing the Loggregator variant allows us to eliminate a significant amount of code and complexity (e.g., metricsserver and metricsgateway).

Migration FAQs

How can I switch from Loggregator to Log Cache?

  1. Remove the configuration of the metricsserver and metricsgateway jobs from your deployment manifest.
  2. Configure metricscollector of the eventgenerator job with valid Log Cache properties. Example:
    metricscollector:
      ca_cert: ((/bosh-autoscaler/cf/log_cache.ca))
      client_cert: ((/bosh-autoscaler/cf/log_cache.certificate))
      client_key: ((/bosh-autoscaler/cf/log_cache.private_key))
      port: 8080
      host: logcache

Do I need to take action if I am already using the feature flag use_log_cache?

If you are already using the Log Cache variant and have configured the use_log_cache feature flag in your deployment manifest, everything will continue to work. However, note that this feature flag has been removed, so configuring use_log_cache no longer has any effect. Feel free to remove use_log_cache from your deployment manifest .

geigerj0 added 2 commits June 4, 2024 13:11
* remove anchor containing all ops-files for loggregator deployment
* remove acceptance-job
* run performance tests with log-cache sending metrics to syslog-server
@geigerj0 geigerj0 force-pushed the appautoscaler-703/make-logcache-default branch 2 times, most recently from 1c73767 to 63ae3ca Compare June 4, 2024 11:23
@geigerj0 geigerj0 force-pushed the appautoscaler-703/make-logcache-default branch from 63ae3ca to 907e134 Compare June 4, 2024 11:24
@geigerj0 geigerj0 changed the title feat(metric-architecture)!: remove loggregator support and make log-cache the default feat(metric-architecture)!: remove loggregator support and make log-cache the default metric source Jun 5, 2024
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@geigerj0 geigerj0 force-pushed the appautoscaler-703/make-logcache-default branch 3 times, most recently from a7f2d31 to 772f71a Compare June 5, 2024 08:57
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@geigerj0 geigerj0 force-pushed the appautoscaler-703/make-logcache-default branch 3 times, most recently from 9cb18e7 to 75ac14e Compare June 7, 2024 14:57
@geigerj0 geigerj0 force-pushed the appautoscaler-703/make-logcache-default branch 2 times, most recently from 8a7819b to b874465 Compare June 7, 2024 16:07
@geigerj0 geigerj0 marked this pull request as ready for review June 11, 2024 15:09
Copy link
Contributor

@bonzofenix bonzofenix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial review, I still have more code to review. I will finish tomorrow morning

README.md Outdated Show resolved Hide resolved
ci/autoscaler/pipeline.yml Outdated Show resolved Hide resolved
example/operation/cf-mysql-db.yml Show resolved Hide resolved
example/operation/external-db.yml Show resolved Hide resolved
scripts/generate_test_certs.sh Show resolved Hide resolved
spec/jobs/eventgenerator/eventgenerator_spec.rb Outdated Show resolved Hide resolved
docs/images/autoscaler.svg Show resolved Hide resolved
docs/images/eventgenerator.svg Outdated Show resolved Hide resolved
example/operation/cf-mysql-db.yml Show resolved Hide resolved
templates/app-autoscaler.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@bonzofenix bonzofenix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@geigerj0 geigerj0 force-pushed the appautoscaler-703/make-logcache-default branch from fd032ee to 3f4e6df Compare June 14, 2024 15:35
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
40.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@bonzofenix bonzofenix merged commit a25c18c into main Jun 17, 2024
23 of 24 checks passed
@bonzofenix bonzofenix deleted the appautoscaler-703/make-logcache-default branch June 17, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. breaking-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants