Skip to content

Move etag caching middleware after rack middleware

Sylvester Chin requested to merge sc1-fix-etag-cache-middleware-order into master

What does this MR do and why?

This MR moves Gitlab::EtagCaching::Middleware after Gitlab::Metrics::RackMiddleware to ensure that WebTransaction.current is not nil.

Resolves a missing-metric issue noticed in gitlab-com/gl-infra/scalability#2441 (closed).

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

In a hacky fashion, I applied the following change and ran rake middleware to check the order in a puma/sidekiq.

diff --git a/config/initializers/zz_metrics.rb b/config/initializers/zz_metrics.rb
index c277692e4077..ecc03b0a17bb 100644
--- a/config/initializers/zz_metrics.rb
+++ b/config/initializers/zz_metrics.rb
@@ -2,7 +2,7 @@

 # This file was prefixed with zz_ because we want to load it the last!
 # See: https://gitlab.com/gitlab-org/gitlab-foss/issues/55611
-if Gitlab::Metrics.enabled? && Gitlab::Runtime.application?
+if true || (Gitlab::Metrics.enabled? && Gitlab::Runtime.application?)
   require 'pathname'
   require 'connection_pool'

Using text compare, it is a slight change in ordering between master (left) and current (right):

Screenshot_2023-07-25_at_2.26.02_PM

Click to see current branch 
➜  gitlab git:(sc1-fix-etag-cache-middleware-order) ✗ rake middleware
use Raven::Rack
use ActionDispatch::RequestId
use Labkit::Middleware::Rack
use Gitlab::Metrics::RequestsRackMiddleware
use ActionDispatch::HostAuthorization
use Gitlab::Middleware::SidekiqWebStatic
use Rack::Sendfile
use Gitlab::Middleware::RackMultipartTempfileFactory
use Gitlab::Webpack::DevServerMiddleware
use Gitlab::Middleware::Static
use ActionDispatch::Executor
use Gitlab::Middleware::CompressedJson
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use Rack::MethodOverride
use Rack::Timeout
use RequestStore::Middleware
use Gitlab::Middleware::WebhookRecursionDetection
use Gitlab::Middleware::RequestContext
use Gitlab::Middleware::HandleIpSpoofAttackError
use ActionDispatch::RemoteIp
use Sprockets::Rails::QuietAssets
use Rails::Rack::Logger
use Gitlab::Middleware::HandleMalformedStrings
use Gitlab::Middleware::BasicHealthCheck
use ActionDispatch::ShowExceptions
use Sentry::Rails::CaptureExceptions
use ActionDispatch::DebugExceptions
use BetterErrors::Middleware
use Sentry::Rails::RescuedExceptionInterceptor
use ActionDispatch::ActionableExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use Gitlab::Middleware::SameSiteCookies
use ActionDispatch::Cookies
use ActionDispatch::Session::RedisStore
use ActionDispatch::Flash
use Gitlab::Middleware::ReadOnly
use ActionDispatch::ContentSecurityPolicy::Middleware
use ActionDispatch::PermissionsPolicy::Middleware
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
use Rack::TempfileReaper
use Rack::Cors
use Warden::Manager
use Rack::Attack
use Rack::Static
use Gitlab::Middleware::Multipart
use ApolloUploadServer::Middleware
use Rack::Attack
use Bullet::Rack
use BatchLoader::Middleware
use Gitlab::Middleware::QueryAnalyzer
use Gitlab::Middleware::Go
use Gitlab::Jira::Middleware
use Gitlab::Metrics::RackMiddleware
use Gitlab::EtagCaching::Middleware
use Gitlab::Middleware::RailsQueueDuration
use Gitlab::Database::LoadBalancing::RackMiddleware
use Gitlab::QueryLimiting::Middleware
use Gitlab::Middleware::Speedscope
use Gitlab::Middleware::MemoryReport
use Gitlab::Metrics::ElasticsearchRackMiddleware
use Flipper::Middleware::Memoizer
use ActionDispatch::Static
use OmniAuth::Strategies::GoogleOauth2
use Gitlab::Experiment::Middleware
run Gitlab::Application.routes
Click to show master branch 
➜  gitlab git:(master) ✗ rake middleware
use Raven::Rack
use ActionDispatch::RequestId
use Labkit::Middleware::Rack
use Gitlab::Metrics::RequestsRackMiddleware
use ActionDispatch::HostAuthorization
use Gitlab::Middleware::SidekiqWebStatic
use Rack::Sendfile
use Gitlab::Middleware::RackMultipartTempfileFactory
use Gitlab::Webpack::DevServerMiddleware
use Gitlab::Middleware::Static
use ActionDispatch::Executor
use Gitlab::Middleware::CompressedJson
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use Rack::MethodOverride
use Rack::Timeout
use RequestStore::Middleware
use Gitlab::Middleware::WebhookRecursionDetection
use Gitlab::Middleware::RequestContext
use Gitlab::Middleware::HandleIpSpoofAttackError
use ActionDispatch::RemoteIp
use Sprockets::Rails::QuietAssets
use Rails::Rack::Logger
use Gitlab::Middleware::HandleMalformedStrings
use Gitlab::Middleware::BasicHealthCheck
use ActionDispatch::ShowExceptions
use Sentry::Rails::CaptureExceptions
use ActionDispatch::DebugExceptions
use BetterErrors::Middleware
use Sentry::Rails::RescuedExceptionInterceptor
use ActionDispatch::ActionableExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use Gitlab::Middleware::SameSiteCookies
use ActionDispatch::Cookies
use ActionDispatch::Session::RedisStore
use ActionDispatch::Flash
use Gitlab::Middleware::ReadOnly
use ActionDispatch::ContentSecurityPolicy::Middleware
use ActionDispatch::PermissionsPolicy::Middleware
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
use Rack::TempfileReaper
use Rack::Cors
use Warden::Manager
use Rack::Attack
use Rack::Static
use Gitlab::Middleware::Multipart
use ApolloUploadServer::Middleware
use Rack::Attack
use Bullet::Rack
use BatchLoader::Middleware
use Gitlab::Middleware::QueryAnalyzer
use Gitlab::EtagCaching::Middleware
use Gitlab::Middleware::Go
use Gitlab::Jira::Middleware
use Gitlab::Metrics::RackMiddleware
use Gitlab::Middleware::RailsQueueDuration
use Gitlab::Database::LoadBalancing::RackMiddleware
use Gitlab::QueryLimiting::Middleware
use Gitlab::Middleware::Speedscope
use Gitlab::Middleware::MemoryReport
use Gitlab::Metrics::ElasticsearchRackMiddleware
use Flipper::Middleware::Memoizer
use ActionDispatch::Static
use OmniAuth::Strategies::GoogleOauth2
use Gitlab::Experiment::Middleware
Before After

How to set up and validate locally

On master branch, navigate to http://gdk.test:3000/gitlab-org/gitlab-test/-/issues/35

➜  gitlab git:(master) ✗ curl 'gdk.test:3000/-/metrics' | rg etag
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 1426k  100 1426k    0     0  15.4M      0 --:--:-- --:--:-- --:--:-- 16.3M

Restart gdk restart rails-web to reload the middlewares

On this branch, go to http://gdk.test:3000/gitlab-org/gitlab-test/-/issues/35

➜  gitlab git:(sc1-fix-etag-cache-middleware-order) ✗ curl 'gdk.test:3000/-/metrics' | rg etag
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0# HELP gitlab_transaction_event_etag_caching_cache_hit_total Multiprocess metric
# TYPE gitlab_transaction_event_etag_caching_cache_hit_total counter
gitlab_transaction_event_etag_caching_cache_hit_total{action="",controller="",endpoint="issue_notes",feature_category=""} 1
gitlab_transaction_event_etag_caching_cache_hit_total{action="",controller="",endpoint="issue_title",feature_category=""} 1
# HELP gitlab_transaction_event_etag_caching_header_missing_total Multiprocess metric
# TYPE gitlab_transaction_event_etag_caching_header_missing_total counter
gitlab_transaction_event_etag_caching_header_missing_total{action="",controller="",endpoint="issue_notes",feature_category=""} 1
gitlab_transaction_event_etag_caching_header_missing_total{action="",controller="",endpoint="issue_title",feature_category=""} 1
# HELP gitlab_transaction_event_etag_caching_middleware_used_total Multiprocess metric
# TYPE gitlab_transaction_event_etag_caching_middleware_used_total counter
gitlab_transaction_event_etag_caching_middleware_used_total{action="",controller="",endpoint="issue_notes",feature_category=""} 2
gitlab_transaction_event_etag_caching_middleware_used_total{action="",controller="",endpoint="issue_title",feature_category=""} 2
100 1414k  100 1414k    0     0  13.6M      0 --:--:-- --:--:-- --:--:-- 14.3M

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Gregorius Marco

Merge request reports