Merge branch 'relocate-influxdb-settings' into 'master'

Move InfluxDB settings to ApplicationSetting

This moves the settings from the YAML files to the database.

cc @sytses

See merge request !2228

app/controllers/admin/application_settings_controller.rb

@@ -67,6 +67,14 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :user_oauth_applications,
       :shared_runners_enabled,
       :max_artifacts_size,
+      :metrics_enabled,
+      :metrics_host,
+      :metrics_database,
+      :metrics_username,
+      :metrics_password,
+      :metrics_pool_size,
+      :metrics_timeout,
+      :metrics_method_call_threshold,
       restricted_visibility_levels: [],
       import_sources: []
     )

app/views/admin/application_settings/_form.html.haml

@@ -156,5 +156,58 @@
       .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_database, 'InfluxDB database', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :metrics_database, class: 'form-control', placeholder: 'gitlab'
+        .help-block
+          The name of the InfluxDB database to store data in. Users will have to
+          create this database manually, GitLab does not do so automatically.
+    .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.
+
   .form-actions
     = f.submit 'Save', class: 'btn btn-primary'

config/gitlab.yml.example

@@ -449,26 +449,9 @@ production: &base
       #
       # Ban an IP for one hour (3600s) after too many auth attempts
       # bantime: 3600
-  metrics:
-    host: localhost
-    enabled: false
-    # The name of the InfluxDB database to store metrics in.
-    database: gitlab
-    # Credentials to use for logging in to InfluxDB.
-    # username:
-    # password:
-    # The amount of InfluxDB connections to open.
-    # pool_size: 16
-    # The timeout of a connection in seconds.
-    # timeout: 10
-    # The minimum amount of milliseconds a method call has to take before it's
-    # tracked. Defaults to 10.
-    # method_call_threshold: 10
 
 development:
   <<: *base
-  metrics:
-    enabled: false
 
 test:
   <<: *base
@@ -511,10 +494,6 @@ test:
         user_filter: ''
         group_base: 'ou=groups,dc=example,dc=com'
         admin_group: ''
-  metrics:
-    enabled: false
 
 staging:
   <<: *base
-  metrics:
-    enabled: false

config/initializers/metrics.rb

@@ -32,12 +32,19 @@ if Gitlab::Metrics.enabled?
     )
 
     Gitlab::Metrics::Instrumentation.
-      instrument_class_hierarchy(ActiveRecord::Base) do |_, method|
+      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)

db/migrate/20151228150906_influxdb_settings.rb

+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

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: 20151228150906) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -52,6 +52,14 @@ ActiveRecord::Schema.define(version: 20151224123230) do
     t.string   "runners_registration_token"
     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_database",                              default: "gitlab"
+    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
   end
 
   create_table "audit_events", force: :cascade do |t|

lib/gitlab/metrics.rb

@@ -4,16 +4,29 @@ module Gitlab
     METRICS_ROOT = Rails.root.join('lib', 'gitlab', 'metrics').to_s
     PATH_REGEX   = /^#{RAILS_ROOT}\/?/
 
+    # Returns the current settings, ensuring we _always_ have a default set of
+    # metrics settings (even during tests, when the migrations are lacking,
+    # etc). This ensures the application is able to boot up even when the
+    # migrations have not been executed.
+    def self.settings
+      ApplicationSetting.current || {
+        metrics_pool_size:             16,
+        metrics_timeout:               10,
+        metrics_enabled:               false,
+        metrics_method_call_threshold: 10
+      }
+    end
+
     def self.pool_size
-      Settings.metrics['pool_size'] || 16
+      settings[:metrics_pool_size]
     end
 
     def self.timeout
-      Settings.metrics['timeout'] || 10
+      settings[:metrics_timeout]
     end
 
     def self.enabled?
-      !!Settings.metrics['enabled']
+      settings[:metrics_enabled]
     end
 
     def self.mri?
@@ -21,7 +34,10 @@ module Gitlab
     end
 
     def self.method_call_threshold
-      Settings.metrics['method_call_threshold'] || 10
+      # 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 ||= settings[:metrics_method_call_threshold]
     end
 
     def self.pool
@@ -52,10 +68,10 @@ module Gitlab
     # "@foo ||= bar" is _not_ thread-safe.
     if enabled?
       @pool = ConnectionPool.new(size: pool_size, timeout: timeout) do
-        host = Settings.metrics['host']
-        db   = Settings.metrics['database']
-        user = Settings.metrics['username']
-        pw   = Settings.metrics['password']
+        host = settings[:metrics_host]
+        db   = settings[:metrics_database]
+        user = settings[:metrics_username]
+        pw   = settings[:metrics_password]
 
         InfluxDB::Client.new(db, host: host, username: user, password: pw)
       end