diff options
64 files changed, 2084 insertions, 118 deletions
diff --git a/CHANGELOG b/CHANGELOG index 8135a5d5180..425beb17743 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,11 +9,16 @@ v 8.4.0 (unreleased) - Expose Git's version in the admin area - Add "Frequently used" category to emoji picker - Add CAS support (tduehr) - - Add link to merge request on build detail page. + - Add link to merge request on build detail page - Revert back upvote and downvote button to the issue and MR pages + - Enable "Add key" button when user fills in a proper key (Stan Hu) -v 8.3.2 (unreleased) - - Enable "Add key" button when user fills in a proper key +v 8.3.3 (unreleased) + - Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu) + +v 8.3.2 + - Disable --follow in `git log` to avoid loading duplicate commit data in infinite scroll (Stan Hu) + - Add support for Google reCAPTCHA in user registration v 8.3.1 - Fix Error 500 when global milestones have slashes (Stan Hu) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 950824e35ab..b9c2b3d2f8e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,6 +155,28 @@ sudo -u git -H bundle exec rake gitlab:env:info) ``` +### Issue weight + +Issue weight allows us to get an idea of the amount of work required to solve +one or multiple issues. This makes it possible to schedule work more accurately. + +You are encouraged to set the weight of any issue. Following the guidelines +below will make it easy to manage this, without unnecessary overhead. + +1. Set weight for any issue at the earliest possible convenience +1. If you don't agree with a set weight, discuss with other developers until +consensus is reached about the weight +1. Issue weights are an abstract measurement of complexity of the issue. Do not +relate issue weight directly to time. This is called [anchoring](https://en.wikipedia.org/wiki/Anchoring) +and something you want to avoid. +1. Something that has a weight of 1 (or no weight) is really small and simple. +Something that is 9 is rewriting a large fundamental part of GitLab, +which might lead to many hard problems to solve. Changing some text in GitLab +is probably 1, adding a new Git Hook maybe 4 or 5, big features 7-9. +1. If something is very large, it should probably be split up in multiple +issues or chunks. You can simply not set the weight of a parent issue and set +weights to children issues. + ## Merge requests We welcome merge requests with fixes and improvements to GitLab code, tests, @@ -35,6 +35,9 @@ gem 'omniauth-twitter', '~> 1.2.0' gem 'omniauth_crowd' gem 'rack-oauth2', '~> 1.2.1' +# reCAPTCHA protection +gem 'recaptcha', require: 'recaptcha/rails' + # Two-factor authentication gem 'devise-two-factor', '~> 2.0.0' gem 'rqrcode-rails3', '~> 0.1.7' @@ -212,6 +215,14 @@ gem 'select2-rails', '~> 3.5.9' gem 'virtus', '~> 1.0.1' gem 'net-ssh', '~> 3.0.1' +# Metrics +group :metrics do + gem 'allocations', '~> 1.0', require: false, platform: :mri + gem 'method_source', '~> 0.8', require: false + gem 'influxdb', '~> 0.2', require: false + gem 'connection_pool', '~> 2.0', require: false +end + group :development do gem "foreman" gem 'brakeman', '~> 3.1.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 59051b9f36f..c4cadbafa26 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -49,6 +49,7 @@ GEM addressable (2.3.8) after_commit_queue (1.3.0) activerecord (>= 3.0) + allocations (1.0.1) annotate (2.6.10) activerecord (>= 3.2, <= 4.3) rake (~> 10.4) @@ -65,7 +66,7 @@ GEM attr_encrypted (1.3.4) encryptor (>= 1.3.0) attr_required (1.0.0) - autoprefixer-rails (6.1.1) + autoprefixer-rails (6.1.2) execjs json awesome_print (1.2.0) @@ -104,7 +105,7 @@ GEM bundler-audit (0.4.0) bundler (~> 1.2) thor (~> 0.18) - byebug (8.2.0) + byebug (8.2.1) cal-heatmap-rails (0.0.1) capybara (2.4.4) mime-types (>= 1.16) @@ -119,6 +120,7 @@ GEM activemodel (>= 3.2.0) activesupport (>= 3.2.0) json (>= 1.7) + cause (0.1) charlock_holmes (0.7.3) chunky_png (1.3.5) cliver (0.3.2) @@ -142,10 +144,10 @@ GEM term-ansicolor (~> 1.3) thor (~> 0.19.1) tins (~> 1.6.0) - crack (0.4.2) + crack (0.4.3) safe_yaml (~> 1.0.0) creole (0.5.0) - d3_rails (3.5.6) + d3_rails (3.5.11) railties (>= 3.1.0) daemons (1.2.3) database_cleaner (1.4.1) @@ -232,7 +234,7 @@ GEM ipaddress (~> 0.5) nokogiri (~> 1.5, >= 1.5.11) opennebula - fog-brightbox (0.9.0) + fog-brightbox (0.10.1) fog-core (~> 1.22) fog-json inflecto (~> 0.0.2) @@ -251,7 +253,7 @@ GEM fog-core (>= 1.21.0) fog-json fog-xml (>= 0.0.1) - fog-sakuracloud (1.4.0) + fog-sakuracloud (1.5.0) fog-core fog-json fog-softlayer (1.0.2) @@ -279,11 +281,11 @@ GEM ruby-progressbar (~> 1.4) gemnasium-gitlab-service (0.2.6) rugged (~> 0.21) - gemojione (2.1.0) + gemojione (2.1.1) json get_process_mem (0.2.0) gherkin-ruby (0.3.2) - github-linguist (4.7.2) + github-linguist (4.7.3) charlock_holmes (~> 0.7.3) escape_utils (~> 1.1.0) mime-types (>= 1.19) @@ -300,7 +302,7 @@ GEM posix-spawn (~> 0.3) gitlab_emoji (0.2.0) gemojione (~> 2.1) - gitlab_git (7.2.21) + gitlab_git (7.2.22) activesupport (~> 4.0) charlock_holmes (~> 0.7.3) github-linguist (~> 4.7.0) @@ -372,6 +374,9 @@ GEM i18n (0.7.0) ice_nine (0.11.1) inflecto (0.0.2) + influxdb (0.2.3) + cause + json ipaddress (0.8.0) jquery-atwho-rails (1.3.2) jquery-rails (4.0.5) @@ -419,7 +424,7 @@ GEM net-ldap (0.12.1) net-ssh (3.0.1) netrc (0.11.0) - newrelic-grape (2.0.0) + newrelic-grape (2.1.0) grape newrelic_rpm newrelic_rpm (3.9.4.245) @@ -568,6 +573,8 @@ GEM trollop rdoc (3.12.2) json (~> 1.4) + recaptcha (1.0.2) + json redcarpet (3.3.3) redis (3.2.2) redis-actionpack (4.0.1) @@ -795,7 +802,7 @@ GEM coercible (~> 1.0) descendants_tracker (~> 0.0, >= 0.0.3) equalizer (~> 0.0, >= 0.0.9) - warden (1.2.3) + warden (1.2.4) rack (>= 1.0) web-console (2.2.1) activemodel (>= 4.0) @@ -826,6 +833,7 @@ DEPENDENCIES acts-as-taggable-on (~> 3.4) addressable (~> 2.3.8) after_commit_queue + allocations (~> 1.0) annotate (~> 2.6.0) asana (~> 0.4.0) asciidoctor (~> 1.5.2) @@ -848,6 +856,7 @@ DEPENDENCIES charlock_holmes (~> 0.7.3) coffee-rails (~> 4.1.0) colorize (~> 0.7.0) + connection_pool (~> 2.0) coveralls (~> 0.8.2) creole (~> 0.5.0) d3_rails (~> 3.5.5) @@ -885,6 +894,7 @@ DEPENDENCIES hipchat (~> 1.5.0) html-pipeline (~> 1.11.0) httparty (~> 0.13.3) + influxdb (~> 0.2) jquery-atwho-rails (~> 1.3.2) jquery-rails (~> 4.0.0) jquery-scrollto-rails (~> 1.4.3) @@ -893,6 +903,7 @@ DEPENDENCIES kaminari (~> 0.16.3) letter_opener (~> 1.1.2) mail_room (~> 0.6.1) + method_source (~> 0.8) minitest (~> 5.7.0) mousetrap-rails (~> 1.4.6) mysql2 (~> 0.3.16) @@ -930,6 +941,7 @@ DEPENDENCIES raphael-rails (~> 2.1.2) rblineprof rdoc (~> 3.6) + recaptcha redcarpet (~> 3.3.3) redis-namespace redis-rails (~> 4.0.0) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 599b4c49540..69e061ce6e9 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -49,7 +49,7 @@ class Dispatcher new DropzoneInput($('.release-form')) when 'projects:merge_requests:show' new Diff() - shortcut_handler = new ShortcutsIssuable() + shortcut_handler = new ShortcutsIssuable(true) new ZenMode() when "projects:merge_requests:diffs" new Diff() diff --git a/app/assets/javascripts/shortcuts.js.coffee b/app/assets/javascripts/shortcuts.js.coffee index e9aeb1e9525..4d915bfc8c5 100644 --- a/app/assets/javascripts/shortcuts.js.coffee +++ b/app/assets/javascripts/shortcuts.js.coffee @@ -7,7 +7,7 @@ class @Shortcuts selectiveHelp: (e) => Shortcuts.showHelp(e, @enabledHelp) - + @showHelp: (e, location) -> if $('#modal-shortcuts').length > 0 $('#modal-shortcuts').modal('show') @@ -17,8 +17,7 @@ class @Shortcuts dataType: 'script', success: (e) -> if location and location.length > 0 - for l in location - $(l).show() + $(l).show() for l in location else $('.hidden-shortcut').show() $('.js-more-help-button').remove() @@ -28,3 +27,8 @@ class @Shortcuts @focusSearch: (e) -> $('#search').focus() e.preventDefault() + +$(document).on 'click.more_help', '.js-more-help-button', (e) -> + $(@).remove() + $('.hidden-shortcut').show() + e.preventDefault() diff --git a/app/assets/javascripts/users_select.js.coffee b/app/assets/javascripts/users_select.js.coffee index 12abf806bfa..9467011799f 100644 --- a/app/assets/javascripts/users_select.js.coffee +++ b/app/assets/javascripts/users_select.js.coffee @@ -117,5 +117,5 @@ class @UsersSelect callback(users) buildUrl: (url) -> - url = gon.relative_url_root + url if gon.relative_url_root? + url = gon.relative_url_root.replace(/\/$/, '') + url if gon.relative_url_root? return url diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 2f4a855c118..10e736fd362 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -67,6 +67,17 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :user_oauth_applications, :shared_runners_enabled, :max_artifacts_size, + :metrics_enabled, + :metrics_host, + :metrics_port, + :metrics_username, + :metrics_password, + :metrics_pool_size, + :metrics_timeout, + :metrics_method_call_threshold, + :recaptcha_enabled, + :recaptcha_site_key, + :recaptcha_private_key, restricted_visibility_levels: [], import_sources: [] ) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 3b3dc86cb68..c48175a4c5a 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -1,10 +1,21 @@ class RegistrationsController < Devise::RegistrationsController before_action :signup_enabled? + include Recaptcha::Verify def new redirect_to(new_user_session_path) end + def create + if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha + super + else + flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code." + flash.delete :recaptcha_error + render action: 'new' + end + end + def destroy DeleteUserService.new(current_user).execute(current_user) @@ -38,4 +49,16 @@ class RegistrationsController < Devise::RegistrationsController def sign_up_params params.require(:user).permit(:username, :email, :name, :password, :password_confirmation) end + + def resource_name + :user + end + + def resource + @resource ||= User.new(sign_up_params) + end + + def devise_mapping + @devise_mapping ||= Devise.mappings[:user] + end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1b60d3e27d0..825f85199be 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,9 +1,11 @@ class SessionsController < Devise::SessionsController include AuthenticatesWithTwoFactor + include Recaptcha::ClientHelper prepend_before_action :authenticate_with_two_factor, only: [:create] prepend_before_action :store_redirect_path, only: [:new] before_action :auto_sign_in_with_provider, only: [:new] + before_action :load_recaptcha def new if Gitlab.config.ldap.enabled @@ -40,7 +42,7 @@ class SessionsController < Devise::SessionsController User.find(session[:otp_user_id]) end end - + def store_redirect_path redirect_path = if request.referer.present? && (params['redirect_to_referer'] == 'yes') @@ -87,14 +89,14 @@ class SessionsController < Devise::SessionsController provider = Gitlab.config.omniauth.auto_sign_in_with_provider return unless provider.present? - # Auto sign in with an Omniauth provider only if the standard "you need to sign-in" alert is - # registered or no alert at all. In case of another alert (such as a blocked user), it is safer + # Auto sign in with an Omniauth provider only if the standard "you need to sign-in" alert is + # registered or no alert at all. In case of another alert (such as a blocked user), it is safer # to do nothing to prevent redirection loops with certain Omniauth providers. return unless flash[:alert].blank? || flash[:alert] == I18n.t('devise.failure.unauthenticated') - + # Prevent alert from popping up on the first page shown after authentication. - flash[:alert] = nil - + flash[:alert] = nil + redirect_to user_omniauth_authorize_path(provider.to_sym) end @@ -107,4 +109,8 @@ class SessionsController < Devise::SessionsController AuditEventService.new(user, user, options). for_authentication.security_event end + + def load_recaptcha + Gitlab::Recaptcha.load_configurations! + end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0b00b9a0702..f7f7a1a02d3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -72,7 +72,7 @@ module ApplicationHelper if user_or_email.is_a?(User) user = user_or_email else - user = User.find_by(email: user_or_email) + user = User.find_by(email: user_or_email.downcase) end if user diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 7c107da116c..be69d317d73 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -44,24 +44,32 @@ class ApplicationSetting < ActiveRecord::Base attr_accessor :restricted_signup_domains_raw validates :session_expire_delay, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: 0 } + presence: true, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :home_page_url, - allow_blank: true, - url: true, - if: :home_page_url_column_exist + allow_blank: true, + url: true, + if: :home_page_url_column_exist validates :after_sign_out_path, - allow_blank: true, - url: true + allow_blank: true, + url: true validates :admin_notification_email, - allow_blank: true, - email: true + allow_blank: true, + email: true validates :two_factor_grace_period, - numericality: { greater_than_or_equal_to: 0 } + numericality: { greater_than_or_equal_to: 0 } + + validates :recaptcha_site_key, + presence: true, + if: :recaptcha_enabled + + validates :recaptcha_private_key, + presence: true, + if: :recaptcha_enabled validates_each :restricted_visibility_levels do |record, attr, value| unless value.nil? diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 919833f6df5..18a00f95b48 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -95,14 +95,12 @@ module Issuable opened? || reopened? end - # Deprecated. Still exists to preserve API compatibility. def downvotes - 0 + notes.awards.where(note: "thumbsdown").count end - # Deprecated. Still exists to preserve API compatibility. def upvotes - 0 + notes.awards.where(note: "thumbsup").count end def subscribed?(user) diff --git a/app/models/note.rb b/app/models/note.rb index 1222d99cf1f..3d5b663c99f 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -346,14 +346,12 @@ class Note < ActiveRecord::Base read_attribute(:system) end - # Deprecated. Still exists to preserve API compatibility. def downvote? - false + is_award && note == "thumbsdown" end - # Deprecated. Still exists to preserve API compatibility. def upvote? - false + is_award && note == "thumbsup" end def editable? diff --git a/app/models/project.rb b/app/models/project.rb index 75f85310d5f..017471995ec 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -555,7 +555,9 @@ class Project < ActiveRecord::Base end def send_move_instructions(old_path_with_namespace) - NotificationService.new.project_was_moved(self, old_path_with_namespace) + # New project path needs to be committed to the DB or notification will + # retrieve stale information + run_after_commit { NotificationService.new.project_was_moved(self, old_path_with_namespace) } end def owner diff --git a/app/models/repository.rb b/app/models/repository.rb index 9f688e3b45b..a9bf4eb4033 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -76,7 +76,9 @@ class Repository path: path, limit: limit, offset: offset, - follow: path.present? + # --follow doesn't play well with --skip. See: + # https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 + follow: false } commits = Gitlab::Git::Commit.where(options) diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 58f5c621f4a..214e0209bb7 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -156,5 +156,82 @@ .col-sm-10 = f.number_field :max_artifacts_size, class: 'form-control' + %fieldset + %legend Metrics + %p + These settings require a restart to take effect. + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :metrics_enabled do + = f.check_box :metrics_enabled + Enable InfluxDB Metrics + .form-group + = f.label :metrics_host, 'InfluxDB host', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :metrics_host, class: 'form-control', placeholder: 'influxdb.example.com' + .form-group + = f.label :metrics_port, 'InfluxDB port', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :metrics_port, class: 'form-control', placeholder: '8089' + .help-block + The UDP port to use for connecting to InfluxDB. InfluxDB requires that + your server configuration specifies a database to store data in when + sending messages to this port, without it metrics data will not be + saved. + .form-group + = f.label :metrics_username, 'InfluxDB username', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :metrics_username, class: 'form-control' + .form-group + = f.label :metrics_password, 'InfluxDB password', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :metrics_password, class: 'form-control' + .form-group + = f.label :metrics_pool_size, 'Connection pool size', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :metrics_pool_size, class: 'form-control' + .help-block + The amount of InfluxDB connections to open. Connections are opened + lazily. Users using multi-threaded application servers should ensure + enough connections are available (at minimum the amount of application + server threads). + .form-group + = f.label :metrics_timeout, 'Connection timeout', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :metrics_timeout, class: 'form-control' + .help-block + The amount of seconds after which an InfluxDB connection will time + out. + .form-group + = f.label :metrics_method_call_threshold, 'Method Call Threshold (ms)', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :metrics_method_call_threshold, class: 'form-control' + .help-block + A method call is only tracked when it takes longer to complete than + the given amount of milliseconds. + + %fieldset + %legend Spam and Anti-bot Protection + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :recaptcha_enabled do + = f.check_box :recaptcha_enabled + Enable reCAPTCHA + %span.help-block#recaptcha_help_block Helps preventing bots from creating accounts + + .form-group + = f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :recaptcha_site_key, class: 'form-control' + .help-block + Generate site and private keys here: + %a{ href: 'http://www.google.com/recaptcha', target: 'blank'} http://www.google.com/recaptcha + .form-group + = f.label :recaptcha_private_key, 'reCAPTCHA Private Key', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :recaptcha_private_key, class: 'form-control' + .form-actions = f.submit 'Save', class: 'btn btn-primary' diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 9dc6aeffd59..cb93ff2465e 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -6,17 +6,21 @@ .login-heading %h3 Create an account .login-body + - user = params[:user].present? ? params[:user] : {} = form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| .devise-errors = devise_error_messages! %div - = f.text_field :name, class: "form-control top", placeholder: "Name", required: true + = f.text_field :name, class: "form-control top", value: user[:name], placeholder: "Name", required: true %div - = f.text_field :username, class: "form-control middle", placeholder: "Username", required: true + = f.text_field :username, class: "form-control middle", value: user[:username], placeholder: "Username", required: true %div - = f.email_field :email, class: "form-control middle", placeholder: "Email", required: true + = f.email_field :email, class: "form-control middle", value: user[:email], placeholder: "Email", required: true .form-group.append-bottom-20#password-strength - = f.password_field :password, class: "form-control bottom", id: "user_password_sign_up", placeholder: "Password", required: true + = f.password_field :password, class: "form-control bottom", value: user[:password], id: "user_password_sign_up", placeholder: "Password", required: true + %div + - if current_application_settings.recaptcha_enabled + = recaptcha_tags %div = f.submit "Sign up", class: "btn-create btn" diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 8daac585960..1dea77c2e96 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -31,7 +31,7 @@ .col-sm-10 .checkbox = f.check_box :public - %span.descr Make this group public (even if there is no any public project inside this group) + %span.descr Make this group public (even if there are no public projects inside this group) .form-actions = f.submit 'Save group', class: "btn btn-save" diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml index 7e801b5332d..e8e331dd109 100644 --- a/app/views/help/_shortcuts.html.haml +++ b/app/views/help/_shortcuts.html.haml @@ -219,11 +219,3 @@ %td.shortcut .key r %td Reply (quoting selected text) - - -:javascript - $('.js-more-help-button').click(function (e) { - $(this).remove()l - $('.hidden-shortcut').show(); - e.preventDefault(); - }); diff --git a/config/database.yml.env b/config/database.yml.env new file mode 100644 index 00000000000..b2ff23cb5ab --- /dev/null +++ b/config/database.yml.env @@ -0,0 +1,9 @@ +<%= ENV['RAILS_ENV'] %>: + adapter: <%= ENV['GITLAB_DATABASE_ADAPTER'] || 'postgresql' %> + encoding: <%= ENV['GITLAB_DATABASE_ENCODING'] || 'unicode' %> + database: <%= ENV['GITLAB_DATABASE_DATABASE'] || "gitlab_#{ENV['RAILS_ENV']}" %> + pool: <%= ENV['GITLAB_DATABASE_POOL'] || '10' %> + username: <%= ENV['GITLAB_DATABASE_USERNAME'] || 'root' %> + password: <%= ENV['GITLAB_DATABASE_PASSWORD'] || '' %> + host: <%= ENV['GITLAB_DATABASE_HOST'] || 'localhost' %> + port: <%= ENV['GITLAB_DATABASE_PORT'] || '5432' %> diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index db68b5512b8..2d9f730c183 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -4,8 +4,8 @@ # ########################### NOTE ##################################### # This file should not receive new settings. All configuration options # -# that do not require an application restart are being moved to # -# ApplicationSetting model! # +# * are being moved to ApplicationSetting model! # +# If a setting requires an application restart say so in that screen. # # If you change this file in a Merge Request, please also create # # a MR on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests # ######################################################################## diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 816cb0c02a9..dea59f4fec8 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -131,6 +131,7 @@ Settings.omniauth.cas3['session_duration'] ||= 8.hours Settings.omniauth['session_tickets'] ||= Settingslogic.new({}) Settings.omniauth.session_tickets['cas3'] = 'ticket' + Settings['shared'] ||= Settingslogic.new({}) Settings.shared['path'] = File.expand_path(Settings.shared['path'] || "shared", Rails.root) @@ -144,7 +145,7 @@ Settings.gitlab['default_projects_limit'] ||= 10 Settings.gitlab['default_branch_protection'] ||= 2 Settings.gitlab['default_can_create_group'] = true if Settings.gitlab['default_can_create_group'].nil? Settings.gitlab['default_theme'] = Gitlab::Themes::APPLICATION_DEFAULT if Settings.gitlab['default_theme'].nil? -Settings.gitlab['host'] ||= 'localhost' +Settings.gitlab['host'] ||= ENV['GITLAB_HOST'] || 'localhost' Settings.gitlab['ssh_host'] ||= Settings.gitlab.host Settings.gitlab['https'] = false if Settings.gitlab['https'].nil? Settings.gitlab['port'] ||= Settings.gitlab.https ? 443 : 80 diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb new file mode 100644 index 00000000000..2e4908192a1 --- /dev/null +++ b/config/initializers/metrics.rb @@ -0,0 +1,64 @@ +if Gitlab::Metrics.enabled? + require 'influxdb' + require 'socket' + require 'connection_pool' + require 'method_source' + + # These are manually require'd so the classes are registered properly with + # ActiveSupport. + require 'gitlab/metrics/subscribers/action_view' + require 'gitlab/metrics/subscribers/active_record' + + Gitlab::Application.configure do |config| + config.middleware.use(Gitlab::Metrics::RackMiddleware) + end + + Sidekiq.configure_server do |config| + config.server_middleware do |chain| + chain.add Gitlab::Metrics::SidekiqMiddleware + end + end + + # This instruments all methods residing in app/models that (appear to) use any + # of the ActiveRecord methods. This has to take place _after_ initializing as + # for some unknown reason calling eager_load! earlier breaks Devise. + Gitlab::Application.config.after_initialize do + Rails.application.eager_load! + + models = Rails.root.join('app', 'models').to_s + + regex = Regexp.union( + ActiveRecord::Querying.public_instance_methods(false).map(&:to_s) + ) + + Gitlab::Metrics::Instrumentation. + instrument_class_hierarchy(ActiveRecord::Base) do |klass, method| + # Instrumenting the ApplicationSetting class can lead to an infinite + # loop. Since the data is cached any way we don't really need to + # instrument it. + if klass == ApplicationSetting + false + else + loc = method.source_location + + loc && loc[0].start_with?(models) && method.source =~ regex + end + end + end + + Gitlab::Metrics::Instrumentation.configure do |config| + config.instrument_instance_methods(Gitlab::Shell) + + config.instrument_methods(Gitlab::Git) + + Gitlab::Git.constants.each do |name| + const = Gitlab::Git.const_get(name) + + config.instrument_methods(const) if const.is_a?(Module) + end + end + + GC::Profiler.enable + + Gitlab::Metrics::Sampler.new.start +end diff --git a/db/migrate/20151228150906_influxdb_settings.rb b/db/migrate/20151228150906_influxdb_settings.rb new file mode 100644 index 00000000000..3012bd52cfd --- /dev/null +++ b/db/migrate/20151228150906_influxdb_settings.rb @@ -0,0 +1,18 @@ +class InfluxdbSettings < ActiveRecord::Migration + def change + add_column :application_settings, :metrics_enabled, :boolean, default: false + + add_column :application_settings, :metrics_host, :string, + default: 'localhost' + + add_column :application_settings, :metrics_database, :string, + default: 'gitlab' + + add_column :application_settings, :metrics_username, :string + add_column :application_settings, :metrics_password, :string + add_column :application_settings, :metrics_pool_size, :integer, default: 16 + add_column :application_settings, :metrics_timeout, :integer, default: 10 + add_column :application_settings, :metrics_method_call_threshold, + :integer, default: 10 + end +end diff --git a/db/migrate/20151228175719_add_recaptcha_to_application_settings.rb b/db/migrate/20151228175719_add_recaptcha_to_application_settings.rb new file mode 100644 index 00000000000..259fd0248d2 --- /dev/null +++ b/db/migrate/20151228175719_add_recaptcha_to_application_settings.rb @@ -0,0 +1,9 @@ +class AddRecaptchaToApplicationSettings < ActiveRecord::Migration + def change + change_table :application_settings do |t| + t.boolean :recaptcha_enabled, default: false + t.string :recaptcha_site_key + t.string :recaptcha_private_key + end + end +end diff --git a/db/migrate/20151229102248_influxdb_udp_port_setting.rb b/db/migrate/20151229102248_influxdb_udp_port_setting.rb new file mode 100644 index 00000000000..ae0499f936d --- /dev/null +++ b/db/migrate/20151229102248_influxdb_udp_port_setting.rb @@ -0,0 +1,5 @@ +class InfluxdbUdpPortSetting < ActiveRecord::Migration + def change + add_column :application_settings, :metrics_port, :integer, default: 8089 + end +end diff --git a/db/migrate/20151229112614_influxdb_remote_database_setting.rb b/db/migrate/20151229112614_influxdb_remote_database_setting.rb new file mode 100644 index 00000000000..f0e1ee1e7a7 --- /dev/null +++ b/db/migrate/20151229112614_influxdb_remote_database_setting.rb @@ -0,0 +1,5 @@ +class InfluxdbRemoteDatabaseSetting < ActiveRecord::Migration + def change + remove_column :application_settings, :metrics_database + end +end diff --git a/db/schema.rb b/db/schema.rb index 49fa258660d..df7f72d5ad4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20151224123230) do +ActiveRecord::Schema.define(version: 20151229112614) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -33,25 +33,36 @@ ActiveRecord::Schema.define(version: 20151224123230) do t.datetime "created_at" t.datetime "updated_at" t.string "home_page_url" - t.integer "default_branch_protection", default: 2 - t.boolean "twitter_sharing_enabled", default: true + t.integer "default_branch_protection", default: 2 + t.boolean "twitter_sharing_enabled", default: true t.text "restricted_visibility_levels" - t.boolean "version_check_enabled", default: true - t.integer "max_attachment_size", default: 10, null: false + t.boolean "version_check_enabled", default: true + t.integer "max_attachment_size", default: 10, null: false t.integer "default_project_visibility" t.integer "default_snippet_visibility" t.text "restricted_signup_domains" - t.boolean "user_oauth_applications", default: true + t.boolean "user_oauth_applications", default: true t.string "after_sign_out_path" - t.integer "session_expire_delay", default: 10080, null: false + t.integer "session_expire_delay", default: 10080, null: false t.text "import_sources" t.text "help_page_text" t.string "admin_notification_email" - t.boolean "shared_runners_enabled", default: true, null: false - t.integer "max_artifacts_size", default: 100, null: false + t.boolean "shared_runners_enabled", default: true, null: false + t.integer "max_artifacts_size", default: 100, null: false t.string "runners_registration_token" - t.boolean "require_two_factor_authentication", default: false - t.integer "two_factor_grace_period", default: 48 + t.boolean "require_two_factor_authentication", default: false + t.integer "two_factor_grace_period", default: 48 + t.boolean "metrics_enabled", default: false + t.string "metrics_host", default: "localhost" + t.string "metrics_username" + t.string "metrics_password" + t.integer "metrics_pool_size", default: 16 + t.integer "metrics_timeout", default: 10 + t.integer "metrics_method_call_threshold", default: 10 + t.boolean "recaptcha_enabled", default: false + t.string "recaptcha_site_key" + t.string "recaptcha_private_key" + t.integer "metrics_port", default: 8089 end create_table "audit_events", force: :cascade do |t| @@ -785,12 +796,12 @@ ActiveRecord::Schema.define(version: 20151224123230) do add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree create_table "users", force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0 + t.integer "sign_in_count", default: 0 t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" @@ -798,22 +809,22 @@ ActiveRecord::Schema.define(version: 20151224123230) do t.datetime "created_at" t.datetime "updated_at" t.string "name" - t.boolean "admin", default: false, null: false - t.integer "projects_limit", default: 10 - t.string "skype", default: "", null: false - t.string "linkedin", default: "", null: false - t.string "twitter", default: "", null: false + t.boolean "admin", default: false, null: false + t.integer "projects_limit", default: 10 + t.string "skype", default: "", null: false + t.string "linkedin", default: "", null: false + t.string "twitter", default: "", null: false t.string "authentication_token" - t.integer "theme_id", default: 1, null: false + t.integer "theme_id", default: 1, null: false t.string "bio" - t.integer "failed_attempts", default: 0 + t.integer "failed_attempts", default: 0 t.datetime "locked_at" t.string "username" - t.boolean "can_create_group", default: true, null: false - t.boolean "can_create_team", default: true, null: false + t.boolean "can_create_group", default: true, null: false + t.boolean "can_create_team", default: true, null: false t.string "state" - t.integer "color_scheme_id", default: 1, null: false - t.integer "notification_level", default: 1, null: false + t.integer "color_scheme_id", default: 1, null: false + t.integer "notification_level", default: 1, null: false t.datetime "password_expires_at" t.integer "created_by_id" t.datetime "last_credential_check_at" @@ -822,23 +833,23 @@ ActiveRecord::Schema.define(version: 20151224123230) do t.datetime "confirmed_at" t.datetime "confirmation_sent_at" t.string "unconfirmed_email" - t.boolean "hide_no_ssh_key", default: false - t.string "website_url", default: "", null: false + t.boolean "hide_no_ssh_key", default: false + t.string "website_url", default: "", null: false t.string "notification_email" - t.boolean "hide_no_password", default: false - t.boolean "password_automatically_set", default: false + t.boolean "hide_no_password", default: false + t.boolean "password_automatically_set", default: false t.string "location" t.string "encrypted_otp_secret" t.string "encrypted_otp_secret_iv" t.string "encrypted_otp_secret_salt" - t.boolean "otp_required_for_login", default: false, null: false + t.boolean "otp_required_for_login", default: false, null: false t.text "otp_backup_codes" - t.string "public_email", default: "", null: false - t.integer "dashboard", default: 0 - t.integer "project_view", default: 0 + t.string "public_email", default: "", null: false + t.integer "dashboard", default: 0 + t.integer "project_view", default: 0 t.integer "consumed_timestep" - t.integer "layout", default: 0 - t.boolean "hide_project_limit", default: false + t.integer "layout", default: 0 + t.boolean "hide_project_limit", default: false t.string "unlock_token" t.datetime "otp_grace_period_started_at" end diff --git a/doc/README.md b/doc/README.md index f40a257b42f..d82ff8b908b 100644 --- a/doc/README.md +++ b/doc/README.md @@ -56,6 +56,7 @@ - [Issue closing](customization/issue_closing.md) Customize how to close an issue from commit messages. - [Libravatar](customization/libravatar.md) Use Libravatar for user avatars. - [Log system](logs/logs.md) Log system. +- [Environmental Variables](administration/environmental_variables.md) to configure GitLab. - [Operations](operations/README.md) Keeping GitLab up and running - [Raketasks](raketasks/README.md) Backups, maintenance, automatic web hook setup and the importing of projects. - [Security](security/README.md) Learn what you can do to further secure your GitLab instance. diff --git a/doc/administration/enviroment_variables.md b/doc/administration/enviroment_variables.md new file mode 100644 index 00000000000..d7f5cb7c21f --- /dev/null +++ b/doc/administration/enviroment_variables.md @@ -0,0 +1,45 @@ +# Environment Variables + +## Introduction + +Commonly people configure GitLab via the gitlab.rb configuration file in the Omnibus package. + +But if you prefer to use environment variables we allow that too. + +## Supported environment variables + +Variable | Type | Explanation +--- | --- | --- +GITLAB_ROOT_PASSWORD | string | sets the password for the `root` user on installation +GITLAB_HOST | url | hostname of the GitLab server includes http or https +RAILS_ENV | production/development/staging/test | Rails environment +DATABASE_URL | url | For example: postgresql://localhost/blog_development?pool=5 + +## Complete database variables + +As explained in the [Heroku documentation](https://devcenter.heroku.com/articles/rails-database-connection-behavior) the DATABASE_URL doesn't let you set: + +- adapter +- database +- username +- password +- host +- port + +To do so please `cp config/database.yml.env config/database.yml` and use the following variables: + +Variable | Default +--- | --- +GITLAB_DATABASE_ADAPTER | postgresql +GITLAB_DATABASE_ENCODING | unicode +GITLAB_DATABASE_DATABASE | gitlab_#{ENV['RAILS_ENV'] +GITLAB_DATABASE_POOL | 10 +GITLAB_DATABASE_USERNAME | root +GITLAB_DATABASE_PASSWORD | +GITLAB_DATABASE_HOST | localhost +GITLAB_DATABASE_PORT | 5432 + +## Other variables + +We welcome merge requests to make more settings configurable via variables. +Please stick to the naming scheme "GITLAB_#{name 1_settings.rb in upper case}". diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 366a1f8abec..8bc0a67067a 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -4,8 +4,7 @@ Get all merge requests for this project. The `state` parameter can be used to get only merge requests with a given state (`opened`, `closed`, or `merged`) or all of them (`all`). -The pagination parameters `page` and `per_page` can be used to restrict the list of merge requests. With GitLab 8.2 the return fields `upvotes` and -`downvotes` are deprecated and always return `0`. +The pagination parameters `page` and `per_page` can be used to restrict the list of merge requests. ``` GET /projects/:id/merge_requests @@ -58,7 +57,7 @@ Parameters: ## Get single MR -Shows information about a single merge request. With GitLab 8.2 the return fields `upvotes` and `downvotes` are deprecated and always return `0`. +Shows information about a single merge request. ``` GET /projects/:id/merge_request/:merge_request_id @@ -141,8 +140,6 @@ Parameters: ## Get single MR changes Shows information about the merge request including its files and changes. -With GitLab 8.2 the return fields `upvotes` and `downvotes` are deprecated and -always return `0`. ``` GET /projects/:id/merge_request/:merge_request_id/changes @@ -213,9 +210,7 @@ Parameters: ## Create MR -Creates a new merge request. With GitLab 8.2 the return fields `upvotes` and ` -downvotes` are deprecated and always return `0`. - +Creates a new merge request. ``` POST /projects/:id/merge_requests ``` @@ -266,8 +261,7 @@ If an error occurs, an error number and a message explaining the reason is retur ## Update MR -Updates an existing merge request. You can change the target branch, title, or even close the MR. With GitLab 8.2 the return fields `upvotes` and `downvotes` -are deprecated and always return `0`. +Updates an existing merge request. You can change the target branch, title, or even close the MR. ``` PUT /projects/:id/merge_request/:merge_request_id @@ -318,8 +312,7 @@ If an error occurs, an error number and a message explaining the reason is retur ## Accept MR -Merge changes submitted with MR using this API. With GitLab 8.2 the return -fields `upvotes` and `downvotes` are deprecated and always return `0`. +Merge changes submitted with MR using this API. If merge success you get `200 OK`. diff --git a/doc/api/notes.md b/doc/api/notes.md index 4d7ef288df8..d4d63e825ab 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -6,8 +6,7 @@ Notes are comments on snippets, issues or merge requests. ### List project issue notes -Gets a list of all notes for a single issue. With GitLab 8.2 the return fields -`upvote` and `downvote` are deprecated and always return `false`. +Gets a list of all notes for a single issue. ``` GET /projects/:id/issues/:issue_id/notes diff --git a/doc/integration/README.md b/doc/integration/README.md index 6263353851f..2a9f76533b7 100644 --- a/doc/integration/README.md +++ b/doc/integration/README.md @@ -13,6 +13,7 @@ See the documentation below for details on how to configure these services. - [Slack](slack.md) Integrate with the Slack chat service - [OAuth2 provider](oauth_provider.md) OAuth2 application creation - [Gmail actions buttons](gmail_action_buttons_for_gitlab.md) Adds GitLab actions to messages +- [reCAPTCHA](recaptcha.md) Configure GitLab to use Google reCAPTCHA for new users GitLab Enterprise Edition contains [advanced JIRA support](http://doc.gitlab.com/ee/integration/jira.html) and [advanced Jenkins support](http://doc.gitlab.com/ee/integration/jenkins.html). diff --git a/doc/integration/recaptcha.md b/doc/integration/recaptcha.md new file mode 100644 index 00000000000..a301d1a613c --- /dev/null +++ b/doc/integration/recaptcha.md @@ -0,0 +1,23 @@ +# reCAPTCHA + +GitLab leverages [Google's reCAPTCHA](https://www.google.com/recaptcha/intro/index.html) +to protect against spam and abuse. GitLab displays the CAPTCHA form on the sign-up page +to confirm that a real user, not a bot, is attempting to create an account. + +## Configuration + +To use reCAPTCHA, first you must create a site and private key. + +1. Go to the URL: https://www.google.com/recaptcha/admin + +2. Fill out the form necessary to obtain reCAPTCHA keys. + +3. Login to your GitLab server, with administrator credentials. + +4. Go to Applications Settings on Admin Area (`admin/application_settings`) + +5. Fill all recaptcha fields with keys from previous steps + +6. Check the `Enable reCAPTCHA` checkbox + +7. Save the configuration. diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md index bcd00cfc6bf..1be78ac1823 100644 --- a/doc/permissions/permissions.md +++ b/doc/permissions/permissions.md @@ -6,11 +6,11 @@ If a user is both in a project group and in the project itself, the highest perm If a user is a GitLab administrator they receive all permissions. -On public projects the Guest role is not enforced. -All users will be able to create issues, leave comments, and pull or download the project code. +On public projects the Guest role is not enforced. +All users will be able to create issues, leave comments, and pull or download the project code. To add or import a user, you can follow the [project users and members -documentation](doc/workflow/add-user/add-user.md). +documentation](../workflow/add-user/add-user.md). ## Project diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f8511ac5f5c..26e7c956e8f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -166,7 +166,6 @@ module API class MergeRequest < ProjectEntity expose :target_branch, :source_branch - # deprecated, always returns 0 expose :upvotes, :downvotes expose :author, :assignee, using: Entities::UserBasic expose :source_project_id, :target_project_id diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb new file mode 100644 index 00000000000..2d266ccfe9e --- /dev/null +++ b/lib/gitlab/metrics.rb @@ -0,0 +1,104 @@ +module Gitlab + module Metrics + extend Gitlab::CurrentSettings + + RAILS_ROOT = Rails.root.to_s + METRICS_ROOT = Rails.root.join('lib', 'gitlab', 'metrics').to_s + PATH_REGEX = /^#{RAILS_ROOT}\/?/ + + def self.pool_size + current_application_settings[:metrics_pool_size] || 16 + end + + def self.timeout + current_application_settings[:metrics_timeout] || 10 + end + + def self.enabled? + current_application_settings[:metrics_enabled] || false + end + + def self.mri? + RUBY_ENGINE == 'ruby' + end + + def self.method_call_threshold + # This is memoized since this method is called for every instrumented + # method. Loading data from an external cache on every method call slows + # things down too much. + @method_call_threshold ||= + (current_application_settings[:metrics_method_call_threshold] || 10) + end + + def self.pool + @pool + end + + def self.hostname + @hostname + end + + # Returns a relative path and line number based on the last application call + # frame. + def self.last_relative_application_frame + frame = caller_locations.find do |l| + l.path.start_with?(RAILS_ROOT) && !l.path.start_with?(METRICS_ROOT) + end + + if frame + return frame.path.sub(PATH_REGEX, ''), frame.lineno + else + return nil, nil + end + end + + def self.submit_metrics(metrics) + prepared = prepare_metrics(metrics) + + pool.with do |connection| + prepared.each do |metric| + begin + connection.write_points([metric]) + rescue StandardError + end + end + end + end + + def self.prepare_metrics(metrics) + metrics.map do |hash| + new_hash = hash.symbolize_keys + + new_hash[:tags].each do |key, value| + if value.blank? + new_hash[:tags].delete(key) + else + new_hash[:tags][key] = escape_value(value) + end + end + + new_hash + end + end + + def self.escape_value(value) + value.to_s.gsub('=', '\\=') + end + + @hostname = Socket.gethostname + + # When enabled this should be set before being used as the usual pattern + # "@foo ||= bar" is _not_ thread-safe. + if enabled? + @pool = ConnectionPool.new(size: pool_size, timeout: timeout) do + host = current_application_settings[:metrics_host] + user = current_application_settings[:metrics_username] + pw = current_application_settings[:metrics_password] + port = current_application_settings[:metrics_port] + + InfluxDB::Client. + new(udp: { host: host, port: port }, username: user, password: pw) + end + end + end +end diff --git a/lib/gitlab/metrics/delta.rb b/lib/gitlab/metrics/delta.rb new file mode 100644 index 00000000000..bcf28eed84d --- /dev/null +++ b/lib/gitlab/metrics/delta.rb @@ -0,0 +1,32 @@ +module Gitlab + module Metrics + # Class for calculating the difference between two numeric values. + # + # Every call to `compared_with` updates the internal value. This makes it + # possible to use a single Delta instance to calculate the delta over time + # of an ever increasing number. + # + # Example usage: + # + # delta = Delta.new(0) + # + # delta.compared_with(10) # => 10 + # delta.compared_with(15) # => 5 + # delta.compared_with(20) # => 5 + class Delta + def initialize(value = 0) + @value = value + end + + # new_value - The value to compare with as a Numeric. + # + # Returns a new Numeric (depending on the type of `new_value`). + def compared_with(new_value) + delta = new_value - @value + @value = new_value + + delta + end + end + end +end diff --git a/lib/gitlab/metrics/instrumentation.rb b/lib/gitlab/metrics/instrumentation.rb new file mode 100644 index 00000000000..06fc2f25948 --- /dev/null +++ b/lib/gitlab/metrics/instrumentation.rb @@ -0,0 +1,146 @@ +module Gitlab + module Metrics + # Module for instrumenting methods. + # + # This module allows instrumenting of methods without having to actually + # alter the target code (e.g. by including modules). + # + # Example usage: + # + # Gitlab::Metrics::Instrumentation.instrument_method(User, :by_login) + module Instrumentation + SERIES = 'method_calls' + + def self.configure + yield self + end + + # Instruments a class method. + # + # mod - The module to instrument as a Module/Class. + # name - The name of the method to instrument. + def self.instrument_method(mod, name) + instrument(:class, mod, name) + end + + # Instruments an instance method. + # + # mod - The module to instrument as a Module/Class. + # name - The name of the method to instrument. + def self.instrument_instance_method(mod, name) + instrument(:instance, mod, name) + end + + # Recursively instruments all subclasses of the given root module. + # + # This can be used to for example instrument all ActiveRecord models (as + # these all inherit from ActiveRecord::Base). + # + # This method can optionally take a block to pass to `instrument_methods` + # and `instrument_instance_methods`. + # + # root - The root module for which to instrument subclasses. The root + # module itself is not instrumented. + def self.instrument_class_hierarchy(root, &block) + visit = root.subclasses + + until visit.empty? + klass = visit.pop + + instrument_methods(klass, &block) + instrument_instance_methods(klass, &block) + + klass.subclasses.each { |c| visit << c } + end + end + + # Instruments all public methods of a module. + # + # This method optionally takes a block that can be used to determine if a + # method should be instrumented or not. The block is passed the receiving + # module and an UnboundMethod. If the block returns a non truthy value the + # method is not instrumented. + # + # mod - The module to instrument. + def self.instrument_methods(mod) + mod.public_methods(false).each do |name| + method = mod.method(name) + + if method.owner == mod.singleton_class + if !block_given? || block_given? && yield(mod, method) + instrument_method(mod, name) + end + end + end + end + + # Instruments all public instance methods of a module. + # + # See `instrument_methods` for more information. + # + # mod - The module to instrument. + def self.instrument_instance_methods(mod) + mod.public_instance_methods(false).each do |name| + method = mod.instance_method(name) + + if method.owner == mod + if !block_given? || block_given? && yield(mod, method) + instrument_instance_method(mod, name) + end + end + end + end + + # Instruments a method. + # + # type - The type (:class or :instance) of method to instrument. + # mod - The module containing the method. + # name - The name of the method to instrument. + def self.instrument(type, mod, name) + return unless Metrics.enabled? + + name = name.to_sym + alias_name = :"_original_#{name}" + target = type == :instance ? mod : mod.singleton_class + + if type == :instance + target = mod + label = "#{mod.name}##{name}" + else + target = mod.singleton_class + label = "#{mod.name}.#{name}" + end + + target.class_eval <<-EOF, __FILE__, __LINE__ + 1 + alias_method #{alias_name.inspect}, #{name.inspect} + + def #{name}(*args, &block) + trans = Gitlab::Metrics::Instrumentation.transaction + + if trans + start = Time.now + retval = __send__(#{alias_name.inspect}, *args, &block) + duration = (Time.now - start) * 1000.0 + + if duration >= Gitlab::Metrics.method_call_threshold + trans.add_metric(Gitlab::Metrics::Instrumentation::SERIES, + { duration: duration }, + method: #{label.inspect}) + end + + retval + else + __send__(#{alias_name.inspect}, *args, &block) + end + end + EOF + end + + # Small layer of indirection to make it easier to stub out the current + # transaction. + def self.transaction + Transaction.current + end + end + end +end diff --git a/lib/gitlab/metrics/metric.rb b/lib/gitlab/metrics/metric.rb new file mode 100644 index 00000000000..79241f56874 --- /dev/null +++ b/lib/gitlab/metrics/metric.rb @@ -0,0 +1,34 @@ +module Gitlab + module Metrics + # Class for storing details of a single metric (label, value, etc). + class Metric + attr_reader :series, :values, :tags, :created_at + + # series - The name of the series (as a String) to store the metric in. + # values - A Hash containing the values to store. + # tags - A Hash containing extra tags to add to the metrics. + def initialize(series, values, tags = {}) + @values = values + @series = series + @tags = tags + @created_at = Time.now.utc + end + + # Returns a Hash in a format that can be directly written to InfluxDB. + def to_hash + { + series: @series, + tags: @tags.merge( + hostname: Metrics.hostname, + ruby_engine: RUBY_ENGINE, + ruby_version: RUBY_VERSION, + gitlab_version: Gitlab::VERSION, + process_type: Sidekiq.server? ? 'sidekiq' : 'rails' + ), + values: @values, + timestamp: @created_at.to_i * 1_000_000_000 + } + end + end + end +end diff --git a/lib/gitlab/metrics/obfuscated_sql.rb b/lib/gitlab/metrics/obfuscated_sql.rb new file mode 100644 index 00000000000..fe97d7a0534 --- /dev/null +++ b/lib/gitlab/metrics/obfuscated_sql.rb @@ -0,0 +1,47 @@ +module Gitlab + module Metrics + # Class for producing SQL queries with sensitive data stripped out. + class ObfuscatedSQL + REPLACEMENT = / + \d+(\.\d+)? # integers, floats + | '.+?' # single quoted strings + | \/.+?(?<!\\)\/ # regexps (including escaped slashes) + /x + + MYSQL_REPLACEMENTS = / + ".+?" # double quoted strings + /x + + # Regex to replace consecutive placeholders with a single one indicating + # the length. This can be useful when a "IN" statement uses thousands of + # IDs (storing this would just be a waste of space). + CONSECUTIVE = /(\?(\s*,\s*)?){2,}/ + + # sql - The raw SQL query as a String. + def initialize(sql) + @sql = sql + end + + # Returns a new, obfuscated SQL query. + def to_s + regex = REPLACEMENT + + if Gitlab::Database.mysql? + regex = Regexp.union(regex, MYSQL_REPLACEMENTS) + end + + sql = @sql.gsub(regex, '?').gsub(CONSECUTIVE) do |match| + "#{match.count(',') + 1} values" + end + + # InfluxDB escapes double quotes upon output, so lets get rid of them + # whenever we can. + if Gitlab::Database.postgresql? + sql = sql.delete('"') + end + + sql.tr("\n", ' ') + end + end + end +end diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb new file mode 100644 index 00000000000..5c0587c4c51 --- /dev/null +++ b/lib/gitlab/metrics/rack_middleware.rb @@ -0,0 +1,49 @@ +module Gitlab + module Metrics + # Rack middleware for tracking Rails requests. + class RackMiddleware + CONTROLLER_KEY = 'action_controller.instance' + + def initialize(app) + @app = app + end + + # env - A Hash containing Rack environment details. + def call(env) + trans = transaction_from_env(env) + retval = nil + + begin + retval = trans.run { @app.call(env) } + + # Even in the event of an error we want to submit any metrics we + # might've gathered up to this point. + ensure + if env[CONTROLLER_KEY] + tag_controller(trans, env) + end + + trans.finish + end + + retval + end + + def transaction_from_env(env) + trans = Transaction.new + + trans.add_tag(:request_method, env['REQUEST_METHOD']) + trans.add_tag(:request_uri, env['REQUEST_URI']) + + trans + end + + def tag_controller(trans, env) + controller = env[CONTROLLER_KEY] + label = "#{controller.class.name}##{controller.action_name}" + + trans.add_tag(:action, label) + end + end + end +end diff --git a/lib/gitlab/metrics/sampler.rb b/lib/gitlab/metrics/sampler.rb new file mode 100644 index 00000000000..998578e1c0a --- /dev/null +++ b/lib/gitlab/metrics/sampler.rb @@ -0,0 +1,98 @@ +module Gitlab + module Metrics + # Class that sends certain metrics to InfluxDB at a specific interval. + # + # This class is used to gather statistics that can't be directly associated + # with a transaction such as system memory usage, garbage collection + # statistics, etc. + class Sampler + # interval - The sampling interval in seconds. + def initialize(interval = 15) + @interval = interval + @metrics = [] + + @last_minor_gc = Delta.new(GC.stat[:minor_gc_count]) + @last_major_gc = Delta.new(GC.stat[:major_gc_count]) + + if Gitlab::Metrics.mri? + require 'allocations' + + Allocations.start + end + end + + def start + Thread.new do + Thread.current.abort_on_exception = true + + loop do + sleep(@interval) + + sample + end + end + end + + def sample + sample_memory_usage + sample_file_descriptors + sample_objects + sample_gc + + flush + ensure + GC::Profiler.clear + @metrics.clear + end + + def flush + Metrics.submit_metrics(@metrics.map(&:to_hash)) + end + + def sample_memory_usage + @metrics << Metric.new('memory_usage', value: System.memory_usage) + end + + def sample_file_descriptors + @metrics << Metric. + new('file_descriptors', value: System.file_descriptor_count) + end + + if Metrics.mri? + def sample_objects + sample = Allocations.to_hash + counts = sample.each_with_object({}) do |(klass, count), hash| + hash[klass.name] = count + end + + # Symbols aren't allocated so we'll need to add those manually. + counts['Symbol'] = Symbol.all_symbols.length + + counts.each do |name, count| + @metrics << Metric.new('object_counts', { count: count }, type: name) + end + end + else + def sample_objects + end + end + + def sample_gc + time = GC::Profiler.total_time * 1000.0 + stats = GC.stat.merge(total_time: time) + + # We want the difference of GC runs compared to the last sample, not the + # total amount since the process started. + stats[:minor_gc_count] = + @last_minor_gc.compared_with(stats[:minor_gc_count]) + + stats[:major_gc_count] = + @last_major_gc.compared_with(stats[:major_gc_count]) + + stats[:count] = stats[:minor_gc_count] + stats[:major_gc_count] + + @metrics << Metric.new('gc_statistics', stats) + end + end + end +end diff --git a/lib/gitlab/metrics/sidekiq_middleware.rb b/lib/gitlab/metrics/sidekiq_middleware.rb new file mode 100644 index 00000000000..ad441decfa2 --- /dev/null +++ b/lib/gitlab/metrics/sidekiq_middleware.rb @@ -0,0 +1,23 @@ +module Gitlab + module Metrics + # Sidekiq middleware for tracking jobs. + # + # This middleware is intended to be used as a server-side middleware. + class SidekiqMiddleware + def call(worker, message, queue) + trans = Transaction.new + + begin + trans.run { yield } + ensure + tag_worker(trans, worker) + trans.finish + end + end + + def tag_worker(trans, worker) + trans.add_tag(:action, "#{worker.class.name}#perform") + end + end + end +end diff --git a/lib/gitlab/metrics/subscribers/action_view.rb b/lib/gitlab/metrics/subscribers/action_view.rb new file mode 100644 index 00000000000..7e0dcf99d92 --- /dev/null +++ b/lib/gitlab/metrics/subscribers/action_view.rb @@ -0,0 +1,53 @@ +module Gitlab + module Metrics + module Subscribers + # Class for tracking the rendering timings of views. + class ActionView < ActiveSupport::Subscriber + attach_to :action_view + + SERIES = 'views' + + def render_template(event) + track(event) if current_transaction + end + + alias_method :render_view, :render_template + + private + + def track(event) + values = values_for(event) + tags = tags_for(event) + + current_transaction.add_metric(SERIES, values, tags) + end + + def relative_path(path) + path.gsub(/^#{Rails.root.to_s}\/?/, '') + end + + def values_for(event) + { duration: event.duration } + end + + def tags_for(event) + path = relative_path(event.payload[:identifier]) + tags = { view: path } + + file, line = Metrics.last_relative_application_frame + + if file and line + tags[:file] = file + tags[:line] = line + end + + tags + end + + def current_transaction + Transaction.current + end + end + end + end +end diff --git a/lib/gitlab/metrics/subscribers/active_record.rb b/lib/gitlab/metrics/subscribers/active_record.rb new file mode 100644 index 00000000000..d947c128ce2 --- /dev/null +++ b/lib/gitlab/metrics/subscribers/active_record.rb @@ -0,0 +1,48 @@ +module Gitlab + module Metrics + module Subscribers + # Class for tracking raw SQL queries. + # + # Queries are obfuscated before being logged to ensure no private data is + # exposed via InfluxDB/Grafana. + class ActiveRecord < ActiveSupport::Subscriber + attach_to :active_record + + SERIES = 'sql_queries' + + def sql(event) + return unless current_transaction + + values = values_for(event) + tags = tags_for(event) + + current_transaction.add_metric(SERIES, values, tags) + end + + private + + def values_for(event) + { duration: event.duration } + end + + def tags_for(event) + sql = ObfuscatedSQL.new(event.payload[:sql]).to_s + tags = { sql: sql } + + file, line = Metrics.last_relative_application_frame + + if file and line + tags[:file] = file + tags[:line] = line + end + + tags + end + + def current_transaction + Transaction.current + end + end + end + end +end diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb new file mode 100644 index 00000000000..83371265278 --- /dev/null +++ b/lib/gitlab/metrics/system.rb @@ -0,0 +1,35 @@ +module Gitlab + module Metrics + # Module for gathering system/process statistics such as the memory usage. + # + # This module relies on the /proc filesystem being available. If /proc is + # not available the methods of this module will be stubbed. + module System + if File.exist?('/proc') + # Returns the current process' memory usage in bytes. + def self.memory_usage + mem = 0 + match = File.read('/proc/self/status').match(/VmRSS:\s+(\d+)/) + + if match and match[1] + mem = match[1].to_f * 1024 + end + + mem + end + + def self.file_descriptor_count + Dir.glob('/proc/self/fd/*').length + end + else + def self.memory_usage + 0.0 + end + + def self.file_descriptor_count + 0 + end + end + end + end +end diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb new file mode 100644 index 00000000000..a61dbd989e7 --- /dev/null +++ b/lib/gitlab/metrics/transaction.rb @@ -0,0 +1,66 @@ +module Gitlab + module Metrics + # Class for storing metrics information of a single transaction. + class Transaction + THREAD_KEY = :_gitlab_metrics_transaction + + SERIES = 'transactions' + + attr_reader :uuid, :tags + + def self.current + Thread.current[THREAD_KEY] + end + + # name - The name of this transaction as a String. + def initialize + @metrics = [] + @uuid = SecureRandom.uuid + + @started_at = nil + @finished_at = nil + + @tags = {} + end + + def duration + @finished_at ? (@finished_at - @started_at) * 1000.0 : 0.0 + end + + def run + Thread.current[THREAD_KEY] = self + + @started_at = Time.now + + yield + ensure + @finished_at = Time.now + + Thread.current[THREAD_KEY] = nil + end + + def add_metric(series, values, tags = {}) + tags = tags.merge(transaction_id: @uuid) + + @metrics << Metric.new(series, values, tags) + end + + def add_tag(key, value) + @tags[key] = value + end + + def finish + track_self + submit + end + + def track_self + add_metric(SERIES, { duration: duration }, @tags) + end + + def submit + Metrics.submit_metrics(@metrics.map(&:to_hash)) + end + end + end +end diff --git a/lib/gitlab/recaptcha.rb b/lib/gitlab/recaptcha.rb new file mode 100644 index 00000000000..70e7f25d518 --- /dev/null +++ b/lib/gitlab/recaptcha.rb @@ -0,0 +1,14 @@ +module Gitlab + module Recaptcha + def self.load_configurations! + if current_application_settings.recaptcha_enabled + ::Recaptcha.configure do |config| + config.public_key = current_application_settings.recaptcha_site_key + config.private_key = current_application_settings.recaptcha_private_key + end + + true + end + end + end +end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index c90133fbf03..d15100fc6d8 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' module Ci describe GitlabCiYamlProcessor, lib: true do let(:path) { 'path' } - + describe "#builds_for_ref" do let(:type) { 'test' } @@ -29,7 +29,7 @@ module Ci when: "on_success" }) end - + describe :only do it "does not return builds if only has another branch" do config = YAML.dump({ @@ -517,7 +517,7 @@ module Ci end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra") end - it "returns errors if there is no any jobs defined" do + it "returns errors if there are no jobs defined" do config = YAML.dump({ before_script: ["bundle update"] }) expect do GitlabCiYamlProcessor.new(config, path) diff --git a/spec/lib/gitlab/metrics/delta_spec.rb b/spec/lib/gitlab/metrics/delta_spec.rb new file mode 100644 index 00000000000..718387cdee1 --- /dev/null +++ b/spec/lib/gitlab/metrics/delta_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Delta do + let(:delta) { described_class.new } + + describe '#compared_with' do + it 'returns the delta as a Numeric' do + expect(delta.compared_with(5)).to eq(5) + end + + it 'bases the delta on a previously used value' do + expect(delta.compared_with(5)).to eq(5) + expect(delta.compared_with(15)).to eq(10) + end + end +end diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb new file mode 100644 index 00000000000..a7eab9d11cc --- /dev/null +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -0,0 +1,234 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Instrumentation do + let(:transaction) { Gitlab::Metrics::Transaction.new } + + before do + @dummy = Class.new do + def self.foo(text = 'foo') + text + end + + def bar(text = 'bar') + text + end + end + + allow(@dummy).to receive(:name).and_return('Dummy') + end + + describe '.configure' do + it 'yields self' do + described_class.configure do |c| + expect(c).to eq(described_class) + end + end + end + + describe '.instrument_method' do + describe 'with metrics enabled' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(true) + + described_class.instrument_method(@dummy, :foo) + end + + it 'renames the original method' do + expect(@dummy).to respond_to(:_original_foo) + end + + it 'calls the instrumented method with the correct arguments' do + expect(@dummy.foo).to eq('foo') + end + + it 'tracks the call duration upon calling the method' do + allow(Gitlab::Metrics).to receive(:method_call_threshold). + and_return(0) + + allow(described_class).to receive(:transaction). + and_return(transaction) + + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, an_instance_of(Hash), + method: 'Dummy.foo') + + @dummy.foo + end + + it 'does not track method calls below a given duration threshold' do + allow(Gitlab::Metrics).to receive(:method_call_threshold). + and_return(100) + + expect(transaction).to_not receive(:add_metric) + + @dummy.foo + end + end + + describe 'with metrics disabled' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(false) + end + + it 'does not instrument the method' do + described_class.instrument_method(@dummy, :foo) + + expect(@dummy).to_not respond_to(:_original_foo) + end + end + end + + describe '.instrument_instance_method' do + describe 'with metrics enabled' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(true) + + described_class. + instrument_instance_method(@dummy, :bar) + end + + it 'renames the original method' do + expect(@dummy.method_defined?(:_original_bar)).to eq(true) + end + + it 'calls the instrumented method with the correct arguments' do + expect(@dummy.new.bar).to eq('bar') + end + + it 'tracks the call duration upon calling the method' do + allow(Gitlab::Metrics).to receive(:method_call_threshold). + and_return(0) + + allow(described_class).to receive(:transaction). + and_return(transaction) + + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, an_instance_of(Hash), + method: 'Dummy#bar') + + @dummy.new.bar + end + + it 'does not track method calls below a given duration threshold' do + allow(Gitlab::Metrics).to receive(:method_call_threshold). + and_return(100) + + expect(transaction).to_not receive(:add_metric) + + @dummy.new.bar + end + end + + describe 'with metrics disabled' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(false) + end + + it 'does not instrument the method' do + described_class. + instrument_instance_method(@dummy, :bar) + + expect(@dummy.method_defined?(:_original_bar)).to eq(false) + end + end + end + + describe '.instrument_class_hierarchy' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(true) + + @child1 = Class.new(@dummy) do + def self.child1_foo; end + def child1_bar; end + end + + @child2 = Class.new(@child1) do + def self.child2_foo; end + def child2_bar; end + end + end + + it 'recursively instruments a class hierarchy' do + described_class.instrument_class_hierarchy(@dummy) + + expect(@child1).to respond_to(:_original_child1_foo) + expect(@child2).to respond_to(:_original_child2_foo) + + expect(@child1.method_defined?(:_original_child1_bar)).to eq(true) + expect(@child2.method_defined?(:_original_child2_bar)).to eq(true) + end + + it 'does not instrument the root module' do + described_class.instrument_class_hierarchy(@dummy) + + expect(@dummy).to_not respond_to(:_original_foo) + expect(@dummy.method_defined?(:_original_bar)).to eq(false) + end + end + + describe '.instrument_methods' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(true) + end + + it 'instruments all public class methods' do + described_class.instrument_methods(@dummy) + + expect(@dummy).to respond_to(:_original_foo) + end + + it 'only instruments methods directly defined in the module' do + mod = Module.new do + def kittens + end + end + + @dummy.extend(mod) + + described_class.instrument_methods(@dummy) + + expect(@dummy).to_not respond_to(:_original_kittens) + end + + it 'can take a block to determine if a method should be instrumented' do + described_class.instrument_methods(@dummy) do + false + end + + expect(@dummy).to_not respond_to(:_original_foo) + end + end + + describe '.instrument_instance_methods' do + before do + allow(Gitlab::Metrics).to receive(:enabled?).and_return(true) + end + + it 'instruments all public instance methods' do + described_class.instrument_instance_methods(@dummy) + + expect(@dummy.method_defined?(:_original_bar)).to eq(true) + end + + it 'only instruments methods directly defined in the module' do + mod = Module.new do + def kittens + end + end + + @dummy.include(mod) + + described_class.instrument_instance_methods(@dummy) + + expect(@dummy.method_defined?(:_original_kittens)).to eq(false) + end + + it 'can take a block to determine if a method should be instrumented' do + described_class.instrument_instance_methods(@dummy) do + false + end + + expect(@dummy.method_defined?(:_original_bar)).to eq(false) + end + end +end diff --git a/spec/lib/gitlab/metrics/metric_spec.rb b/spec/lib/gitlab/metrics/metric_spec.rb new file mode 100644 index 00000000000..ec39bc9cce8 --- /dev/null +++ b/spec/lib/gitlab/metrics/metric_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Metric do + let(:metric) do + described_class.new('foo', { number: 10 }, { host: 'localtoast' }) + end + + describe '#series' do + subject { metric.series } + + it { is_expected.to eq('foo') } + end + + describe '#values' do + subject { metric.values } + + it { is_expected.to eq({ number: 10 }) } + end + + describe '#tags' do + subject { metric.tags } + + it { is_expected.to eq({ host: 'localtoast' }) } + end + + describe '#to_hash' do + it 'returns a Hash' do + expect(metric.to_hash).to be_an_instance_of(Hash) + end + + describe 'the returned Hash' do + let(:hash) { metric.to_hash } + + it 'includes the series' do + expect(hash[:series]).to eq('foo') + end + + it 'includes the tags' do + expect(hash[:tags]).to be_an_instance_of(Hash) + + expect(hash[:tags][:hostname]).to be_an_instance_of(String) + expect(hash[:tags][:ruby_engine]).to be_an_instance_of(String) + expect(hash[:tags][:ruby_version]).to be_an_instance_of(String) + expect(hash[:tags][:gitlab_version]).to be_an_instance_of(String) + expect(hash[:tags][:process_type]).to be_an_instance_of(String) + end + + it 'includes the values' do + expect(hash[:values]).to eq({ number: 10 }) + end + + it 'includes the timestamp' do + expect(hash[:timestamp]).to be_an_instance_of(Fixnum) + end + end + end +end diff --git a/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb b/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb new file mode 100644 index 00000000000..2b681c9fe34 --- /dev/null +++ b/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +describe Gitlab::Metrics::ObfuscatedSQL do + describe '#to_s' do + it 'replaces newlines with a space' do + sql = described_class.new("SELECT x\nFROM y") + + expect(sql.to_s).to eq('SELECT x FROM y') + end + + describe 'using single values' do + it 'replaces a single integer' do + sql = described_class.new('SELECT x FROM y WHERE a = 10') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') + end + + it 'replaces a single float' do + sql = described_class.new('SELECT x FROM y WHERE a = 10.5') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') + end + + it 'replaces a single quoted string' do + sql = described_class.new("SELECT x FROM y WHERE a = 'foo'") + + expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') + end + + if Gitlab::Database.mysql? + it 'replaces a double quoted string' do + sql = described_class.new('SELECT x FROM y WHERE a = "foo"') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') + end + end + + it 'replaces a single regular expression' do + sql = described_class.new('SELECT x FROM y WHERE a = /foo/') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') + end + + it 'replaces regular expressions using escaped slashes' do + sql = described_class.new('SELECT x FROM y WHERE a = /foo\/bar/') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') + end + end + + describe 'using consecutive values' do + it 'replaces multiple integers' do + sql = described_class.new('SELECT x FROM y WHERE z IN (10, 20, 30)') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (3 values)') + end + + it 'replaces multiple floats' do + sql = described_class.new('SELECT x FROM y WHERE z IN (1.5, 2.5, 3.5)') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (3 values)') + end + + it 'replaces multiple single quoted strings' do + sql = described_class.new("SELECT x FROM y WHERE z IN ('foo', 'bar')") + + expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (2 values)') + end + + if Gitlab::Database.mysql? + it 'replaces multiple double quoted strings' do + sql = described_class.new('SELECT x FROM y WHERE z IN ("foo", "bar")') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (2 values)') + end + end + + it 'replaces multiple regular expressions' do + sql = described_class.new('SELECT x FROM y WHERE z IN (/foo/, /bar/)') + + expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (2 values)') + end + end + + if Gitlab::Database.postgresql? + it 'replaces double quotes' do + sql = described_class.new('SELECT "x" FROM "y"') + + expect(sql.to_s).to eq('SELECT x FROM y') + end + end + end +end diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb new file mode 100644 index 00000000000..a143fe4cfcd --- /dev/null +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe Gitlab::Metrics::RackMiddleware do + let(:app) { double(:app) } + + let(:middleware) { described_class.new(app) } + + let(:env) { { 'REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/foo' } } + + describe '#call' do + before do + expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) + end + + it 'tracks a transaction' do + expect(app).to receive(:call).with(env).and_return('yay') + + expect(middleware.call(env)).to eq('yay') + end + + it 'tags a transaction with the name and action of a controller' do + klass = double(:klass, name: 'TestController') + controller = double(:controller, class: klass, action_name: 'show') + + env['action_controller.instance'] = controller + + allow(app).to receive(:call).with(env) + + expect(middleware).to receive(:tag_controller). + with(an_instance_of(Gitlab::Metrics::Transaction), env) + + middleware.call(env) + end + end + + describe '#transaction_from_env' do + let(:transaction) { middleware.transaction_from_env(env) } + + it 'returns a Transaction' do + expect(transaction).to be_an_instance_of(Gitlab::Metrics::Transaction) + end + + it 'tags the transaction with the request method and URI' do + expect(transaction.tags[:request_method]).to eq('GET') + expect(transaction.tags[:request_uri]).to eq('/foo') + end + end + + describe '#tag_controller' do + let(:transaction) { middleware.transaction_from_env(env) } + + it 'tags a transaction with the name and action of a controller' do + klass = double(:klass, name: 'TestController') + controller = double(:controller, class: klass, action_name: 'show') + + env['action_controller.instance'] = controller + + middleware.tag_controller(transaction, env) + + expect(transaction.tags[:action]).to eq('TestController#show') + end + end +end diff --git a/spec/lib/gitlab/metrics/sampler_spec.rb b/spec/lib/gitlab/metrics/sampler_spec.rb new file mode 100644 index 00000000000..51a941c48cd --- /dev/null +++ b/spec/lib/gitlab/metrics/sampler_spec.rb @@ -0,0 +1,97 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Sampler do + let(:sampler) { described_class.new(5) } + + after do + Allocations.stop if Gitlab::Metrics.mri? + end + + describe '#start' do + it 'gathers a sample at a given interval' do + expect(sampler).to receive(:sleep).with(5) + expect(sampler).to receive(:sample) + expect(sampler).to receive(:loop).and_yield + + sampler.start.join + end + end + + describe '#sample' do + it 'samples various statistics' do + expect(sampler).to receive(:sample_memory_usage) + expect(sampler).to receive(:sample_file_descriptors) + expect(sampler).to receive(:sample_objects) + expect(sampler).to receive(:sample_gc) + expect(sampler).to receive(:flush) + + sampler.sample + end + + it 'clears any GC profiles' do + expect(sampler).to receive(:flush) + expect(GC::Profiler).to receive(:clear) + + sampler.sample + end + end + + describe '#flush' do + it 'schedules the metrics using Sidekiq' do + expect(Gitlab::Metrics).to receive(:submit_metrics). + with([an_instance_of(Hash)]) + + sampler.sample_memory_usage + sampler.flush + end + end + + describe '#sample_memory_usage' do + it 'adds a metric containing the memory usage' do + expect(Gitlab::Metrics::System).to receive(:memory_usage). + and_return(9000) + + expect(Gitlab::Metrics::Metric).to receive(:new). + with('memory_usage', value: 9000). + and_call_original + + sampler.sample_memory_usage + end + end + + describe '#sample_file_descriptors' do + it 'adds a metric containing the amount of open file descriptors' do + expect(Gitlab::Metrics::System).to receive(:file_descriptor_count). + and_return(4) + + expect(Gitlab::Metrics::Metric).to receive(:new). + with('file_descriptors', value: 4). + and_call_original + + sampler.sample_file_descriptors + end + end + + describe '#sample_objects' do + it 'adds a metric containing the amount of allocated objects' do + expect(Gitlab::Metrics::Metric).to receive(:new). + with('object_counts', an_instance_of(Hash), an_instance_of(Hash)). + at_least(:once). + and_call_original + + sampler.sample_objects + end + end + + describe '#sample_gc' do + it 'adds a metric containing garbage collection statistics' do + expect(GC::Profiler).to receive(:total_time).and_return(0.24) + + expect(Gitlab::Metrics::Metric).to receive(:new). + with('gc_statistics', an_instance_of(Hash)). + and_call_original + + sampler.sample_gc + end + end +end diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb new file mode 100644 index 00000000000..5882e7d81c7 --- /dev/null +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Gitlab::Metrics::SidekiqMiddleware do + let(:middleware) { described_class.new } + + describe '#call' do + it 'tracks the transaction' do + worker = Class.new.new + + expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) + + middleware.call(worker, 'test', :test) { nil } + end + end + + describe '#tag_worker' do + it 'adds the worker class and action to the transaction' do + trans = Gitlab::Metrics::Transaction.new + worker = double(:worker, class: double(:class, name: 'TestWorker')) + + expect(trans).to receive(:add_tag).with(:action, 'TestWorker#perform') + + middleware.tag_worker(trans, worker) + end + end +end diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb new file mode 100644 index 00000000000..c6cd584663f --- /dev/null +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Subscribers::ActionView do + let(:transaction) { Gitlab::Metrics::Transaction.new } + + let(:subscriber) { described_class.new } + + let(:event) do + root = Rails.root.to_s + + double(:event, duration: 2.1, + payload: { identifier: "#{root}/app/views/x.html.haml" }) + end + + before do + allow(subscriber).to receive(:current_transaction).and_return(transaction) + + allow(Gitlab::Metrics).to receive(:last_relative_application_frame). + and_return(['app/views/x.html.haml', 4]) + end + + describe '#render_template' do + it 'tracks rendering of a template' do + values = { duration: 2.1 } + tags = { + view: 'app/views/x.html.haml', + file: 'app/views/x.html.haml', + line: 4 + } + + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, values, tags) + + subscriber.render_template(event) + end + end +end diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb new file mode 100644 index 00000000000..05b6cc14716 --- /dev/null +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Subscribers::ActiveRecord do + let(:transaction) { Gitlab::Metrics::Transaction.new } + + let(:subscriber) { described_class.new } + + let(:event) do + double(:event, duration: 0.2, + payload: { sql: 'SELECT * FROM users WHERE id = 10' }) + end + + before do + allow(subscriber).to receive(:current_transaction).and_return(transaction) + + allow(Gitlab::Metrics).to receive(:last_relative_application_frame). + and_return(['app/models/foo.rb', 4]) + end + + describe '#sql' do + it 'tracks the execution of a SQL query' do + sql = 'SELECT * FROM users WHERE id = ?' + values = { duration: 0.2 } + tags = { sql: sql, file: 'app/models/foo.rb', line: 4 } + + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, values, tags) + + subscriber.sql(event) + end + end +end diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb new file mode 100644 index 00000000000..f8c1d956ca1 --- /dev/null +++ b/spec/lib/gitlab/metrics/system_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::Metrics::System do + if File.exist?('/proc') + describe '.memory_usage' do + it "returns the process' memory usage in bytes" do + expect(described_class.memory_usage).to be > 0 + end + end + + describe '.file_descriptor_count' do + it 'returns the amount of open file descriptors' do + expect(described_class.file_descriptor_count).to be > 0 + end + end + else + describe '.memory_usage' do + it 'returns 0.0' do + expect(described_class.memory_usage).to eq(0.0) + end + end + + describe '.file_descriptor_count' do + it 'returns 0' do + expect(described_class.file_descriptor_count).to eq(0) + end + end + end +end diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb new file mode 100644 index 00000000000..6862fc9e2d1 --- /dev/null +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Transaction do + let(:transaction) { described_class.new } + + describe '#duration' do + it 'returns the duration of a transaction in seconds' do + transaction.run { sleep(0.5) } + + expect(transaction.duration).to be >= 0.5 + end + end + + describe '#run' do + it 'yields the supplied block' do + expect { |b| transaction.run(&b) }.to yield_control + end + + it 'stores the transaction in the current thread' do + transaction.run do + expect(Thread.current[described_class::THREAD_KEY]).to eq(transaction) + end + end + + it 'removes the transaction from the current thread upon completion' do + transaction.run { } + + expect(Thread.current[described_class::THREAD_KEY]).to be_nil + end + end + + describe '#add_metric' do + it 'adds a metric tagged with the transaction UUID' do + expect(Gitlab::Metrics::Metric).to receive(:new). + with('foo', { number: 10 }, { transaction_id: transaction.uuid }) + + transaction.add_metric('foo', number: 10) + end + end + + describe '#add_tag' do + it 'adds a tag' do + transaction.add_tag(:foo, 'bar') + + expect(transaction.tags).to eq({ foo: 'bar' }) + end + end + + describe '#finish' do + it 'tracks the transaction details and submits them to Sidekiq' do + expect(transaction).to receive(:track_self) + expect(transaction).to receive(:submit) + + transaction.finish + end + end + + describe '#track_self' do + it 'adds a metric for the transaction itself' do + expect(transaction).to receive(:add_metric). + with(described_class::SERIES, { duration: transaction.duration }, {}) + + transaction.track_self + end + end + + describe '#submit' do + it 'submits the metrics to Sidekiq' do + transaction.track_self + + expect(Gitlab::Metrics).to receive(:submit_metrics). + with([an_instance_of(Hash)]) + + transaction.submit + end + end +end diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb new file mode 100644 index 00000000000..6c0682cac4d --- /dev/null +++ b/spec/lib/gitlab/metrics_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe Gitlab::Metrics do + describe '.pool_size' do + it 'returns a Fixnum' do + expect(described_class.pool_size).to be_an_instance_of(Fixnum) + end + end + + describe '.timeout' do + it 'returns a Fixnum' do + expect(described_class.timeout).to be_an_instance_of(Fixnum) + end + end + + describe '.enabled?' do + it 'returns a boolean' do + expect([true, false].include?(described_class.enabled?)).to eq(true) + end + end + + describe '.hostname' do + it 'returns a String containing the hostname' do + expect(described_class.hostname).to eq(Socket.gethostname) + end + end + + describe '.last_relative_application_frame' do + it 'returns an Array containing a file path and line number' do + file, line = described_class.last_relative_application_frame + + expect(line).to eq(__LINE__ - 2) + expect(file).to eq('spec/lib/gitlab/metrics_spec.rb') + end + end + + describe '#submit_metrics' do + it 'prepares and writes the metrics to InfluxDB' do + connection = double(:connection) + pool = double(:pool) + + expect(pool).to receive(:with).and_yield(connection) + expect(connection).to receive(:write_points).with(an_instance_of(Array)) + expect(Gitlab::Metrics).to receive(:pool).and_return(pool) + + described_class.submit_metrics([{ 'series' => 'kittens', 'tags' => {} }]) + end + end + + describe '#prepare_metrics' do + it 'returns a Hash with the keys as Symbols' do + metrics = described_class. + prepare_metrics([{ 'values' => {}, 'tags' => {} }]) + + expect(metrics).to eq([{ values: {}, tags: {} }]) + end + + it 'escapes tag values' do + metrics = described_class.prepare_metrics([ + { 'values' => {}, 'tags' => { 'foo' => 'bar=' } } + ]) + + expect(metrics).to eq([{ values: {}, tags: { 'foo' => 'bar\\=' } }]) + end + + it 'drops empty tags' do + metrics = described_class.prepare_metrics([ + { 'values' => {}, 'tags' => { 'cats' => '', 'dogs' => nil } } + ]) + + expect(metrics).to eq([{ values: {}, tags: {} }]) + end + end + + describe '#escape_value' do + it 'escapes an equals sign' do + expect(described_class.escape_value('foo=')).to eq('foo\\=') + end + + it 'casts values to Strings' do + expect(described_class.escape_value(10)).to eq('10') + end + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index f9d3c56750f..021d62cdf0c 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -99,4 +99,18 @@ describe Issue, "Issuable" do to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' }) end end + + describe "votes" do + before do + author = create :user + project = create :empty_project + issue.notes.awards.create!(note: "thumbsup", author: author, project: project) + issue.notes.awards.create!(note: "thumbsdown", author: author, project: project) + end + + it "returns correct values" do + expect(issue.upvotes).to eq(1) + expect(issue.downvotes).to eq(1) + end + end end |