diff options
26 files changed, 367 insertions, 151 deletions
diff --git a/.gitpod.yml b/.gitpod.yml index c9adad3e3fe..762adb72cce 100644 --- a/.gitpod.yml +++ b/.gitpod.yml @@ -19,6 +19,8 @@ tasks: # ensure gdk.yml has correct instance settings gdk config set gitlab.rails.port 443 gdk config set gitlab.rails.https.enabled true + # make documentation builds available + gdk config set gitlab_docs.enabled true # reconfigure GDK echo "$(date) – Reconfiguring GDK" | tee -a /workspace/startup.log gdk reconfigure @@ -83,6 +85,8 @@ ports: onOpen: ignore - port: 3000 # rails-web onOpen: notify + - port: 3005 # gitlab-docs + onOpen: notify - port: 3010 # gitlab-pages onOpen: ignore - port: 3808 # webpack @@ -102,3 +106,4 @@ vscode: - octref.vetur@0.34.1 - dbaeumer.vscode-eslint@2.1.8 - gitlab.gitlab-workflow@3.24.0 + - DavidAnson.vscode-markdownlint@0.44.4 diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/nothing_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/nothing_to_merge.vue index 7827c79cd31..2d704d3b07a 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/nothing_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/nothing_to_merge.vue @@ -1,6 +1,7 @@ <script> import { GlButton, GlSprintf, GlLink, GlSafeHtmlDirective } from '@gitlab/ui'; import emptyStateSVG from 'icons/_mr_widget_empty_state.svg'; +import api from '~/api'; import { helpPagePath } from '~/helpers/help_page_helper'; export default { @@ -22,6 +23,11 @@ export default { data() { return { emptyStateSVG }; }, + methods: { + onClickNewFile() { + api.trackRedisHllUserEvent('i_code_review_widget_nothing_merge_click_new_file'); + }, + }, ciHelpPage: helpPagePath('/ci/quick_start/index.html'), safeHtmlConfig: { ADD_TAGS: ['use'] }, }; @@ -59,6 +65,7 @@ export default { category="secondary" variant="success" data-testid="createFileButton" + @click="onClickNewFile" > {{ __('Create file') }} </gl-button> diff --git a/app/views/layouts/_loading_hints.html.haml b/app/views/layouts/_loading_hints.html.haml index e2189009045..fc130490d8b 100644 --- a/app/views/layouts/_loading_hints.html.haml +++ b/app/views/layouts/_loading_hints.html.haml @@ -11,5 +11,5 @@ = preload_link_tag(path_to_stylesheet('application_utilities'), crossorigin: css_crossorigin) = preload_link_tag(path_to_stylesheet('application'), crossorigin: css_crossorigin) = preload_link_tag(path_to_stylesheet("highlight/themes/#{user_color_scheme}"), crossorigin: css_crossorigin) - - if Gitlab::Tracking.enabled? && Gitlab::CurrentSettings.snowplow_collector_hostname - %link{ rel: 'preconnect', href: Gitlab::CurrentSettings.snowplow_collector_hostname, crossorigin: '' } + - if Gitlab::Tracking.enabled? && Gitlab::Tracking.collector_hostname + %link{ rel: 'preconnect', href: Gitlab::Tracking.collector_hostname, crossorigin: '' } diff --git a/config/initializers/load_balancing.rb b/config/initializers/load_balancing.rb index a31b11bb2be..df75178740b 100644 --- a/config/initializers/load_balancing.rb +++ b/config/initializers/load_balancing.rb @@ -20,7 +20,7 @@ Gitlab::Database::LoadBalancing.base_models.each do |model| Gitlab::Cluster::LifecycleEvents.on_before_fork do # When forking, we don't want to wait until the connections aren't in use # any more, as this could delay the boot cycle. - model.connection.load_balancer.disconnect!(timeout: 0) + model.load_balancer.disconnect!(timeout: 0) end # Service discovery only needs to run in the worker processes, as the main one diff --git a/config/metrics/aggregates/code_review.yml b/config/metrics/aggregates/code_review.yml index 54ebe5da192..36a56a95121 100644 --- a/config/metrics/aggregates/code_review.yml +++ b/config/metrics/aggregates/code_review.yml @@ -67,6 +67,7 @@ - 'i_code_review_user_resolve_conflict' - 'i_code_review_user_searches_diff' - 'i_code_review_user_resolve_thread_in_issue' + - 'i_code_review_widget_nothing_merge_click_new_file' - name: code_review_category_monthly_active_users operator: OR source: redis @@ -126,6 +127,7 @@ - 'i_code_review_user_resolve_conflict' - 'i_code_review_user_searches_diff' - 'i_code_review_user_resolve_thread_in_issue' + - 'i_code_review_widget_nothing_merge_click_new_file' - name: code_review_extension_category_monthly_active_users operator: OR source: redis diff --git a/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml b/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml new file mode 100644 index 00000000000..1aa0edf60e6 --- /dev/null +++ b/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml @@ -0,0 +1,25 @@ +--- +key_path: redis_hll_counters.code_review.i_code_review_widget_nothing_merge_click_new_file_monthly +description: Count of users who click the create file button in the nothing to merge widget state +product_section: dev +product_stage: create +product_group: group::code review +product_category: code_review +value_type: number +status: active +milestone: '14.5' +introduced_by_url: +time_frame: 28d +data_source: redis_hll +instrumentation_class: RedisHLLMetric +options: + events: + - i_code_review_widget_nothing_merge_click_new_file +data_category: optional +distribution: + - ce + - ee +tier: + - free + - premium + - ultimate diff --git a/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml b/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml new file mode 100644 index 00000000000..9f8ae151a80 --- /dev/null +++ b/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml @@ -0,0 +1,25 @@ +--- +key_path: redis_hll_counters.code_review.i_code_review_widget_nothing_merge_click_new_file_weekly +description: Count of users who click the create file button in the nothing to merge widget state +product_section: dev +product_stage: create +product_group: group::code review +product_category: code_review +value_type: number +status: active +milestone: '14.5' +introduced_by_url: +time_frame: 7d +data_source: redis_hll +instrumentation_class: RedisHLLMetric +options: + events: + - i_code_review_widget_nothing_merge_click_new_file +data_category: optional +distribution: + - ce + - ee +tier: + - free + - premium + - ultimate diff --git a/doc/development/snowplow/implementation.md b/doc/development/snowplow/implementation.md index a7755e285f8..fe1de789eae 100644 --- a/doc/development/snowplow/implementation.md +++ b/doc/development/snowplow/implementation.md @@ -424,7 +424,7 @@ records the same events as the full Snowplow pipeline. To query events, use the To install and run Snowplow Micro, complete these steps to modify the [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit): -1. Ensure Docker is installed and running. +1. Ensure [Docker is installed](https://docs.docker.com/get-docker/) and running. 1. To install Snowplow Micro, clone the settings in [this project](https://gitlab.com/gitlab-org/snowplow-micro-configuration). @@ -436,73 +436,18 @@ To install and run Snowplow Micro, complete these steps to modify the ./snowplow-micro.sh ``` -1. Use GDK to start the PostgreSQL terminal and connect - to the `gitlabhq_development` database: +1. Set the environment variable to tell the GDK to use Snowplow Micro in development. This overrides two `application_settings` options: + - `snowplow_enabled` setting will instead return `true` from `Gitlab::Tracking.enabled?` + - `snowplow_collector_hostname` setting will instead always return `localhost:9090` (or whatever is set for `SNOWPLOW_MICRO_URI`) from `Gitlab::Tracking.collector_hostname`. ```shell - gdk psql -d gitlabhq_development + export SNOWPLOW_MICRO_ENABLE=1 ``` -1. Update your instance's settings to enable Snowplow events and - point to the Snowplow Micro collector: + Optionally, you can set the URI for you Snowplow Micro instance as well (the default value is `http://localhost:9090`): ```shell - update application_settings set snowplow_collector_hostname='localhost:9090', snowplow_enabled=true, snowplow_cookie_domain='.gitlab.com'; - ``` - -1. Update `DEFAULT_SNOWPLOW_OPTIONS` in `app/assets/javascripts/tracking/constants.js` to remove `forceSecureTracker: true`: - - ```diff - diff --git a/app/assets/javascripts/tracking/constants.js b/app/assets/javascripts/tracking/constants.js - index 598111e4086..eff38074d4c 100644 - --- a/app/assets/javascripts/tracking/constants.js - +++ b/app/assets/javascripts/tracking/constants.js - @@ -7,7 +7,6 @@ export const DEFAULT_SNOWPLOW_OPTIONS = { - appId: '', - userFingerprint: false, - respectDoNotTrack: true, - - forceSecureTracker: true, - eventMethod: 'post', - contexts: { webPage: true, performanceTiming: true }, - formTracking: false, - ``` - -1. Update `options` in `lib/gitlab/tracking.rb` to add `protocol` and `port`: - - ```diff - diff --git a/lib/gitlab/tracking.rb b/lib/gitlab/tracking.rb - index 618e359211b..e9084623c43 100644 - --- a/lib/gitlab/tracking.rb - +++ b/lib/gitlab/tracking.rb - @@ -41,7 +41,9 @@ def options(group) - cookie_domain: Gitlab::CurrentSettings.snowplow_cookie_domain, - app_id: Gitlab::CurrentSettings.snowplow_app_id, - form_tracking: additional_features, - - link_click_tracking: additional_features - + link_click_tracking: additional_features, - + protocol: 'http', - + port: 9090 - }.transform_keys! { |key| key.to_s.camelize(:lower).to_sym } - end - ``` - -1. Update `emitter` in `lib/gitlab/tracking/destinations/snowplow.rb` to change `protocol`: - - ```diff - diff --git a/lib/gitlab/tracking/destinations/snowplow.rb b/lib/gitlab/tracking/destinations/snowplow.rb - index 4fa844de325..5dd9d0eacfb 100644 - --- a/lib/gitlab/tracking/destinations/snowplow.rb - +++ b/lib/gitlab/tracking/destinations/snowplow.rb - @@ -40,7 +40,7 @@ def tracker - def emitter - SnowplowTracker::AsyncEmitter.new( - Gitlab::CurrentSettings.snowplow_collector_hostname, - - protocol: 'https' - + protocol: 'http' - ) - end - end - + export SNOWPLOW_MICRO_URI=https://127.0.0.1:8080 ``` 1. Restart GDK: diff --git a/lib/gitlab/database/load_balancing.rb b/lib/gitlab/database/load_balancing.rb index 3e322e752b7..52eb0764ae3 100644 --- a/lib/gitlab/database/load_balancing.rb +++ b/lib/gitlab/database/load_balancing.rb @@ -26,7 +26,7 @@ module Gitlab return to_enum(__method__) unless block_given? base_models.each do |model| - yield model.connection.load_balancer + yield model.load_balancer end end diff --git a/lib/gitlab/database/load_balancing/connection_proxy.rb b/lib/gitlab/database/load_balancing/connection_proxy.rb index 1be63da8896..9c51fa21936 100644 --- a/lib/gitlab/database/load_balancing/connection_proxy.rb +++ b/lib/gitlab/database/load_balancing/connection_proxy.rb @@ -13,7 +13,15 @@ module Gitlab WriteInsideReadOnlyTransactionError = Class.new(StandardError) READ_ONLY_TRANSACTION_KEY = :load_balacing_read_only_transaction - attr_reader :load_balancer + # The load balancer is intentionally not exposed since the returned instance + # might be different `model.connection.load_balancer` vs `model.load_balancer` + # + # The used `model.connection` is dependent on `use_model_load_balancing`. + # See more in: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73949. + # + # Always use `model.load_balancer` or `model.sticking`. + # + # attr_reader :load_balancer # These methods perform writes after which we need to stick to the # primary. diff --git a/lib/gitlab/database/load_balancing/setup.rb b/lib/gitlab/database/load_balancing/setup.rb index bcb40e45dac..ef38f42f50b 100644 --- a/lib/gitlab/database/load_balancing/setup.rb +++ b/lib/gitlab/database/load_balancing/setup.rb @@ -40,6 +40,7 @@ module Gitlab def setup_connection_proxy # We just use a simple `class_attribute` here so we don't need to # inject any modules and/or expose unnecessary methods. + setup_class_attribute(:load_balancer, load_balancer) setup_class_attribute(:connection, ConnectionProxy.new(load_balancer)) setup_class_attribute(:sticking, Sticking.new(load_balancer)) end @@ -107,10 +108,6 @@ module Gitlab def connection use_model_load_balancing? ? super : ActiveRecord::Base.connection end - - def sticking - use_model_load_balancing? ? super : ActiveRecord::Base.sticking - end # rubocop:enable Database/MultipleDatabases end end diff --git a/lib/gitlab/tracking.rb b/lib/gitlab/tracking.rb index 1f280765bf1..ec032cf2d3c 100644 --- a/lib/gitlab/tracking.rb +++ b/lib/gitlab/tracking.rb @@ -6,7 +6,7 @@ module Gitlab class << self def enabled? - Gitlab::CurrentSettings.snowplow_enabled? + snowplow_micro_enabled? || Gitlab::CurrentSettings.snowplow_enabled? end def event(category, action, label: nil, property: nil, value: nil, context: [], project: nil, user: nil, namespace: nil, **extra) # rubocop:disable Metrics/ParameterLists @@ -18,21 +18,25 @@ module Gitlab end def options(group) - additional_features = Feature.enabled?(:additional_snowplow_tracking, group, type: :ops) - { - namespace: SNOWPLOW_NAMESPACE, - hostname: Gitlab::CurrentSettings.snowplow_collector_hostname, - cookie_domain: Gitlab::CurrentSettings.snowplow_cookie_domain, - app_id: Gitlab::CurrentSettings.snowplow_app_id, - form_tracking: additional_features, - link_click_tracking: additional_features - }.transform_keys! { |key| key.to_s.camelize(:lower).to_sym } + snowplow.options(group) + end + + def collector_hostname + snowplow.hostname end private def snowplow - @snowplow ||= Gitlab::Tracking::Destinations::Snowplow.new + @snowplow ||= if snowplow_micro_enabled? + Gitlab::Tracking::Destinations::SnowplowMicro.new + else + Gitlab::Tracking::Destinations::Snowplow.new + end + end + + def snowplow_micro_enabled? + Rails.env.development? && Gitlab::Utils.to_boolean(ENV['SNOWPLOW_MICRO_ENABLE']) end end end diff --git a/lib/gitlab/tracking/destinations/snowplow.rb b/lib/gitlab/tracking/destinations/snowplow.rb index 07a53b0892b..5596e9acd30 100644 --- a/lib/gitlab/tracking/destinations/snowplow.rb +++ b/lib/gitlab/tracking/destinations/snowplow.rb @@ -16,25 +16,53 @@ module Gitlab increment_total_events_counter end + def options(group) + additional_features = Feature.enabled?(:additional_snowplow_tracking, group, type: :ops) + { + namespace: Gitlab::Tracking::SNOWPLOW_NAMESPACE, + hostname: hostname, + cookie_domain: cookie_domain, + app_id: app_id, + form_tracking: additional_features, + link_click_tracking: additional_features + }.transform_keys! { |key| key.to_s.camelize(:lower).to_sym } + end + + def hostname + Gitlab::CurrentSettings.snowplow_collector_hostname + end + private def enabled? Gitlab::Tracking.enabled? end + def app_id + Gitlab::CurrentSettings.snowplow_app_id + end + + def protocol + 'https' + end + + def cookie_domain + Gitlab::CurrentSettings.snowplow_cookie_domain + end + def tracker @tracker ||= SnowplowTracker::Tracker.new( emitter, SnowplowTracker::Subject.new, Gitlab::Tracking::SNOWPLOW_NAMESPACE, - Gitlab::CurrentSettings.snowplow_app_id + app_id ) end def emitter SnowplowTracker::AsyncEmitter.new( - Gitlab::CurrentSettings.snowplow_collector_hostname, - protocol: 'https', + hostname, + protocol: protocol, on_success: method(:increment_successful_events_emissions), on_failure: method(:failure_callback) ) @@ -68,8 +96,6 @@ module Gitlab end def log_failures(failures) - hostname = Gitlab::CurrentSettings.snowplow_collector_hostname - failures.each do |failure| Gitlab::AppLogger.error("#{failure["se_ca"]} #{failure["se_ac"]} failed to be reported to collector at #{hostname}") end diff --git a/lib/gitlab/tracking/destinations/snowplow_micro.rb b/lib/gitlab/tracking/destinations/snowplow_micro.rb new file mode 100644 index 00000000000..b818d349a6d --- /dev/null +++ b/lib/gitlab/tracking/destinations/snowplow_micro.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true +# +module Gitlab + module Tracking + module Destinations + class SnowplowMicro < Snowplow + include ::Gitlab::Utils::StrongMemoize + extend ::Gitlab::Utils::Override + + DEFAULT_URI = 'http://localhost:9090' + + override :options + def options(group) + super.update( + protocol: uri.scheme, + port: uri.port, + force_secure_tracker: false + ) + end + + override :hostname + def hostname + "#{uri.host}:#{uri.port}" + end + + private + + def uri + strong_memoize(:snowplow_uri) do + uri = URI(ENV['SNOWPLOW_MICRO_URI'] || DEFAULT_URI) + uri = URI("http://#{ENV['SNOWPLOW_MICRO_URI']}") unless %w[http https].include?(uri.scheme) + uri + end + end + + override :cookie_domain + def cookie_domain + '.gitlab.com' + end + + override :protocol + def protocol + uri.scheme + end + end + end + end +end diff --git a/lib/gitlab/usage_data_counters/known_events/code_review_events.yml b/lib/gitlab/usage_data_counters/known_events/code_review_events.yml index d4a818f8fe0..429667b62e3 100644 --- a/lib/gitlab/usage_data_counters/known_events/code_review_events.yml +++ b/lib/gitlab/usage_data_counters/known_events/code_review_events.yml @@ -249,3 +249,7 @@ redis_slot: code_review category: code_review aggregation: weekly +- name: i_code_review_widget_nothing_merge_click_new_file + redis_slot: code_review + category: code_review + aggregation: weekly diff --git a/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb b/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb index b81f9c26421..ee2718171c0 100644 --- a/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb @@ -3,12 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do - let(:proxy) do - config = Gitlab::Database::LoadBalancing::Configuration - .new(ActiveRecord::Base) - - described_class.new(Gitlab::Database::LoadBalancing::LoadBalancer.new(config)) - end + let(:config) { Gitlab::Database::LoadBalancing::Configuration.new(ActiveRecord::Base) } + let(:load_balancer) { Gitlab::Database::LoadBalancing::LoadBalancer.new(config) } + let(:proxy) { described_class.new(load_balancer) } describe '#select' do it 'performs a read' do @@ -143,9 +140,9 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do context 'with a read query' do it 'runs the transaction and any nested queries on the replica' do - expect(proxy.load_balancer).to receive(:read) + expect(load_balancer).to receive(:read) .twice.and_yield(replica) - expect(proxy.load_balancer).not_to receive(:read_write) + expect(load_balancer).not_to receive(:read_write) expect(session).not_to receive(:write!) proxy.transaction { proxy.select('true') } @@ -154,8 +151,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do context 'with a write query' do it 'raises an exception' do - allow(proxy.load_balancer).to receive(:read).and_yield(replica) - allow(proxy.load_balancer).to receive(:read_write).and_yield(replica) + allow(load_balancer).to receive(:read).and_yield(replica) + allow(load_balancer).to receive(:read_write).and_yield(replica) expect do proxy.transaction { proxy.insert('something') } @@ -178,9 +175,9 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do context 'with a read query' do it 'runs the transaction and any nested queries on the primary and stick to it' do - expect(proxy.load_balancer).to receive(:read_write) + expect(load_balancer).to receive(:read_write) .twice.and_yield(primary) - expect(proxy.load_balancer).not_to receive(:read) + expect(load_balancer).not_to receive(:read) expect(session).to receive(:write!) proxy.transaction { proxy.select('true') } @@ -189,9 +186,9 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do context 'with a write query' do it 'runs the transaction and any nested queries on the primary and stick to it' do - expect(proxy.load_balancer).to receive(:read_write) + expect(load_balancer).to receive(:read_write) .twice.and_yield(primary) - expect(proxy.load_balancer).not_to receive(:read) + expect(load_balancer).not_to receive(:read) expect(session).to receive(:write!).twice proxy.transaction { proxy.insert('something') } @@ -209,7 +206,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do end it 'properly forwards keyword arguments' do - allow(proxy.load_balancer).to receive(:read_write) + allow(load_balancer).to receive(:read_write) expect(proxy).to receive(:write_using_load_balancer).and_call_original @@ -234,7 +231,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do end it 'properly forwards keyword arguments' do - allow(proxy.load_balancer).to receive(:read) + allow(load_balancer).to receive(:read) expect(proxy).to receive(:read_using_load_balancer).and_call_original @@ -259,7 +256,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do allow(session).to receive(:use_replicas_for_read_queries?).and_return(false) expect(connection).to receive(:foo).with('foo') - expect(proxy.load_balancer).to receive(:read).and_yield(connection) + expect(load_balancer).to receive(:read).and_yield(connection) proxy.read_using_load_balancer(:foo, 'foo') end @@ -271,7 +268,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do allow(session).to receive(:use_replicas_for_read_queries?).and_return(true) expect(connection).to receive(:foo).with('foo') - expect(proxy.load_balancer).to receive(:read).and_yield(connection) + expect(load_balancer).to receive(:read).and_yield(connection) proxy.read_using_load_balancer(:foo, 'foo') end @@ -283,7 +280,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do allow(session).to receive(:use_replicas_for_read_queries?).and_return(true) expect(connection).to receive(:foo).with('foo') - expect(proxy.load_balancer).to receive(:read).and_yield(connection) + expect(load_balancer).to receive(:read).and_yield(connection) proxy.read_using_load_balancer(:foo, 'foo') end @@ -296,7 +293,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do expect(connection).to receive(:foo).with('foo') - expect(proxy.load_balancer).to receive(:read_write) + expect(load_balancer).to receive(:read_write) .and_yield(connection) proxy.read_using_load_balancer(:foo, 'foo') @@ -314,7 +311,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do end it 'uses but does not stick to the primary' do - expect(proxy.load_balancer).to receive(:read_write).and_yield(connection) + expect(load_balancer).to receive(:read_write).and_yield(connection) expect(connection).to receive(:foo).with('foo') expect(session).not_to receive(:write!) diff --git a/spec/lib/gitlab/database/load_balancing/setup_spec.rb b/spec/lib/gitlab/database/load_balancing/setup_spec.rb index dfdf7d19e72..953d83d3b48 100644 --- a/spec/lib/gitlab/database/load_balancing/setup_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/setup_spec.rb @@ -61,7 +61,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do setup.setup_connection_proxy - expect(model.connection.load_balancer).to eq(lb) + expect(model.load_balancer).to eq(lb) expect(model.sticking) .to be_an_instance_of(Gitlab::Database::LoadBalancing::Sticking) end @@ -265,17 +265,19 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do stub_env('GITLAB_USE_MODEL_LOAD_BALANCING', env_GITLAB_USE_MODEL_LOAD_BALANCING) stub_env('GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci', env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci) stub_feature_flags(use_model_load_balancing: ff_use_model_load_balancing) - end - it 'results match expectations' do - result = models.transform_values do |model| - # Make load balancer to force init with a dedicated replicas connections + # Make load balancer to force init with a dedicated replicas connections + models.each do |_, model| described_class.new(model).tap do |subject| subject.configuration.hosts = [subject.configuration.replica_db_config.host] subject.setup end + end + end - load_balancer = model.connection.load_balancer + it 'results match expectations' do + result = models.transform_values do |model| + load_balancer = model.connection.instance_variable_get(:@load_balancer) { read: load_balancer.read { |connection| connection.pool.db_config.name }, @@ -285,6 +287,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do expect(result).to eq(expectations) end + + it 'does return load_balancer assigned to a given connection' do + models.each do |name, model| + expect(model.load_balancer.name).to eq(name) + expect(model.sticking.instance_variable_get(:@load_balancer)).to eq(model.load_balancer) + end + end end end end diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb index 08dd6a0a788..9acf80e684f 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb @@ -181,11 +181,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do end context 'when worker data consistency is :delayed' do - include_examples 'mark data consistency location', :delayed + include_examples 'mark data consistency location', :delayed end context 'when worker data consistency is :sticky' do - include_examples 'mark data consistency location', :sticky + include_examples 'mark data consistency location', :sticky end end end diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb index 06efdcd8f99..de2ad662d16 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb @@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ let(:wal_locations) { { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => location } } it 'does not stick to the primary', :aggregate_failures do - expect(ActiveRecord::Base.connection.load_balancer) + expect(ActiveRecord::Base.load_balancer) .to receive(:select_up_to_date_host) .with(location) .and_return(true) @@ -107,7 +107,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'dedup_wal_locations' => wal_locations } } before do - allow(ActiveRecord::Base.connection.load_balancer) + allow(ActiveRecord::Base.load_balancer) .to receive(:select_up_to_date_host) .with(wal_locations[:main]) .and_return(true) @@ -120,7 +120,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'database_write_location' => '0/D525E3A8' } } before do - allow(ActiveRecord::Base.connection.load_balancer) + allow(ActiveRecord::Base.load_balancer) .to receive(:select_up_to_date_host) .with('0/D525E3A8') .and_return(true) diff --git a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb index 9d0937f004c..d88554614cf 100644 --- a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do let(:sticking) do - described_class.new(ActiveRecord::Base.connection.load_balancer) + described_class.new(ActiveRecord::Base.load_balancer) end after do @@ -73,7 +73,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do end describe '#all_caught_up?' do - let(:lb) { ActiveRecord::Base.connection.load_balancer } + let(:lb) { ActiveRecord::Base.load_balancer } let(:last_write_location) { 'foo' } before do @@ -137,7 +137,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do end describe '#unstick_or_continue_sticking' do - let(:lb) { ActiveRecord::Base.connection.load_balancer } + let(:lb) { ActiveRecord::Base.load_balancer } it 'simply returns if no write location could be found' do allow(sticking) @@ -182,13 +182,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do RSpec.shared_examples 'sticking' do before do - allow(ActiveRecord::Base.connection.load_balancer) + allow(ActiveRecord::Base.load_balancer) .to receive(:primary_write_location) .and_return('foo') end it 'sticks an entity to the primary', :aggregate_failures do - allow(ActiveRecord::Base.connection.load_balancer) + allow(ActiveRecord::Base.load_balancer) .to receive(:primary_only?) .and_return(false) @@ -227,11 +227,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do describe '#mark_primary_write_location' do it 'updates the write location with the load balancer' do - allow(ActiveRecord::Base.connection.load_balancer) + allow(ActiveRecord::Base.load_balancer) .to receive(:primary_write_location) .and_return('foo') - allow(ActiveRecord::Base.connection.load_balancer) + allow(ActiveRecord::Base.load_balancer) .to receive(:primary_only?) .and_return(false) @@ -291,7 +291,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do end describe '#select_caught_up_replicas' do - let(:lb) { ActiveRecord::Base.connection.load_balancer } + let(:lb) { ActiveRecord::Base.load_balancer } context 'with no write location' do before do diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb index d1b811deca9..65ffe539910 100644 --- a/spec/lib/gitlab/database/load_balancing_spec.rb +++ b/spec/lib/gitlab/database/load_balancing_spec.rb @@ -76,7 +76,7 @@ RSpec.describe Gitlab::Database::LoadBalancing do context 'when a read connection is used' do it 'returns :replica' do - proxy.load_balancer.read do |connection| + load_balancer.read do |connection| expect(described_class.db_role_for_connection(connection)).to eq(:replica) end end @@ -84,7 +84,7 @@ RSpec.describe Gitlab::Database::LoadBalancing do context 'when a read_write connection is used' do it 'returns :primary' do - proxy.load_balancer.read_write do |connection| + load_balancer.read_write do |connection| expect(described_class.db_role_for_connection(connection)).to eq(:primary) end end diff --git a/spec/lib/gitlab/database/query_analyzer_spec.rb b/spec/lib/gitlab/database/query_analyzer_spec.rb index b8356522e55..3567bd6a058 100644 --- a/spec/lib/gitlab/database/query_analyzer_spec.rb +++ b/spec/lib/gitlab/database/query_analyzer_spec.rb @@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::QueryAnalyzer do end def process_sql(sql) - ApplicationRecord.connection.load_balancer.read_write do |connection| + ApplicationRecord.load_balancer.read_write do |connection| described_class.new.send(:process_sql, sql, connection) end end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index a2e7b6d27b9..1ab08314a39 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -205,7 +205,7 @@ RSpec.describe Gitlab::Database do context 'when replicas are configured', :database_replica do it 'returns the name for a replica' do - replica = ActiveRecord::Base.connection.load_balancer.host + replica = ActiveRecord::Base.load_balancer.host expect(described_class.db_config_name(replica)).to eq('main_replica') end diff --git a/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb b/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb new file mode 100644 index 00000000000..6004698d092 --- /dev/null +++ b/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Tracking::Destinations::SnowplowMicro do + include StubENV + + before do + stub_application_setting(snowplow_enabled: true) + stub_env('SNOWPLOW_MICRO_ENABLE', '1') + allow(Rails.env).to receive(:development?).and_return(true) + end + + describe '#hostname' do + context 'when SNOWPLOW_MICRO_URI is set' do + before do + stub_env('SNOWPLOW_MICRO_URI', 'http://gdk.test:9091') + end + + it 'returns hostname URI part' do + expect(subject.hostname).to eq('gdk.test:9091') + end + end + + context 'when SNOWPLOW_MICRO_URI is without protocol' do + before do + stub_env('SNOWPLOW_MICRO_URI', 'gdk.test:9091') + end + + it 'returns hostname URI part' do + expect(subject.hostname).to eq('gdk.test:9091') + end + end + + context 'when SNOWPLOW_MICRO_URI is hostname only' do + before do + stub_env('SNOWPLOW_MICRO_URI', 'uriwithoutport') + end + + it 'returns hostname URI with default HTTP port' do + expect(subject.hostname).to eq('uriwithoutport:80') + end + end + + context 'when SNOWPLOW_MICRO_URI is not set' do + it 'returns localhost hostname' do + expect(subject.hostname).to eq('localhost:9090') + end + end + end +end diff --git a/spec/lib/gitlab/tracking_spec.rb b/spec/lib/gitlab/tracking_spec.rb index b9902ab42e5..61b2c89ffa1 100644 --- a/spec/lib/gitlab/tracking_spec.rb +++ b/spec/lib/gitlab/tracking_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' RSpec.describe Gitlab::Tracking do + include StubENV + before do stub_application_setting(snowplow_enabled: true) stub_application_setting(snowplow_collector_hostname: 'gitfoo.com') @@ -12,17 +14,62 @@ RSpec.describe Gitlab::Tracking do end describe '.options' do - it 'returns useful client options' do - expected_fields = { - namespace: 'gl', - hostname: 'gitfoo.com', - cookieDomain: '.gitfoo.com', - appId: '_abc123_', - formTracking: true, - linkClickTracking: true - } - - expect(subject.options(nil)).to match(expected_fields) + shared_examples 'delegates to destination' do |klass| + before do + allow_next_instance_of(klass) do |instance| + allow(instance).to receive(:options).and_call_original + end + end + + it "delegates to #{klass} destination" do + expect_next_instance_of(klass) do |instance| + expect(instance).to receive(:options) + end + + subject.options(nil) + end + end + + context 'when destination is Snowplow' do + it_behaves_like 'delegates to destination', Gitlab::Tracking::Destinations::Snowplow + + it 'returns useful client options' do + expected_fields = { + namespace: 'gl', + hostname: 'gitfoo.com', + cookieDomain: '.gitfoo.com', + appId: '_abc123_', + formTracking: true, + linkClickTracking: true + } + + expect(subject.options(nil)).to match(expected_fields) + end + end + + context 'when destination is SnowplowMicro' do + before do + stub_env('SNOWPLOW_MICRO_ENABLE', '1') + allow(Rails.env).to receive(:development?).and_return(true) + end + + it_behaves_like 'delegates to destination', Gitlab::Tracking::Destinations::SnowplowMicro + + it 'returns useful client options' do + expected_fields = { + namespace: 'gl', + hostname: 'localhost:9090', + cookieDomain: '.gitlab.com', + appId: '_abc123_', + protocol: 'http', + port: 9090, + force_secure_tracker: false, + formTracking: true, + linkClickTracking: true + } + + expect(subject.options(nil)).to match(expected_fields) + end end it 'when feature flag is disabled' do @@ -71,7 +118,23 @@ RSpec.describe Gitlab::Tracking do end end - it_behaves_like 'delegates to destination', Gitlab::Tracking::Destinations::Snowplow + context 'when destination is Snowplow' do + before do + stub_env('SNOWPLOW_MICRO_ENABLE', '0') + allow(Rails.env).to receive(:development?).and_return(true) + end + + it_behaves_like 'delegates to destination', Gitlab::Tracking::Destinations::Snowplow + end + + context 'when destination is SnowplowMicro' do + before do + stub_env('SNOWPLOW_MICRO_ENABLE', '1') + allow(Rails.env).to receive(:development?).and_return(true) + end + + it_behaves_like 'delegates to destination', Gitlab::Tracking::Destinations::SnowplowMicro + end it 'tracks errors' do expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with( diff --git a/spec/support/database_load_balancing.rb b/spec/support/database_load_balancing.rb index 014575e8a82..d2902ddcc7c 100644 --- a/spec/support/database_load_balancing.rb +++ b/spec/support/database_load_balancing.rb @@ -2,17 +2,17 @@ RSpec.configure do |config| config.around(:each, :database_replica) do |example| - old_proxies = [] + old_proxies = {} Gitlab::Database::LoadBalancing.base_models.each do |model| + old_proxies[model] = [model.load_balancer, model.connection, model.sticking] + config = Gitlab::Database::LoadBalancing::Configuration .new(model, [model.connection_db_config.configuration_hash[:host]]) - lb = Gitlab::Database::LoadBalancing::LoadBalancer.new(config) - - old_proxies << [model, model.connection] - model.connection = - Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb) + model.load_balancer = Gitlab::Database::LoadBalancing::LoadBalancer.new(config) + model.sticking = Gitlab::Database::LoadBalancing::Sticking.new(model.load_balancer) + model.connection = Gitlab::Database::LoadBalancing::ConnectionProxy.new(model.load_balancer) end Gitlab::Database::LoadBalancing::Session.clear_session @@ -23,8 +23,8 @@ RSpec.configure do |config| Gitlab::Database::LoadBalancing::Session.clear_session redis_shared_state_cleanup! - old_proxies.each do |(model, proxy)| - model.connection = proxy + old_proxies.each do |model, proxy| + model.load_balancer, model.connection, model.sticking = proxy end end end |