Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/monitoring/utils/multiple_time_series.js

@@ -92,7 +92,11 @@ function queryTimeSeries(query, graphDrawData, lineStyle) {
     if (seriesCustomizationData) {
       metricTag = seriesCustomizationData.value || timeSeriesMetricLabel;
       [lineColor, areaColor] = pickColor(seriesCustomizationData.color);
+      if (timeSeriesParsed.length > 0) {
         shouldRenderLegend = false;
+      } else {
+        shouldRenderLegend = true;
+      }
     } else {
       metricTag = timeSeriesMetricLabel || query.label || `series ${timeSeriesNumber + 1}`;
       [lineColor, areaColor] = pickColor();
@@ -101,19 +105,6 @@ function queryTimeSeries(query, graphDrawData, lineStyle) {
       }
     }
 
-    if (!shouldRenderLegend) {
-      if (!timeSeriesParsed[0].tracksLegend) {
-        timeSeriesParsed[0].tracksLegend = [];
-      }
-      timeSeriesParsed[0].tracksLegend.push({
-        max: maximumValue,
-        average: accum / timeSeries.values.length,
-        lineStyle,
-        lineColor,
-        metricTag,
-      });
-    }
-
     const values = datesWithoutGaps.map(time => ({
       time,
       value: findByDate(timeSeries.values, time),
@@ -135,6 +126,19 @@ function queryTimeSeries(query, graphDrawData, lineStyle) {
       shouldRenderLegend,
       renderCanary,
     });
+
+    if (!shouldRenderLegend) {
+      if (!timeSeriesParsed[0].tracksLegend) {
+        timeSeriesParsed[0].tracksLegend = [];
+      }
+      timeSeriesParsed[0].tracksLegend.push({
+        max: maximumValue,
+        average: accum / timeSeries.values.length,
+        lineStyle,
+        lineColor,
+        metricTag,
+      });
+    }
   });
 
   return timeSeriesParsed;

app/models/prometheus_metric.rb

@@ -5,11 +5,12 @@ class PrometheusMetric < ActiveRecord::Base
 
   enum group: {
     # built-in groups
-    nginx_ingress: -1,
+    nginx_ingress_vts: -1,
     ha_proxy: -2,
     aws_elb: -3,
     nginx: -4,
     kubernetes: -5,
+    nginx_ingress: -6,
 
     # custom/user groups
     business: 0,
@@ -30,6 +31,7 @@ class PrometheusMetric < ActiveRecord::Base
 
   GROUP_TITLES = {
     # built-in groups
+    nginx_ingress_vts: _('Response metrics (NGINX Ingress VTS)'),
     nginx_ingress: _('Response metrics (NGINX Ingress)'),
     ha_proxy: _('Response metrics (HA Proxy)'),
     aws_elb: _('Response metrics (AWS ELB)'),
@@ -43,7 +45,8 @@ class PrometheusMetric < ActiveRecord::Base
   }.freeze
 
   REQUIRED_METRICS = {
-    nginx_ingress: %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg),
+    nginx_ingress_vts: %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg),
+    nginx_ingress: %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum),
     ha_proxy: %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total),
     aws_elb: %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum),
     nginx: %w(nginx_server_requests nginx_server_requestMsec),

changelogs/unreleased/add-new-nginx-metrics.yml

+---
+title: Add NGINX 0.16.0 and above metrics
+merge_request: 22133
+author:
+type: added

config/prometheus/common_metrics.yml

-- group: Response metrics (NGINX Ingress)
+  # NGINX Ingress metrics for pre-0.16.0 versions
+- group: Response metrics (NGINX Ingress VTS)
   priority: 10
   metrics:
   - title: "Throughput"
@@ -40,6 +41,51 @@
       query_range: 'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) * 100'
       label: 5xx Errors
       unit: "%"
+# NGINX Ingress metrics for post-0.16.0 versions
+- group: Response metrics (NGINX Ingress)
+  priority: 10
+  metrics:
+  - title: "Throughput"
+    y_label: "Requests / Sec"
+    required_metrics:
+      - nginx_ingress_controller_requests
+    weight: 1
+    queries:
+    - id: response_metrics_nginx_ingress_16_throughput_status_code
+      query_range: 'sum(label_replace(rate(nginx_ingress_controller_requests{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m]), "status_code", "${1}xx", "status", "(.)..")) by (status_code)'
+      unit: req / sec
+      label: Status Code
+      series:
+        - label: status_code
+          when:
+            - value: 2xx
+              color: green
+            - value: 3xx
+              color: blue
+            - value: 4xx
+              color: orange
+            - value: 5xx
+              color: red
+  - title: "Latency"
+    y_label: "Latency (ms)"
+    required_metrics:
+      - nginx_ingress_controller_ingress_upstream_latency_seconds_sum
+    weight: 1
+    queries:
+    - id: response_metrics_nginx_ingress_16_latency_pod_average
+      query_range: 'sum(rate(nginx_ingress_controller_ingress_upstream_latency_seconds_sum{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) / sum(rate(nginx_ingress_controller_ingress_upstream_latency_seconds_count{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) * 1000'
+      label: Pod average
+      unit: ms
+  - title: "HTTP Error Rate"
+    y_label: "HTTP Errors"
+    required_metrics:
+      - nginx_ingress_controller_requests
+    weight: 1
+    queries:
+    - id: response_metrics_nginx_ingress_16_http_error_rate
+      query_range: 'sum(rate(nginx_ingress_controller_requests{status=~"5.*",namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) / sum(rate(nginx_ingress_controller_requests{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) * 100'
+      label: 5xx Errors
+      unit: "%"
 - group: Response metrics (HA Proxy)
   priority: 10
   metrics:

db/importers/common_metrics_importer.rb

@@ -4,11 +4,12 @@ module Importers
   class PrometheusMetric < ActiveRecord::Base
     enum group: {
       # built-in groups
-      nginx_ingress: -1,
+      nginx_ingress_vts: -1,
       ha_proxy: -2,
       aws_elb: -3,
       nginx: -4,
       kubernetes: -5,
+      nginx_ingress: -6,
 
       # custom groups
       business: 0,
@@ -22,6 +23,7 @@ module Importers
       business: _('Business metrics (Custom)'),
       response: _('Response metrics (Custom)'),
       system: _('System metrics (Custom)'),
+      nginx_ingress_vts: _('Response metrics (NGINX Ingress VTS)'),
       nginx_ingress: _('Response metrics (NGINX Ingress)'),
       ha_proxy: _('Response metrics (HA Proxy)'),
       aws_elb: _('Response metrics (AWS ELB)'),

db/migrate/20181006004100_import_common_metrics_nginx_vts.rb

+class ImportCommonMetricsNginxVts < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  require Rails.root.join('db/importers/common_metrics_importer.rb')
+
+  DOWNTIME = false
+
+  def up
+    Importers::CommonMetricsImporter.new.execute
+  end
+
+  def down
+    # no-op
+  end
+end

lib/gitlab/safe_request_store.rb

@@ -19,5 +19,13 @@ module Gitlab
         NULL_STORE
       end
     end
+
+    # This method accept an options hash to be compatible with
+    # ActiveSupport::Cache::Store#write method. The options are
+    # not passed to the underlying cache implementation because
+    # RequestStore#write accepts only a key, and value params.
+    def self.write(key, value, options = nil)
+      store.write(key, value)
+    end
   end
 end

locale/gitlab.pot

@@ -7346,6 +7346,9 @@ msgstr ""
 msgid "Response metrics (HA Proxy)"
 msgstr ""
 
+msgid "Response metrics (NGINX Ingress VTS)"
+msgstr ""
+
 msgid "Response metrics (NGINX Ingress)"
 msgstr ""
 

spec/lib/gitlab/safe_request_store_spec.rb

@@ -78,6 +78,12 @@ describe Gitlab::SafeRequestStore do
           described_class.write('foo', true)
         end.to change { described_class.read('foo') }.from(nil).to(true)
       end
+
+      it 'does not pass the options hash to the underlying store implementation' do
+        expect(described_class.store).to receive(:write).with('foo', true)
+
+        described_class.write('foo', true, expires_in: 15.seconds)
+      end
     end
 
     context 'when RequestStore is NOT active' do
@@ -86,6 +92,12 @@ describe Gitlab::SafeRequestStore do
           described_class.write('foo', true)
         end.not_to change { described_class.read('foo') }.from(nil)
       end
+
+      it 'does not pass the options hash to the underlying store implementation' do
+        expect(described_class.store).to receive(:write).with('foo', true)
+
+        described_class.write('foo', true, expires_in: 15.seconds)
+      end
     end
   end