diff --git a/config/metrics/settings/20211201012652_flavor.yml b/config/metrics/settings/20211201012652_flavor.yml new file mode 100644 index 00000000000..3a409424eef --- /dev/null +++ b/config/metrics/settings/20211201012652_flavor.yml @@ -0,0 +1,24 @@ +--- +key_path: database.flavor +description: What PostgreSQL flavor is being used. Possible values are + "Amazon Aurora PostgreSQL", "PostgreSQL on Amazon RDS", "Cloud SQL for PostgreSQL", + "Azure Database for PostgreSQL - Single Server", "Azure Database for PostgreSQL - Flexible Server", + or "null". +product_section: enablement +product_stage: enablement +product_group: group::database +product_category: database +value_type: string +status: active +milestone: "14.6" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75587 +time_frame: none +data_source: system +data_category: optional +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/doc/development/service_ping/index.md b/doc/development/service_ping/index.md index 1f751eea4d8..315ff2b090c 100644 --- a/doc/development/service_ping/index.md +++ b/doc/development/service_ping/index.md @@ -311,7 +311,8 @@ The following is example content of the Service Ping payload. "database": { "adapter": "postgresql", "version": "9.6.15", - "pg_system_id": 6842684531675334351 + "pg_system_id": 6842684531675334351, + "flavor": "Cloud SQL for PostgreSQL" }, "analytics_unique_visits": { "g_analytics_contribution": 999, @@ -435,6 +436,10 @@ The following is example content of the Service Ping payload. ## Notable changes +In GitLab 14.6, [`flavor`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75587) was added to try to detect the underlying managed database variant. +Possible values are "Amazon Aurora PostgreSQL", "PostgreSQL on Amazon RDS", "Cloud SQL for PostgreSQL", +"Azure Database for PostgreSQL - Flexible Server", or "null". + In GitLab 13.5, `pg_system_id` was added to send the [PostgreSQL system identifier](https://www.2ndquadrant.com/en/blog/support-for-postgresqls-system-identifier-in-barman/). ## Export Service Ping SQL queries and definitions diff --git a/lib/gitlab/database/reflection.rb b/lib/gitlab/database/reflection.rb index 48a4de28541..3ea7277571f 100644 --- a/lib/gitlab/database/reflection.rb +++ b/lib/gitlab/database/reflection.rb @@ -105,6 +105,35 @@ module Gitlab row['system_identifier'] end + def flavor + { + # Based on https://aws.amazon.com/premiumsupport/knowledge-center/aurora-version-number/ + 'Amazon Aurora PostgreSQL' => { statement: 'SELECT AURORA_VERSION()', error: /PG::UndefinedFunction/ }, + # Based on https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/CHAP_PostgreSQL.html#PostgreSQL.Concepts.General.FeatureSupport.Extensions, + # this is also available for both Aurora and RDS, so we need to check for the former first. + 'PostgreSQL on Amazon RDS' => { statement: 'SHOW rds.extensions', error: /PG::UndefinedObject/ }, + # Based on https://cloud.google.com/sql/docs/postgres/flags#postgres-c this should be specific + # to Cloud SQL for PostgreSQL + 'Cloud SQL for PostgreSQL' => { statement: 'SHOW cloudsql.iam_authentication', error: /PG::UndefinedObject/ }, + # Based on + # - https://docs.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-extensions + # - https://docs.microsoft.com/en-us/azure/postgresql/concepts-extensions + # this should be available only for Azure Database for PostgreSQL - Flexible Server. + 'Azure Database for PostgreSQL - Flexible Server' => { statement: 'SHOW azure.extensions', error: /PG::UndefinedObject/ }, + # Based on + # - https://docs.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-servers + # - https://docs.microsoft.com/en-us/azure/postgresql/concepts-servers#managing-your-server + # this database is present on both Flexible and Single server, so we should check the former first. + 'Azure Database for PostgreSQL - Single Server' => { statement: "SELECT datname FROM pg_database WHERE datname = 'azure_maintenance'" } + }.each do |flavor, conditions| + return flavor if connection.execute(conditions[:statement]).to_a.present? + rescue ActiveRecord::StatementInvalid => e + raise if conditions[:error] && !e.message.match?(conditions[:error]) + end + + nil + end + private def connection diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 917c273d3f6..0c091dd7566 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -304,7 +304,8 @@ module Gitlab # rubocop: disable UsageData/LargeTable adapter: alt_usage_data { ApplicationRecord.database.adapter_name }, version: alt_usage_data { ApplicationRecord.database.version }, - pg_system_id: alt_usage_data { ApplicationRecord.database.system_id } + pg_system_id: alt_usage_data { ApplicationRecord.database.system_id }, + flavor: alt_usage_data { ApplicationRecord.database.flavor } # rubocop: enable UsageData/LargeTable }, mail: { diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 478bd1b7f0a..fb4c0970653 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -62,6 +62,7 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set describe 'GET #usage_data' do before do stub_usage_data_connections + stub_database_flavor_check sign_in(admin) end diff --git a/spec/controllers/admin/instance_review_controller_spec.rb b/spec/controllers/admin/instance_review_controller_spec.rb index 898cd30cdca..2169be4e70c 100644 --- a/spec/controllers/admin/instance_review_controller_spec.rb +++ b/spec/controllers/admin/instance_review_controller_spec.rb @@ -22,6 +22,7 @@ RSpec.describe Admin::InstanceReviewController do before do stub_application_setting(usage_ping_enabled: true) stub_usage_data_connections + stub_database_flavor_check ::Gitlab::UsageData.data(force_refresh: true) subject end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 29323c604ef..2408aa1d3b4 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -530,6 +530,7 @@ RSpec.describe 'Admin updates settings' do it 'loads usage ping payload on click', :js do stub_usage_data_connections + stub_database_flavor_check page.within('#js-usage-settings') do expected_payload_content = /(?=.*"uuid")(?=.*"hostname")/m diff --git a/spec/lib/gitlab/database/reflection_spec.rb b/spec/lib/gitlab/database/reflection_spec.rb index 7c3d797817d..efc5bd1c1e1 100644 --- a/spec/lib/gitlab/database/reflection_spec.rb +++ b/spec/lib/gitlab/database/reflection_spec.rb @@ -259,6 +259,66 @@ RSpec.describe Gitlab::Database::Reflection do end end + describe '#flavor', :delete do + let(:result) { [double] } + let(:connection) { database.model.connection } + + def stub_statements(statements) + statements = Array.wrap(statements) + execute = connection.method(:execute) + + allow(connection).to receive(:execute) do |arg| + if statements.include?(arg) + result + else + execute.call(arg) + end + end + end + + it 're-raises exceptions not matching expected messages' do + expect(database.model.connection) + .to receive(:execute) + .and_raise(ActiveRecord::StatementInvalid, 'Something else') + + expect { database.flavor }.to raise_error ActiveRecord::StatementInvalid, /Something else/ + end + + it 'recognizes Amazon Aurora PostgreSQL' do + stub_statements(['SHOW rds.extensions', 'SELECT AURORA_VERSION()']) + + expect(database.flavor).to eq('Amazon Aurora PostgreSQL') + end + + it 'recognizes PostgreSQL on Amazon RDS' do + stub_statements('SHOW rds.extensions') + + expect(database.flavor).to eq('PostgreSQL on Amazon RDS') + end + + it 'recognizes CloudSQL for PostgreSQL' do + stub_statements('SHOW cloudsql.iam_authentication') + + expect(database.flavor).to eq('Cloud SQL for PostgreSQL') + end + + it 'recognizes Azure Database for PostgreSQL - Flexible Server' do + stub_statements(["SELECT datname FROM pg_database WHERE datname = 'azure_maintenance'", 'SHOW azure.extensions']) + + expect(database.flavor).to eq('Azure Database for PostgreSQL - Flexible Server') + end + + it 'recognizes Azure Database for PostgreSQL - Single Server' do + stub_statements("SELECT datname FROM pg_database WHERE datname = 'azure_maintenance'") + + expect(database.flavor).to eq('Azure Database for PostgreSQL - Single Server') + end + + it 'returns nil if can not recognize the flavor' do + expect(database.flavor).to be_nil + end + end + describe '#config' do it 'returns a HashWithIndifferentAccess' do expect(database.config) diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 015ecd1671e..fa01e7e6c96 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do stub_usage_data_connections stub_object_store_settings clear_memoized_values(described_class::CE_MEMOIZED_VALUES) + stub_database_flavor_check('Cloud SQL for PostgreSQL') end describe '.uncached_data' do @@ -920,6 +921,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do expect(subject[:database][:adapter]).to eq(ApplicationRecord.database.adapter_name) expect(subject[:database][:version]).to eq(ApplicationRecord.database.version) expect(subject[:database][:pg_system_id]).to eq(ApplicationRecord.database.system_id) + expect(subject[:database][:flavor]).to eq('Cloud SQL for PostgreSQL') expect(subject[:mail][:smtp_server]).to eq(ActionMailer::Base.smtp_settings[:address]) expect(subject[:gitaly][:version]).to be_present expect(subject[:gitaly][:servers]).to be >= 1 diff --git a/spec/requests/api/usage_data_non_sql_metrics_spec.rb b/spec/requests/api/usage_data_non_sql_metrics_spec.rb index 225af57a267..0b73d0f96a4 100644 --- a/spec/requests/api/usage_data_non_sql_metrics_spec.rb +++ b/spec/requests/api/usage_data_non_sql_metrics_spec.rb @@ -18,6 +18,7 @@ RSpec.describe API::UsageDataNonSqlMetrics do context 'with authentication' do before do stub_feature_flags(usage_data_non_sql_metrics: true) + stub_database_flavor_check end it 'returns non sql metrics if user is admin' do diff --git a/spec/requests/api/usage_data_queries_spec.rb b/spec/requests/api/usage_data_queries_spec.rb index 0ba4a37bc9b..69a8d865a59 100644 --- a/spec/requests/api/usage_data_queries_spec.rb +++ b/spec/requests/api/usage_data_queries_spec.rb @@ -10,6 +10,7 @@ RSpec.describe API::UsageDataQueries do before do stub_usage_data_connections + stub_database_flavor_check end describe 'GET /usage_data/usage_data_queries' do diff --git a/spec/services/service_ping/submit_service_ping_service_spec.rb b/spec/services/service_ping/submit_service_ping_service_spec.rb index ca387690e83..2971c9a9309 100644 --- a/spec/services/service_ping/submit_service_ping_service_spec.rb +++ b/spec/services/service_ping/submit_service_ping_service_spec.rb @@ -110,6 +110,7 @@ RSpec.describe ServicePing::SubmitService do context 'when product_intelligence_enabled is true' do before do stub_usage_data_connections + stub_database_flavor_check allow(ServicePing::ServicePingSettings).to receive(:product_intelligence_enabled?).and_return(true) end @@ -126,6 +127,7 @@ RSpec.describe ServicePing::SubmitService do context 'when usage ping is enabled' do before do stub_usage_data_connections + stub_database_flavor_check stub_application_setting(usage_ping_enabled: true) end diff --git a/spec/support/helpers/usage_data_helpers.rb b/spec/support/helpers/usage_data_helpers.rb index 5865bafd382..776ea37ffdc 100644 --- a/spec/support/helpers/usage_data_helpers.rb +++ b/spec/support/helpers/usage_data_helpers.rb @@ -183,6 +183,10 @@ module UsageDataHelpers ) end + def stub_database_flavor_check(flavor = nil) + allow(ApplicationRecord.database).to receive(:flavor).and_return(flavor) + end + def clear_memoized_values(values) values.each { |v| described_class.clear_memoization(v) } end diff --git a/spec/support/shared_examples/services/service_ping/service_ping_payload_with_all_expected_metrics_shared_examples.rb b/spec/support/shared_examples/services/service_ping/service_ping_payload_with_all_expected_metrics_shared_examples.rb index 535e7291b7e..856810a4de1 100644 --- a/spec/support/shared_examples/services/service_ping/service_ping_payload_with_all_expected_metrics_shared_examples.rb +++ b/spec/support/shared_examples/services/service_ping/service_ping_payload_with_all_expected_metrics_shared_examples.rb @@ -2,6 +2,8 @@ RSpec.shared_examples 'service ping payload with all expected metrics' do specify do + allow(ApplicationRecord.database).to receive(:flavor).and_return(nil) + aggregate_failures do expected_metrics.each do |metric| is_expected.to have_usage_metric metric['key_path'] diff --git a/spec/support/shared_examples/services/service_ping/service_ping_payload_without_restricted_metrics_shared_examples.rb b/spec/support/shared_examples/services/service_ping/service_ping_payload_without_restricted_metrics_shared_examples.rb index 9f18174cbc7..e05239a9a36 100644 --- a/spec/support/shared_examples/services/service_ping/service_ping_payload_without_restricted_metrics_shared_examples.rb +++ b/spec/support/shared_examples/services/service_ping/service_ping_payload_without_restricted_metrics_shared_examples.rb @@ -2,6 +2,8 @@ RSpec.shared_examples 'service ping payload without restricted metrics' do specify do + allow(ApplicationRecord.database).to receive(:flavor).and_return(nil) + aggregate_failures do restricted_metrics.each do |metric| is_expected.not_to have_usage_metric metric['key_path'] diff --git a/spec/tasks/gitlab/usage_data_rake_spec.rb b/spec/tasks/gitlab/usage_data_rake_spec.rb index acaf9b5729b..442b884b313 100644 --- a/spec/tasks/gitlab/usage_data_rake_spec.rb +++ b/spec/tasks/gitlab/usage_data_rake_spec.rb @@ -9,6 +9,7 @@ RSpec.describe 'gitlab:usage data take tasks', :silence_stdout do Rake.application.rake_require 'tasks/gitlab/usage_data' # stub prometheus external http calls https://gitlab.com/gitlab-org/gitlab/-/issues/245277 stub_prometheus_queries + stub_database_flavor_check end describe 'dump_sql_in_yaml' do