diff --git a/notifier/environment.rb b/notifier/environment.rb new file mode 100644 index 0000000000000000000000000000000000000000..9280dd7934555f27e4c7b121ecd5114dc9afab23 --- /dev/null +++ b/notifier/environment.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +require 'singleton' + +class Environment + include Singleton + + def ci_project_url + ENV['CI_PROJECT_URL'] + end + + def ci_job_id + ENV['CI_JOB_ID'] + end + + def ci_pipeline_id + ENV['CI_PIPELINE_ID'] + end +end diff --git a/notifier/feedback.rb b/notifier/feedback.rb index 99e95b30e390c1aaad00548bd2291c142dfc332a..786a1dd7040e1ed5fc3945e4852bdd3f8c05da60 100644 --- a/notifier/feedback.rb +++ b/notifier/feedback.rb @@ -3,16 +3,20 @@ require 'filesize' require 'erb' require 'active_support/core_ext/module/delegation' require_relative 'niceql' +require 'pg_query' +require_relative 'json_payload' +require_relative 'environment' class Feedback UNKNOWN = ':grey_question:' - attr_reader :result + attr_reader :result, :env delegate :migrations_from_branch, :other_migrations, to: :result - def initialize(result) + def initialize(result, env = Environment.instance) @result = result + @env = env end def render @@ -45,6 +49,14 @@ class Feedback erb('summary_table').result(b) end + def render_json_payload + payload = JsonPayload.new.encode(result) + + b = binding + b.local_variable_set(:payload, payload) + erb('json_payload').result(b) + end + def erb(template) ERB.new(File.read("templates/#{template}.erb"), trim_mode: '<>%') end diff --git a/notifier/json_payload.rb b/notifier/json_payload.rb new file mode 100644 index 0000000000000000000000000000000000000000..aba8d08dbd768b4a25da0b850135762f30ca6815 --- /dev/null +++ b/notifier/json_payload.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true +require 'json' +require "base64" + +class JsonPayload + VERSION = 1 + + ATTRIBUTES = %w[migration walltime total_database_size_change success].freeze + + def encode(result) + data = result.migrations_from_branch.map do |migration| + # We only expose a subset of the statistics available here + migration.statistics.to_h.slice(*ATTRIBUTES) + end + + json = { + # The version should change when the JSON schema significantly changes. + # This will allow readers to know how to read data later + version: VERSION, + data: data + }.to_json + + Base64.encode64(json) + end +end diff --git a/notifier/notifier.rb b/notifier/notifier.rb index 5530094aa72a0e1cb28f104bd0e18017fd9523da..51f74980c4c0064ec5863ff09de00d8b828615e7 100755 --- a/notifier/notifier.rb +++ b/notifier/notifier.rb @@ -56,7 +56,7 @@ class Notifier < Thor end desc "print STATS MIGRATIONS", "only print feedback" - def print(statistics_file, migrations_file) + def print(statistics_file, migrations_file, clone_details_file) puts feedback_for(statistics_file, migrations_file, clone_details_file).render end diff --git a/notifier/spec/feedback_spec.rb b/notifier/spec/feedback_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ce24c9de83ed03a00bf66ad4bbc752e2c08a9b1a --- /dev/null +++ b/notifier/spec/feedback_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Feedback do + # This is an end-to-end test based on the checked in fixtures + # This is a temporary measure to increase our confidence in a change - we can remove + # it if it gets tedious and we have smaller unit tests in place + describe 'end to end test for rendering feedback comment' do + let(:clone_details) { file_fixture('migration-testing/clone-details.json') } + let(:migration_stats) { file_fixture('migration-testing/migration-stats.json') } + let(:migrations) { file_fixture('migration-testing/migrations.json') } + + let(:result) { Result.from_files(migration_stats, migrations, clone_details) } + + let(:expected_comment_file) { file_fixture('migration-testing/expected-comment.txt') } + + subject { described_class.new(result).render } + + before do + override_env_from_fixture('migration-testing/environment.json') + end + + # The expectation for this spec lives in `expected_comment_file` + # It can be re-recorded with: `RECAPTURE_END_TO_END_RESULTS=1 bundle exec rspec spec` + it 'renders the comment for fixtures' do + if ENV['RECAPTURE_END_TO_END_RESULTS'] + File.open(expected_comment_file, 'wb+') do |io| + io << subject + end + end + + expect(subject).to eq(File.read(expected_comment_file)) + end + end +end diff --git a/notifier/spec/fixtures/migration-testing/environment.json b/notifier/spec/fixtures/migration-testing/environment.json new file mode 100644 index 0000000000000000000000000000000000000000..8813351d6ec67121672173edd141f9dff138cbec --- /dev/null +++ b/notifier/spec/fixtures/migration-testing/environment.json @@ -0,0 +1,5 @@ +{ + "CI_PROJECT_URL": "https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing", + "CI_JOB_ID": "1354666720", + "CI_PIPELINE_ID": "4711" +} diff --git a/notifier/spec/fixtures/migration-testing/expected-comment.txt b/notifier/spec/fixtures/migration-testing/expected-comment.txt new file mode 100644 index 0000000000000000000000000000000000000000..f51f7f4d8d8dc4497c9008bb676782fb4ee61b6a --- /dev/null +++ b/notifier/spec/fixtures/migration-testing/expected-comment.txt @@ -0,0 +1,59 @@ + +### Database migrations + +| | 1 Warnings | +| --------- | -------------------- | +| :warning: | 20210602144718 - CreateTestTable had a query that [exceeded timing guidelines](https://docs.gitlab.com/ee/development/query_performance.html#timing-guidelines-for-queries). Run time should not exceed 100ms, but it was 269.888734 
CREATE TABLE "test_tables" ("id" bigserial primary key, "stars" bigint DEFAULT 0 NOT NULL, "created_at" timestamp(6) NOT NULL, "updated_at" timestamp(6) NOT NULL, "title" text, "notes" text) /*application:test*/
| + +Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the [migration testing pipeline](https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing/-/pipelines/4711) (limited access). + +| Migration | Type | Total runtime | Result | DB size change | +| --------- | ---- | ------------- | ------ | -------------- | +| 20210602144718 - CreateTestTable | Regular | 2.0 s | :warning: | +32.00 KiB | +| 20210604232017 - DropTestTable | Post deploy | 1.2 s | :white_check_mark: | -24.00 KiB | + +#### :warning: Migration: 20210602144718 - CreateTestTable + +* Type: Regular +* Duration: 2.0 s +* Database size change: +32.00 KiB + +| Query | Calls | Total Time | Max Time | Mean Time | Rows | +| ----- | ----- | ---------- | -------- | --------- | ---- | +| 
CREATE TABLE "test_tables" ("id" bigserial primary key, "stars" bigint DEFAULT 0 NOT NULL, "created_at" timestamp(6) NOT NULL, "updated_at" timestamp(6) NOT NULL, "title" text, "notes" text) /*application:test*/
| 1 | 269.9 ms | 269.9 ms | 269.9 ms | 0 | +| 
ALTER TABLE "test_tables" ADD CONSTRAINT "check_0770ba173a" CHECK (char_length("title") <= 128), ADD CONSTRAINT "check_9cfc473dbc" CHECK (char_length("notes") <= 1024)
/*application:test*/
| 1 | 24.7 ms | 24.7 ms | 24.7 ms | 0 | +| 
SELECT $1::regtype::oid
| 1 | 0.0 ms | 0.0 ms | 0.0 ms | 1 | +#### Migration: 20210604232017 - DropTestTable + +* Type: Post deploy +* Duration: 1.2 s +* Database size change: -24.00 KiB + +| Query | Calls | Total Time | Max Time | Mean Time | Rows | +| ----- | ----- | ---------- | -------- | --------- | ---- | +| 
DROP TABLE "test_tables" /*application:test*/
| 1 | 5.7 ms | 5.7 ms | 5.7 ms | 0 | + +
+ Other migrations pending on GitLab.com + + | Migration | Type | Total runtime | Result | DB size change | +| --------- | ---- | ------------- | ------ | -------------- | + +
+ +#### Clone Details +| Clone ID | Clone Created At | Clone Data Timestamp | Expected Removal Time | +| -------- | ---------------- | -------------------- | --------------------- | +| [`database-testing-639298`](https://console.postgres.ai/gitlab/gitlab-production-tunnel/instances/59/clones/database-testing-639298) | 2021-06-07 14:42:21 UTC | 2021-06-07 12:00:26 UTC | 2021-06-08 02:43:37 +0000 | + +#### [Artifacts](https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing/-/jobs/1354666720/artifacts/browse/migration-testing/) + +--- +Brought to you by [gitlab-org/database-team/gitlab-com-database-testing](https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing). [Epic](https://gitlab.com/groups/gitlab-org/database-team/-/epics/9) + \ No newline at end of file diff --git a/notifier/spec/json_payload_spec.rb b/notifier/spec/json_payload_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..754dcdba9b9f07ff61ebb83385428c2002694f52 --- /dev/null +++ b/notifier/spec/json_payload_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true +require 'spec_helper' +require "base64" + +RSpec.describe JsonPayload do + let(:clone_details) { file_fixture('migration-testing/clone-details.json') } + let(:migration_stats) { file_fixture('migration-testing/migration-stats.json') } + let(:migrations) { file_fixture('migration-testing/migrations.json') } + + let(:result) { Result.from_files(migration_stats, migrations, clone_details) } + + subject do + encoded = described_class.new.encode(result) + + JSON.parse(Base64.decode64(encoded)) + end + + it 'stores a version' do + expect(subject['version']).to eq(described_class::VERSION) + end + + it 'has data for migrations on the branch' do + result.migrations_from_branch.map(&:version).each do |version| + records = subject['data'].select { |record| record['migration'] == version } + + expect(records.size).to eq(1) + end + end + + it 'stores walltime for each migration' do + subject['data'].each do |record| + expect(record['walltime']).to be_positive + end + end + + it 'stores database size change for each migration' do + subject['data'].each do |record| + expect(record['total_database_size_change']).to be_a(Integer) + end + end + + it 'stores success for each migration' do + subject['data'].each do |record| + expect(record['success']).to be_boolean + end + end +end diff --git a/notifier/spec/result_spec.rb b/notifier/spec/result_spec.rb index 2be3683c2e47919f354910e16479437e35a6a4fc..96a49ff49f24df74c47b21c954a090dd69f1eff2 100644 --- a/notifier/spec/result_spec.rb +++ b/notifier/spec/result_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Result do describe 'sorting and filtering' do let(:migrations) do - # rubocop:disable Metrics/LineLength + # rubocop:disable Layout/LineLength { 4 => Migration.new({ 'version' => 4, 'type' => Migration::TYPE_POST_DEPLOY, 'intro_on_current_branch' => true }, nil), 3 => Migration.new({ 'version' => 3, 'type' => Migration::TYPE_REGULAR, 'intro_on_current_branch' => true }, nil), @@ -30,7 +30,7 @@ RSpec.describe Result do 5 => Migration.new({ 'version' => 5, 'type' => Migration::TYPE_POST_DEPLOY, 'intro_on_current_branch' => false }, nil), 6 => Migration.new({ 'version' => 6, 'type' => Migration::TYPE_REGULAR, 'intro_on_current_branch' => false }, nil) } - # rubocop:enable Metrics/LineLength + # rubocop:enable Layout/LineLength end describe '#migrations_from_branch' do diff --git a/notifier/spec/spec_helper.rb b/notifier/spec/spec_helper.rb index 768caae85f8e5751730aaab8603f0397e097b1ce..9a45f462855733d4080b0d2c2186816c590eab8c 100644 --- a/notifier/spec/spec_helper.rb +++ b/notifier/spec/spec_helper.rb @@ -2,6 +2,7 @@ require 'pry' require_relative '../notifier' +require 'json' RSpec.configure do |config| config.expect_with :rspec do |expectations| @@ -27,3 +28,15 @@ end def file_fixture(path) File.join(Dir.pwd, 'spec', 'fixtures', path) end + +def override_env_from_fixture(path) + JSON.parse(File.read(file_fixture(path))).each do |key, val| + allow(Environment.instance).to receive(key.downcase).and_return(val) + end +end + +RSpec::Matchers.define :be_boolean do + match do |actual| + expect(actual).to be(true).or(be(false)) + end +end diff --git a/notifier/templates/feedback.erb b/notifier/templates/feedback.erb index 45e201612cac91f68427061748048fb91268d4ab..1f2a26e78ddb19cde58072f83a5917a438700efa 100644 --- a/notifier/templates/feedback.erb +++ b/notifier/templates/feedback.erb @@ -3,7 +3,7 @@ <%= warnings.render %> -Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the [migration testing pipeline](<%= ENV['CI_PROJECT_URL'] %>/-/pipelines/<%= ENV['CI_PIPELINE_ID'] %>) (limited access). +Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the [migration testing pipeline](<%= env.ci_project_url %>/-/pipelines/<%= env.ci_pipeline_id %>) (limited access). <%= render_migrations_from_branch_summary %> @@ -20,7 +20,8 @@ Migrations included in this change have been executed on gitlab.com data for tes #### Clone Details <%= render_clone_details(result.clone_details) %> -#### [Artifacts](<%= ENV['CI_PROJECT_URL'] %>/-/jobs/<%= ENV['CI_JOB_ID'] %>/artifacts/browse/migration-testing/) +#### [Artifacts](<%= env.ci_project_url %>/-/jobs/<%= env.ci_job_id %>/artifacts/browse/migration-testing/) --- -Brought to you by [gitlab-org/database-team/gitlab-com-database-testing](https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing). Contributions welcome! [Roadmap](https://gitlab.com/groups/gitlab-org/database-team/-/epics/9) +Brought to you by [gitlab-org/database-team/gitlab-com-database-testing](https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing). [Epic](https://gitlab.com/groups/gitlab-org/database-team/-/epics/9) +<%= render_json_payload %> \ No newline at end of file diff --git a/notifier/templates/json_payload.erb b/notifier/templates/json_payload.erb new file mode 100644 index 0000000000000000000000000000000000000000..207e4c5ddabc4757009ccc42fde8e5f43b05bb67 --- /dev/null +++ b/notifier/templates/json_payload.erb @@ -0,0 +1 @@ + \ No newline at end of file