From a92337013e766fec593d1a32773351d541b49d2e Mon Sep 17 00:00:00 2001 From: Malaber Date: Wed, 15 Apr 2026 10:16:18 +0200 Subject: [PATCH] Move profiling out of prod image --- AGENTS.md | 24 ++++++++++++ app/controllers/tournaments_controller.rb | 19 ++++++++- app/services/request_profiling.rb | 43 --------------------- docker-compose.blackbox.yml | 4 ++ lib/local/request_profiling.rb | 45 ++++++++++++++++++++++ spec/e2e/http/tournament_rendering_spec.rb | 21 ++++++---- spec/e2e_known_behaviors.md | 2 +- tasks.py | 6 +++ 8 files changed, 112 insertions(+), 52 deletions(-) delete mode 100644 app/services/request_profiling.rb create mode 100644 lib/local/request_profiling.rb diff --git a/AGENTS.md b/AGENTS.md index 2dc88db..dfd7cf5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,6 +112,30 @@ Instead, add or update an `inv` task and then have CI or docs call that task. If a new backend capability introduces a meaningful setup, verification, fixture, or scenario workflow, add or extend a task for it as part of the same change. +## Local-Only Helper Code + +If helper code exists only for local development, local profiling, or test/E2E support, +prefer placing it in a path that the production Docker image does not copy. + +Current production image copies: + +- `app` +- `bin` +- `config` +- `db` +- `public` +- `script` +- `config.ru` +- `Rakefile` + +It does not copy `lib`, `spec`, `e2e`, or `tasks.py`. + +Practical rule: + +- production runtime code belongs in copied paths such as `app/` +- local-only helpers should prefer `lib/`, `spec/`, `e2e/`, or task-layer code when feasible +- if production must ignore a local-only feature, cover that with blackbox E2E against the production image + ## HTTP E2E The backend HTTP E2E flow is intended to be reusable outside this repo, especially by frontend tests that need realistic backend state. diff --git a/app/controllers/tournaments_controller.rb b/app/controllers/tournaments_controller.rb index 26564b9..30e1a06 100644 --- a/app/controllers/tournaments_controller.rb +++ b/app/controllers/tournaments_controller.rb @@ -1,6 +1,16 @@ # frozen_string_literal: true class TournamentsController < ApplicationController + class NoOpRequestProfiling + def measure(_name) + yield + end + + def apply_to(_response, label:) + label + end + end + before_action :set_tournament_for_show, only: %i[show] before_action :set_tournament, only: %i[update destroy set_timer_end timer_end] before_action :authenticate_user!, only: %i[create update destroy set_timer_end] @@ -143,7 +153,7 @@ class TournamentsController < ApplicationController end def set_tournament_for_show - profiling = RequestProfiling.new(enabled: profiling_requested?) + profiling = build_request_profiling @tournament = profiling.measure('load_tournament') do Tournament.includes( :user, @@ -172,6 +182,13 @@ class TournamentsController < ApplicationController !Rails.env.production? && show_params.fetch(:profile, 'false') == 'true' end + def build_request_profiling + return NoOpRequestProfiling.new unless profiling_requested? + + require Rails.root.join('lib/local/request_profiling') + Local::RequestProfiling.new(enabled: true) + end + def tournament_params params.slice(:name, :description, :public, :teams, :group_stage, :playoff_teams_amount).permit! end diff --git a/app/services/request_profiling.rb b/app/services/request_profiling.rb deleted file mode 100644 index 6367d57..0000000 --- a/app/services/request_profiling.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -class RequestProfiling - IGNORED_SQL_NAMES = %w[SCHEMA CACHE].freeze - IGNORED_SQL = /\A(?:BEGIN|COMMIT|ROLLBACK|SAVEPOINT|RELEASE)/i - - def initialize(enabled: false) - @enabled = enabled - @measurements = [] - end - - def measure(name) - return yield unless @enabled - - query_count = 0 - subscriber = lambda do |_event_name, _start, _finish, _id, payload| - sql = payload[:sql].to_s - next if IGNORED_SQL_NAMES.include?(payload[:name]) - next if sql.match?(IGNORED_SQL) - - query_count += 1 - end - - started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) - result = nil - ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do - result = yield - end - duration = (Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at) * 1000.0 - @measurements << { name: name, duration: duration.round(1), query_count: query_count } - result - end - - def apply_to(response, label:) - return unless @enabled - - response.set_header('Server-Timing', @measurements.map { |m| "#{m[:name]};dur=#{m[:duration]}" }.join(', ')) - response.set_header( - 'X-Turniere-Profile', - "#{label}: #{@measurements.map { |m| "#{m[:name]}=#{m[:duration]}ms/#{m[:query_count]}sql" }.join(', ')}" - ) - end -end diff --git a/docker-compose.blackbox.yml b/docker-compose.blackbox.yml index acaa2f4..fae52d2 100644 --- a/docker-compose.blackbox.yml +++ b/docker-compose.blackbox.yml @@ -38,9 +38,13 @@ services: working_dir: /app environment: TURNIERE_E2E_BASE_URL: ${TURNIERE_E2E_BASE_URL:-http://app:3000} + TURNIERE_E2E_EXPECT_PROFILING: ${TURNIERE_E2E_EXPECT_PROFILING:-true} TURNIERE_E2E_EMAIL: ${TURNIERE_E2E_EMAIL:-e2e@example.com} TURNIERE_E2E_PASSWORD: ${TURNIERE_E2E_PASSWORD:-password123} TURNIERE_E2E_USERNAME: ${TURNIERE_E2E_USERNAME:-e2e-user} + TURNIERE_E2E_ALT_EMAIL: ${TURNIERE_E2E_ALT_EMAIL:-e2e-alt@example.com} + TURNIERE_E2E_ALT_PASSWORD: ${TURNIERE_E2E_ALT_PASSWORD:-password123} + TURNIERE_E2E_ALT_USERNAME: ${TURNIERE_E2E_ALT_USERNAME:-e2e-alt-user} volumes: turniere-blackbox-postgres: diff --git a/lib/local/request_profiling.rb b/lib/local/request_profiling.rb new file mode 100644 index 0000000..d231eea --- /dev/null +++ b/lib/local/request_profiling.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Local + class RequestProfiling + IGNORED_SQL_NAMES = %w[SCHEMA CACHE].freeze + IGNORED_SQL = /\A(?:BEGIN|COMMIT|ROLLBACK|SAVEPOINT|RELEASE)/i + + def initialize(enabled: false) + @enabled = enabled + @measurements = [] + end + + def measure(name) + return yield unless @enabled + + query_count = 0 + subscriber = lambda do |_event_name, _start, _finish, _id, payload| + sql = payload[:sql].to_s + next if IGNORED_SQL_NAMES.include?(payload[:name]) + next if sql.match?(IGNORED_SQL) + + query_count += 1 + end + + started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + result = nil + ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do + result = yield + end + duration = (Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at) * 1000.0 + @measurements << { name: name, duration: duration.round(1), query_count: query_count } + result + end + + def apply_to(response, label:) + return unless @enabled + + response.set_header('Server-Timing', @measurements.map { |m| "#{m[:name]};dur=#{m[:duration]}" }.join(', ')) + response.set_header( + 'X-Turniere-Profile', + "#{label}: #{@measurements.map { |m| "#{m[:name]}=#{m[:duration]}ms/#{m[:query_count]}sql" }.join(', ')}" + ) + end + end +end diff --git a/spec/e2e/http/tournament_rendering_spec.rb b/spec/e2e/http/tournament_rendering_spec.rb index d64f84a..cf89137 100644 --- a/spec/e2e/http/tournament_rendering_spec.rb +++ b/spec/e2e/http/tournament_rendering_spec.rb @@ -12,6 +12,7 @@ RSpec.describe 'Tournament rendering HTTP E2E' do end let(:base_url) { ENV.fetch('TURNIERE_E2E_BASE_URL', 'http://127.0.0.1:3000') } + let(:expect_profiling) { ENV.fetch('TURNIERE_E2E_EXPECT_PROFILING', 'true') == 'true' } let(:runner) do TurniereE2E::ScenarioRunner.new( base_url: base_url, @@ -41,13 +42,19 @@ RSpec.describe 'Tournament rendering HTTP E2E' do expect(group_stage.fetch(:groups).size).to eq(scenario[:group_count]) expect(tournament.fetch(:teams).size).to eq(scenario[:group_count] * scenario[:teams_per_group]) - expect(profiled_show.fetch(:server_timing)).to include('load_tournament') - expect(profiled_show.fetch(:server_timing)).to include('serialize_tournament') - expect(profiled_show.fetch(:profile_summary)).to include('sql') - expect(sections.fetch('load_tournament')).to be <= scenario[:max_server_duration_ms] - expect(sections.fetch('serialize_tournament')).to be <= scenario[:max_server_duration_ms] - expect(total_server_duration_ms).to be <= scenario[:max_server_duration_ms] - expect(profiled_show.fetch(:request_duration_ms)).to be <= scenario[:max_request_duration_ms] + if expect_profiling + expect(profiled_show.fetch(:server_timing)).to include('load_tournament') + expect(profiled_show.fetch(:server_timing)).to include('serialize_tournament') + expect(profiled_show.fetch(:profile_summary)).to include('sql') + expect(sections.fetch('load_tournament')).to be <= scenario[:max_server_duration_ms] + expect(sections.fetch('serialize_tournament')).to be <= scenario[:max_server_duration_ms] + expect(total_server_duration_ms).to be <= scenario[:max_server_duration_ms] + expect(profiled_show.fetch(:request_duration_ms)).to be <= scenario[:max_request_duration_ms] + else + expect(profiled_show.fetch(:server_timing)).to eq('') + expect(profiled_show.fetch(:profile_summary)).to eq('') + expect(sections).to eq({}) + end end end end diff --git a/spec/e2e_known_behaviors.md b/spec/e2e_known_behaviors.md index 085749a..20243b9 100644 --- a/spec/e2e_known_behaviors.md +++ b/spec/e2e_known_behaviors.md @@ -40,5 +40,5 @@ Observed and covered by `spec/e2e/http/tournament_rendering_spec.rb` and `script - large group-stage tournaments are created through the same authenticated HTTP `POST /tournaments` flow as normal clients - tournament show profiling is requested via `GET /tournaments/:id?profile=true` - profiling is intentionally non-production-only; local/test flows may use it, production should ignore it -- the backend returns `Server-Timing` and `X-Turniere-Profile` headers with `load_tournament` and `serialize_tournament` +- local/test runs return `Server-Timing` and `X-Turniere-Profile` headers with `load_tournament` and `serialize_tournament` - current locked shapes are `8x4`, `64x6`, and `4x32` diff --git a/tasks.py b/tasks.py index 6e78dd9..4248f5d 100644 --- a/tasks.py +++ b/tasks.py @@ -136,6 +136,7 @@ def _compose_env( app_image=PRODUCTION_TAG, runner_image=TEST_TAG, postgres_image=BLACKBOX_POSTGRES_IMAGE, + expect_profiling="true", ): return _env( TURNIERE_BLACKBOX_HOST_PORT=host_port, @@ -149,6 +150,7 @@ def _compose_env( TURNIERE_BLACKBOX_MAILGUN_API_KEY=BLACKBOX_MAILGUN_API_KEY, TURNIERE_BLACKBOX_MAILGUN_DOMAIN=BLACKBOX_MAILGUN_DOMAIN, TURNIERE_E2E_BASE_URL=BLACKBOX_INTERNAL_BASE_URL, + TURNIERE_E2E_EXPECT_PROFILING=expect_profiling, TURNIERE_E2E_EMAIL=E2E_EMAIL, TURNIERE_E2E_PASSWORD=E2E_PASSWORD, TURNIERE_E2E_USERNAME=E2E_USERNAME, @@ -273,6 +275,7 @@ def _run_blackbox_rspec(base_url, email, password, username, alt_email=E2E_ALT_E alt_username=E2E_ALT_USERNAME): env = _env( TURNIERE_E2E_BASE_URL=base_url, + TURNIERE_E2E_EXPECT_PROFILING="false", TURNIERE_E2E_EMAIL=email, TURNIERE_E2E_PASSWORD=password, TURNIERE_E2E_USERNAME=username, @@ -701,6 +704,7 @@ def docker_blackbox_up( app_image=app_image, runner_image=runner_image, postgres_image=postgres_image, + expect_profiling="false", ) _print_header("Starting production blackbox stack") @@ -767,6 +771,7 @@ def docker_blackbox_test( app_image=app_image, runner_image=runner_image, postgres_image=postgres_image, + expect_profiling="false", ) _print_header("Running HTTP E2E against production image") @@ -851,6 +856,7 @@ def blackbox_production( app_image=app_image, runner_image=runner_image, postgres_image=postgres_image, + expect_profiling="false", ) exit_error = None