From 9179c6fc3d589d2a042d3716b3577bb0b1644989 Mon Sep 17 00:00:00 2001 From: Krasimir Angelov <kangelov@gitlab.com> Date: Wed, 1 Dec 2021 14:47:01 +1300 Subject: [PATCH] Add database.flavor to usage data https://gitlab.com/gitlab-org/database-team/team-tasks/-/issues/218 Changelog: added --- .../settings/20211201012652_flavor.yml | 24 +++++++++++++++++++ doc/development/service_ping/index.md | 7 +++++- .../gitlab/usage_data_non_sql_metrics_spec.rb | 1 + ee/spec/lib/ee/gitlab/usage_data_spec.rb | 1 + lib/gitlab/usage_data.rb | 3 ++- .../application_settings_controller_spec.rb | 1 + .../admin/instance_review_controller_spec.rb | 1 + spec/features/admin/admin_settings_spec.rb | 1 + spec/lib/gitlab/usage_data_spec.rb | 2 ++ .../api/usage_data_non_sql_metrics_spec.rb | 1 + spec/requests/api/usage_data_queries_spec.rb | 1 + .../submit_service_ping_service_spec.rb | 2 ++ spec/support/helpers/usage_data_helpers.rb | 4 ++++ ...th_all_expected_metrics_shared_examples.rb | 2 ++ ...hout_restricted_metrics_shared_examples.rb | 2 ++ spec/tasks/gitlab/usage_data_rake_spec.rb | 1 + 16 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 config/metrics/settings/20211201012652_flavor.yml diff --git a/config/metrics/settings/20211201012652_flavor.yml b/config/metrics/settings/20211201012652_flavor.yml new file mode 100644 index 000000000000..3a409424eef0 --- /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 1f751eea4d83..315ff2b090c1 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/ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb b/ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb index c74cc34d5633..1a718a338dbd 100644 --- a/ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb +++ b/ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb @@ -7,6 +7,7 @@ before do stub_usage_data_connections + stub_database_flavor_check end describe '.uncached_data' do diff --git a/ee/spec/lib/ee/gitlab/usage_data_spec.rb b/ee/spec/lib/ee/gitlab/usage_data_spec.rb index a7a71ce9b3f3..d01f660150a9 100644 --- a/ee/spec/lib/ee/gitlab/usage_data_spec.rb +++ b/ee/spec/lib/ee/gitlab/usage_data_spec.rb @@ -7,6 +7,7 @@ before do stub_usage_data_connections + stub_database_flavor_check clear_memoized_values(described_class::EE_MEMOIZED_VALUES + described_class::CE_MEMOIZED_VALUES) end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 917c273d3f63..0c091dd75662 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -304,7 +304,8 @@ def components_usage_data # 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 478bd1b7f0a6..fb4c09706534 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -62,6 +62,7 @@ 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 898cd30cdca2..2169be4e70c5 100644 --- a/spec/controllers/admin/instance_review_controller_spec.rb +++ b/spec/controllers/admin/instance_review_controller_spec.rb @@ -22,6 +22,7 @@ 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 29323c604ef1..2408aa1d3b4f 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -530,6 +530,7 @@ 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/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 015ecd1671ef..fa01e7e6c960 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -9,6 +9,7 @@ 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 @@ def omniauth_providers 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 225af57a2675..0b73d0f96a44 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 @@ 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 0ba4a37bc9b3..69a8d865a598 100644 --- a/spec/requests/api/usage_data_queries_spec.rb +++ b/spec/requests/api/usage_data_queries_spec.rb @@ -10,6 +10,7 @@ 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 ca387690e83d..2971c9a9309a 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 @@ 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 @@ 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 5865bafd3821..776ea37ffdc0 100644 --- a/spec/support/helpers/usage_data_helpers.rb +++ b/spec/support/helpers/usage_data_helpers.rb @@ -183,6 +183,10 @@ def stub_prometheus_queries ) 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 535e7291b7e3..856810a4de1f 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 9f18174cbc74..e05239a9a36e 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 acaf9b5729bd..442b884b313c 100644 --- a/spec/tasks/gitlab/usage_data_rake_spec.rb +++ b/spec/tasks/gitlab/usage_data_rake_spec.rb @@ -9,6 +9,7 @@ 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 -- GitLab