From 2061414054ce43aa6d53d1be3f602114e5a336d2 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 10 May 2017 11:25:30 +0200 Subject: Additional metrics initial work, with working metrics listing, but without actoual metrics mesurements --- .../projects/environments_controller.rb | 6 ++ app/controllers/projects/prometheus_controller.rb | 22 +++++++ app/models/environment.rb | 8 +++ app/models/project_services/prometheus_service.rb | 20 ++++-- config/additional_metrics.yml | 26 ++++++++ config/routes/project.rb | 5 ++ lib/gitlab/prometheus/metric.rb | 28 +++++++++ lib/gitlab/prometheus/metric_group.rb | 33 ++++++++++ lib/gitlab/prometheus/metrics_sources.rb | 7 +++ lib/gitlab/prometheus/parsing_error.rb | 3 + .../prometheus/queries/additional_metrics_query.rb | 70 +++++++++++++++++++++ lib/gitlab/prometheus/queries/base_query.rb | 2 +- .../prometheus/queries/matched_metrics_query.rb | 72 ++++++++++++++++++++++ lib/gitlab/prometheus_client.rb | 8 +++ 14 files changed, 304 insertions(+), 6 deletions(-) create mode 100644 app/controllers/projects/prometheus_controller.rb create mode 100644 config/additional_metrics.yml create mode 100644 lib/gitlab/prometheus/metric.rb create mode 100644 lib/gitlab/prometheus/metric_group.rb create mode 100644 lib/gitlab/prometheus/metrics_sources.rb create mode 100644 lib/gitlab/prometheus/parsing_error.rb create mode 100644 lib/gitlab/prometheus/queries/additional_metrics_query.rb create mode 100644 lib/gitlab/prometheus/queries/matched_metrics_query.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index efe83776834..6d230e84ef7 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -129,6 +129,12 @@ class Projects::EnvironmentsController < Projects::ApplicationController end end + def additional_metrics + additional_metrics = environment.additional_metrics || {} + + render json: additional_metrics, status: additional_metrics.any? ? :ok : :no_content + end + private def verify_api_request! diff --git a/app/controllers/projects/prometheus_controller.rb b/app/controllers/projects/prometheus_controller.rb new file mode 100644 index 00000000000..74e247535d5 --- /dev/null +++ b/app/controllers/projects/prometheus_controller.rb @@ -0,0 +1,22 @@ +class Projects::PrometheusController < Projects::ApplicationController + before_action :authorize_read_project! + + def active_metrics + return render_404 unless has_prometheus_metrics? + matched_metrics = prometheus_service.reactive_query(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) + + if matched_metrics + render json: matched_metrics, status: :ok + else + head :no_content + end + end + + def prometheus_service + project.monitoring_service + end + + def has_prometheus_metrics? + prometheus_service&.respond_to?(:reactive_query) + end +end diff --git a/app/models/environment.rb b/app/models/environment.rb index 61572d8d69a..b4a4f74a8d5 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -149,10 +149,18 @@ class Environment < ActiveRecord::Base project.monitoring_service.present? && available? && last_deployment.present? end + def has_additional_metrics? + has_metrics? && project.monitoring_service&.respond_to?(:reactive_query) + end + def metrics project.monitoring_service.environment_metrics(self) if has_metrics? end + def additional_metrics + project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsQuery, self.id) if has_additional_metrics? + end + # An environment name is not necessarily suitable for use in URLs, DNS # or other third-party contexts, so provide a slugified version. A slug has # the following properties: diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index ec72cb6856d..674d485a03c 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -64,23 +64,26 @@ class PrometheusService < MonitoringService end def environment_metrics(environment) - with_reactive_cache(Gitlab::Prometheus::Queries::EnvironmentQuery.name, environment.id, &:itself) + with_reactive_cache(Gitlab::Prometheus::Queries::EnvironmentQuery.name, environment.id, &method(:rename_data_to_metrics)) end def deployment_metrics(deployment) - metrics = with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.id, &:itself) + metrics = with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.id, &method(:rename_data_to_metrics)) metrics&.merge(deployment_time: created_at.to_i) || {} end + def reactive_query(query_class, *args, &block) + calculate_reactive_cache(query_class, *args, &block) + end + # Cache metrics for specific environment def calculate_reactive_cache(query_class_name, *args) return unless active? && project && !project.pending_delete? - metrics = Kernel.const_get(query_class_name).new(client).query(*args) - + data = Kernel.const_get(query_class_name).new(client).query(*args) { success: true, - metrics: metrics, + data: data, last_update: Time.now.utc } rescue Gitlab::PrometheusError => err @@ -90,4 +93,11 @@ class PrometheusService < MonitoringService def client @prometheus ||= Gitlab::PrometheusClient.new(api_url: api_url) end + + private + + def rename_data_to_metrics(metrics) + metrics[:metrics] = metrics.delete :data + metrics + end end diff --git a/config/additional_metrics.yml b/config/additional_metrics.yml new file mode 100644 index 00000000000..209209f4b30 --- /dev/null +++ b/config/additional_metrics.yml @@ -0,0 +1,26 @@ +- group: Kubernetes + priority: 1 + metrics: + - title: "Memory usage" + detect: container_memory_usage_bytes + weight: 1 + queries: + - query_range: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' + label: Container memory + unit: MiB + - title: "Current memory usage" + detect: container_memory_usage_bytes + weight: 1 + queries: + - query: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' + unit: MiB + - title: "CPU usage" + detect: container_cpu_usage_seconds_total + weight: 1 + queries: + - query_range: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' + - title: "Current CPU usage" + detect: container_cpu_usage_seconds_total + weight: 1 + queries: + - query: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' diff --git a/config/routes/project.rb b/config/routes/project.rb index 9fe8372edf9..f5d00e4e93b 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -72,6 +72,10 @@ constraints(ProjectUrlConstrainer.new) do resource :mattermost, only: [:new, :create] + namespace :prometheus do + get :active_metrics + end + resources :deploy_keys, constraints: { id: /\d+/ }, only: [:index, :new, :create] do member do put :enable @@ -152,6 +156,7 @@ constraints(ProjectUrlConstrainer.new) do post :stop get :terminal get :metrics + get :additional_metrics get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', constraints: { format: nil } end diff --git a/lib/gitlab/prometheus/metric.rb b/lib/gitlab/prometheus/metric.rb new file mode 100644 index 00000000000..2818afb34b0 --- /dev/null +++ b/lib/gitlab/prometheus/metric.rb @@ -0,0 +1,28 @@ +module Gitlab::Prometheus + class Metric + attr_reader :group, :title, :detect, :weight, :queries + + def initialize(group, title, detect, weight, queries = []) + @group = group + @title = title + @detect = detect + @weight = weight + @queries = queries + end + + def self.metric_from_entry(group, entry) + missing_fields = [:title, :detect, :weight, :queries].select { |key| !entry.has_key?(key) } + raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? + + Metric.new(group, entry[:title], entry[:detect], entry[:weight], entry[:queries]) + end + + def self.metrics_from_list(group, list) + list.map { |entry| metric_from_entry(group, entry) } + end + + def self.additional_metrics_raw + @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml')).map(&:deep_symbolize_keys) + end + end +end diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb new file mode 100644 index 00000000000..093390b4fa7 --- /dev/null +++ b/lib/gitlab/prometheus/metric_group.rb @@ -0,0 +1,33 @@ +module Gitlab::Prometheus + class MetricGroup + attr_reader :priority, :name + attr_accessor :metrics + + def initialize(name, priority, metrics = []) + @name = name + @priority = priority + @metrics = metrics + end + + def self.all + load_groups_from_yaml + end + + def self.group_from_entry(entry) + missing_fields = [:group, :priority, :metrics].select { |key| !entry.has_key?(key) } + raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? + + group = MetricGroup.new(entry[:group], entry[:priority]) + group.metrics = Metric.metrics_from_list(group, entry[:metrics]) + group + end + + def self.load_groups_from_yaml + additional_metrics_raw.map(&method(:group_from_entry)) + end + + def self.additional_metrics_raw + @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml'))&.map(&:deep_symbolize_keys).freeze + end + end +end diff --git a/lib/gitlab/prometheus/metrics_sources.rb b/lib/gitlab/prometheus/metrics_sources.rb new file mode 100644 index 00000000000..500b6e971a2 --- /dev/null +++ b/lib/gitlab/prometheus/metrics_sources.rb @@ -0,0 +1,7 @@ +module Gitlab::Prometheus + module MetricsSources + def self.additional_metrics + @additional_metrics ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml')).deep_symbolize_keys.freeze + end + end +end diff --git a/lib/gitlab/prometheus/parsing_error.rb b/lib/gitlab/prometheus/parsing_error.rb new file mode 100644 index 00000000000..067ea7f878a --- /dev/null +++ b/lib/gitlab/prometheus/parsing_error.rb @@ -0,0 +1,3 @@ +module Gitlab::Prometheus + ParsingError = Class.new(StandardError) +end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb new file mode 100644 index 00000000000..001701383c3 --- /dev/null +++ b/lib/gitlab/prometheus/queries/additional_metrics_query.rb @@ -0,0 +1,70 @@ +module Gitlab::Prometheus::Queries + class AdditionalMetricsQuery < BaseQuery + def self.metrics + @metrics ||= YAML.load_file(Rails.root.join('config/custom_metrics.yml')).freeze + end + + def query(environment_id) + environment = Environment.find_by(id: environment_id) + + context = { + environment_slug: environment.slug, + environment_filter: %{container_name!="POD",environment="#{environment.slug}"} + } + + timeframe_start = 8.hours.ago.to_f + timeframe_end = Time.now.to_f + + matched_metrics.map do |group| + group[:metrics].map! do |metric| + metric[:queries].map! do |query| + query = query.symbolize_keys + query[:result] = + if query.has_key?(:query_range) + client_query_range(query[:query_range] % context, start: timeframe_start, stop: timeframe_end) + else + client_query(query[:query] % context, time: timeframe_end) + end + query + end + metric + end + group + end + end + + def process_query(group, query) + result = if query.has_key?(:query_range) + client_query_range(query[:query_range] % context, start: timeframe_start, stop: timeframe_end) + else + client_query(query[:query] % context, time: timeframe_end) + end + contains_metrics = result.all? do |item| + item&.[](:values)&.any? || item&.[](:value)&.any? + end + end + + def process_result(query_result) + contains_metrics = query_result.all? do |item| + item&.[](:values)&.any? || item&.[](:value)&.any? + end + + contains_metrics + end + + def matched_metrics + label_values = client_label_values || [] + + result = Gitlab::Prometheus::MetricsSources.additional_metrics.map do |group| + group[:metrics].map!(&:symbolize_keys) + group[:metrics].select! do |metric| + matcher = Regexp.compile(metric[:detect]) + label_values.any? &matcher.method(:match) + end + group + end + + result.select {|group| !group[:metrics].empty?} + end + end +end diff --git a/lib/gitlab/prometheus/queries/base_query.rb b/lib/gitlab/prometheus/queries/base_query.rb index 2a2eb4ae57f..c60828165bd 100644 --- a/lib/gitlab/prometheus/queries/base_query.rb +++ b/lib/gitlab/prometheus/queries/base_query.rb @@ -3,7 +3,7 @@ module Gitlab module Queries class BaseQuery attr_accessor :client - delegate :query_range, :query, to: :client, prefix: true + delegate :query_range, :query, :label_values, :series, to: :client, prefix: true def raw_memory_usage_query(environment_slug) %{avg(container_memory_usage_bytes{container_name!="POD",environment="#{environment_slug}"}) / 2^20} diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metrics_query.rb new file mode 100644 index 00000000000..61926320e40 --- /dev/null +++ b/lib/gitlab/prometheus/queries/matched_metrics_query.rb @@ -0,0 +1,72 @@ +module Gitlab::Prometheus::Queries + class MatchedMetricsQuery < BaseQuery + MAX_QUERY_ITEMS = 40.freeze + + def self.metrics + @metrics ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml')).map(&:deep_symbolize_keys) + end + + def query + groups_data.map do |group, data| + { + group: group.name, + priority: group.priority, + active_metrics: data[:active_metrics], + metrics_missing_requirements: data[:metrics_missing_requirements] + } + end + end + + def groups_data + metrics_series = metrics_with_series(Gitlab::Prometheus::MetricGroup.all) + lookup = active_series_lookup(metrics_series) + + groups = {} + + metrics_series.each do |metrics, series| + groups[metrics.group] ||= { active_metrics: 0, metrics_missing_requirements: 0 } + group = groups[metrics.group] + + if series.all?(&lookup.method(:has_key?)) + group[:active_metrics] += 1 + else + group[:metrics_missing_requirements] += 1 + end + group + end + + groups + end + + def active_series_lookup(metrics) + timeframe_start = 8.hours.ago + timeframe_end = Time.now + + series = metrics.flat_map { |metrics, series| series }.uniq + + lookup = series.each_slice(MAX_QUERY_ITEMS).flat_map do |batched_series| + client_series(*batched_series, start: timeframe_start, stop: timeframe_end) + .select(&method(:has_matching_label)) + .map { |series_info| [series_info['__name__'], true] } + end + lookup.to_h + end + + def has_matching_label(series_info) + series_info.has_key?('environment') + end + + def metrics_with_series(metric_groups) + label_values = client_label_values || [] + + metrics = metric_groups.flat_map do |group| + group.metrics.map do |metric| + matcher = Regexp.compile(metric.detect) + [metric, label_values.select(&matcher.method(:match))] + end + end + + metrics.select { |metric, labels| labels&.any? } + end + end +end diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb index 5b51a1779dd..f4ef4ff8ba4 100644 --- a/lib/gitlab/prometheus_client.rb +++ b/lib/gitlab/prometheus_client.rb @@ -29,6 +29,14 @@ module Gitlab end end + def label_values(name='__name__') + json_api_get("label/#{name}/values") + end + + def series(*matches, start: 8.hours.ago, stop: Time.now) + json_api_get('series', 'match': matches, start: start.to_f, end: stop.to_f) + end + private def json_api_get(type, args = {}) -- cgit v1.2.1 From 61d7b7f117ff090f34f882918d55691575d76e55 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 25 May 2017 14:01:10 +0200 Subject: Finalize refactoring additional metrics query --- app/models/environment.rb | 4 +- app/models/project_services/prometheus_service.rb | 2 +- lib/gitlab/prometheus/metric_group.rb | 8 +-- .../prometheus/queries/additional_metrics_query.rb | 79 +++++++++++----------- .../prometheus/queries/matched_metrics_query.rb | 6 +- 5 files changed, 49 insertions(+), 50 deletions(-) diff --git a/app/models/environment.rb b/app/models/environment.rb index b4a4f74a8d5..7ab4a23ab16 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -158,7 +158,9 @@ class Environment < ActiveRecord::Base end def additional_metrics - project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsQuery, self.id) if has_additional_metrics? + if has_additional_metrics? + project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsQuery.name, self.id, &:itself) + end end # An environment name is not necessarily suitable for use in URLs, DNS diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 674d485a03c..d3f43d66937 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -73,7 +73,7 @@ class PrometheusService < MonitoringService end def reactive_query(query_class, *args, &block) - calculate_reactive_cache(query_class, *args, &block) + with_reactive_cache(query_class, *args, &block) end # Cache metrics for specific environment diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb index 093390b4fa7..9f95525bc0c 100644 --- a/lib/gitlab/prometheus/metric_group.rb +++ b/lib/gitlab/prometheus/metric_group.rb @@ -13,6 +13,10 @@ module Gitlab::Prometheus load_groups_from_yaml end + def self.load_groups_from_yaml + additional_metrics_raw.map(&method(:group_from_entry)) + end + def self.group_from_entry(entry) missing_fields = [:group, :priority, :metrics].select { |key| !entry.has_key?(key) } raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? @@ -22,10 +26,6 @@ module Gitlab::Prometheus group end - def self.load_groups_from_yaml - additional_metrics_raw.map(&method(:group_from_entry)) - end - def self.additional_metrics_raw @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml'))&.map(&:deep_symbolize_keys).freeze end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb index 001701383c3..de0dab0b19c 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_query.rb @@ -1,47 +1,46 @@ module Gitlab::Prometheus::Queries class AdditionalMetricsQuery < BaseQuery - def self.metrics - @metrics ||= YAML.load_file(Rails.root.join('config/custom_metrics.yml')).freeze - end - def query(environment_id) - environment = Environment.find_by(id: environment_id) - - context = { - environment_slug: environment.slug, - environment_filter: %{container_name!="POD",environment="#{environment.slug}"} - } - - timeframe_start = 8.hours.ago.to_f - timeframe_end = Time.now.to_f + query_processor = method(:process_query).curry[query_context(environment_id)] matched_metrics.map do |group| - group[:metrics].map! do |metric| - metric[:queries].map! do |query| - query = query.symbolize_keys - query[:result] = - if query.has_key?(:query_range) - client_query_range(query[:query_range] % context, start: timeframe_start, stop: timeframe_end) - else - client_query(query[:query] % context, time: timeframe_end) - end - query - end - metric + metrics = group.metrics.map do |metric| + { + title: metric.title, + weight: metric.weight, + queries: metric.queries.map(&query_processor) + } end - group + + { + group: group.name, + priority: group.priority, + metrics: metrics + } end end - def process_query(group, query) - result = if query.has_key?(:query_range) - client_query_range(query[:query_range] % context, start: timeframe_start, stop: timeframe_end) - else - client_query(query[:query] % context, time: timeframe_end) - end - contains_metrics = result.all? do |item| - item&.[](:values)&.any? || item&.[](:value)&.any? - end + private + + def query_context(environment_id) + environment = Environment.find_by(id: environment_id) + { + environment_slug: environment.slug, + environment_filter: %{container_name!="POD",environment="#{environment.slug}"}, + timeframe_start: 8.hours.ago.to_f, + timeframe_end: Time.now.to_f + } + end + + def process_query(context, query) + query_with_result = query.dup + query_with_result[:result] = + if query.has_key?(:query_range) + client_query_range(query[:query_range] % context, start: context[:timeframe_start], stop: context[:timeframe_end]) + else + client_query(query[:query] % context, time: context[:timeframe_end]) + end + query_with_result end def process_result(query_result) @@ -54,17 +53,17 @@ module Gitlab::Prometheus::Queries def matched_metrics label_values = client_label_values || [] + Gitlab::Prometheus::MetricGroup.all - result = Gitlab::Prometheus::MetricsSources.additional_metrics.map do |group| - group[:metrics].map!(&:symbolize_keys) - group[:metrics].select! do |metric| - matcher = Regexp.compile(metric[:detect]) + result = Gitlab::Prometheus::MetricGroup.all.map do |group| + group.metrics.select! do |metric| + matcher = Regexp.compile(metric.detect) label_values.any? &matcher.method(:match) end group end - result.select {|group| !group[:metrics].empty?} + result.select { |group| group.metrics.any? } end end end diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metrics_query.rb index 61926320e40..5a8b0f6c701 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metrics_query.rb @@ -2,10 +2,6 @@ module Gitlab::Prometheus::Queries class MatchedMetricsQuery < BaseQuery MAX_QUERY_ITEMS = 40.freeze - def self.metrics - @metrics ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml')).map(&:deep_symbolize_keys) - end - def query groups_data.map do |group, data| { @@ -17,6 +13,8 @@ module Gitlab::Prometheus::Queries end end + private + def groups_data metrics_series = metrics_with_series(Gitlab::Prometheus::MetricGroup.all) lookup = active_series_lookup(metrics_series) -- cgit v1.2.1 From 4d8f3978a254f33d959de65dbf7b20a1d41a058d Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 25 May 2017 16:19:54 +0200 Subject: Use before action to respond with 404 if prometheus metrics are missing --- app/controllers/projects/prometheus_controller.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/prometheus_controller.rb b/app/controllers/projects/prometheus_controller.rb index 74e247535d5..0402be6f85c 100644 --- a/app/controllers/projects/prometheus_controller.rb +++ b/app/controllers/projects/prometheus_controller.rb @@ -1,8 +1,8 @@ class Projects::PrometheusController < Projects::ApplicationController before_action :authorize_read_project! + before_action :require_prometheus_metrics! def active_metrics - return render_404 unless has_prometheus_metrics? matched_metrics = prometheus_service.reactive_query(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) if matched_metrics @@ -12,6 +12,8 @@ class Projects::PrometheusController < Projects::ApplicationController end end + private + def prometheus_service project.monitoring_service end @@ -19,4 +21,8 @@ class Projects::PrometheusController < Projects::ApplicationController def has_prometheus_metrics? prometheus_service&.respond_to?(:reactive_query) end + + def require_prometheus_metrics! + render_404 unless has_prometheus_metrics? + end end -- cgit v1.2.1 From 608186d54bb9aefb0b867c177ac62d534e8840ad Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 25 May 2017 17:39:43 +0200 Subject: Add per deployment additional metrics --- app/controllers/projects/deployments_controller.rb | 11 +++++++++ app/models/deployment.rb | 10 +++++++++ config/routes/project.rb | 1 + .../queries/additional_metrics_deployment_query.rb | 17 ++++++++++++++ .../prometheus/queries/additional_metrics_query.rb | 26 +++++++++++++--------- 5 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index 6644deb49c9..29d94e2760a 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -22,6 +22,17 @@ class Projects::DeploymentsController < Projects::ApplicationController render_404 end + def additional_metrics + return render_404 unless deployment.has_additional_metrics? + metrics = deployment.additional_metrics + + if metrics&.any? + render json: metrics, status: :ok + else + head :no_content + end + end + private def deployment diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 216cec751e3..f49410f18ae 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -103,12 +103,22 @@ class Deployment < ActiveRecord::Base project.monitoring_service.present? end + def has_additional_metrics? + has_metrics? && project.monitoring_service&.respond_to?(:reactive_query) + end + def metrics return {} unless has_metrics? project.monitoring_service.deployment_metrics(self) end + def additional_metrics + return {} unless has_additional_metrics? + metrics = project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, id, &:itself) + metrics&.merge(deployment_time: created_at.to_i) || {} + end + private def ref_path diff --git a/config/routes/project.rb b/config/routes/project.rb index f5d00e4e93b..89b14ef6527 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -167,6 +167,7 @@ constraints(ProjectUrlConstrainer.new) do resources :deployments, only: [:index] do member do get :metrics + get :additional_metrics end end end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb new file mode 100644 index 00000000000..382052c298f --- /dev/null +++ b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb @@ -0,0 +1,17 @@ +module Gitlab::Prometheus::Queries + class AdditionalMetricsDeploymentQuery < AdditionalMetricsQuery + + def query(deployment_id) + deployment = Deployment.find_by(id: deployment_id) + query_context = { + environment_slug: deployment.environment.slug, + environment_filter: %{container_name!="POD",environment="#{deployment.environment.slug}"}, + timeframe_start: (deployment.created_at - 30.minutes).to_f, + timeframe_end: (deployment.created_at + 30.minutes).to_f + } + + query_metrics(query_context) + end + end +end + diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb index de0dab0b19c..c48fcadee57 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_query.rb @@ -1,7 +1,21 @@ module Gitlab::Prometheus::Queries class AdditionalMetricsQuery < BaseQuery def query(environment_id) - query_processor = method(:process_query).curry[query_context(environment_id)] + environment = Environment.find_by(id: environment_id) + query_context = { + environment_slug: environment.slug, + environment_filter: %{container_name!="POD",environment="#{environment.slug}"}, + timeframe_start: 8.hours.ago.to_f, + timeframe_end: Time.now.to_f + } + + query_metrics(query_context) + end + + protected + + def query_metrics(query_context) + query_processor = method(:process_query).curry[query_context] matched_metrics.map do |group| metrics = group.metrics.map do |metric| @@ -22,16 +36,6 @@ module Gitlab::Prometheus::Queries private - def query_context(environment_id) - environment = Environment.find_by(id: environment_id) - { - environment_slug: environment.slug, - environment_filter: %{container_name!="POD",environment="#{environment.slug}"}, - timeframe_start: 8.hours.ago.to_f, - timeframe_end: Time.now.to_f - } - end - def process_query(context, query) query_with_result = query.dup query_with_result[:result] = -- cgit v1.2.1 From aaeda829ddb5e150be41c9dff374b2be0beab336 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 30 May 2017 13:17:13 +0200 Subject: Add Y Label field to yml and responses --- config/additional_metrics.yml | 1 + lib/gitlab/prometheus/metric.rb | 7 ++++--- lib/gitlab/prometheus/metrics_sources.rb | 7 ------- lib/gitlab/prometheus/queries/additional_metrics_query.rb | 1 + 4 files changed, 6 insertions(+), 10 deletions(-) delete mode 100644 lib/gitlab/prometheus/metrics_sources.rb diff --git a/config/additional_metrics.yml b/config/additional_metrics.yml index 209209f4b30..59ca387f055 100644 --- a/config/additional_metrics.yml +++ b/config/additional_metrics.yml @@ -2,6 +2,7 @@ priority: 1 metrics: - title: "Memory usage" + y_label: "Values" detect: container_memory_usage_bytes weight: 1 queries: diff --git a/lib/gitlab/prometheus/metric.rb b/lib/gitlab/prometheus/metric.rb index 2818afb34b0..777cf030ceb 100644 --- a/lib/gitlab/prometheus/metric.rb +++ b/lib/gitlab/prometheus/metric.rb @@ -1,12 +1,13 @@ module Gitlab::Prometheus class Metric - attr_reader :group, :title, :detect, :weight, :queries + attr_reader :group, :title, :detect, :weight, :y_label, :queries - def initialize(group, title, detect, weight, queries = []) + def initialize(group, title, detect, weight, y_label, queries = []) @group = group @title = title @detect = detect @weight = weight + @y_label = y_label || 'Values' @queries = queries end @@ -14,7 +15,7 @@ module Gitlab::Prometheus missing_fields = [:title, :detect, :weight, :queries].select { |key| !entry.has_key?(key) } raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? - Metric.new(group, entry[:title], entry[:detect], entry[:weight], entry[:queries]) + Metric.new(group, entry[:title], entry[:detect], entry[:weight], entry[:y_label],entry[:queries]) end def self.metrics_from_list(group, list) diff --git a/lib/gitlab/prometheus/metrics_sources.rb b/lib/gitlab/prometheus/metrics_sources.rb deleted file mode 100644 index 500b6e971a2..00000000000 --- a/lib/gitlab/prometheus/metrics_sources.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Gitlab::Prometheus - module MetricsSources - def self.additional_metrics - @additional_metrics ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml')).deep_symbolize_keys.freeze - end - end -end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb index c48fcadee57..fd7f072834d 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_query.rb @@ -22,6 +22,7 @@ module Gitlab::Prometheus::Queries { title: metric.title, weight: metric.weight, + y_label: metric.y_label, queries: metric.queries.map(&query_processor) } end -- cgit v1.2.1 From 9ccda901271eb755cf37f37831f6beef83ed7037 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 30 May 2017 15:25:05 +0200 Subject: Add Prometheus client tests --- spec/lib/gitlab/prometheus_client_spec.rb | 30 ++++++++++++++++++++ spec/support/prometheus_helpers.rb | 46 +++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/spec/lib/gitlab/prometheus_client_spec.rb b/spec/lib/gitlab/prometheus_client_spec.rb index 2d8bd2f6b97..46eaadae206 100644 --- a/spec/lib/gitlab/prometheus_client_spec.rb +++ b/spec/lib/gitlab/prometheus_client_spec.rb @@ -119,6 +119,36 @@ describe Gitlab::PrometheusClient, lib: true do end end + describe '#series' do + let(:query_url) { prometheus_series_url('series_name', 'other_service') } + + around do |example| + Timecop.freeze { example.run } + end + + it 'calls endpoint and returns list of series' do + req_stub = stub_prometheus_request(query_url, body: prometheus_series('series_name')) + expected = prometheus_series('series_name').deep_stringify_keys['data'] + + expect(subject.series('series_name', 'other_service')).to eq(expected) + + expect(req_stub).to have_been_requested + end + end + + describe '#label_values' do + let(:query_url) { prometheus_label_values_url('__name__') } + + it 'calls endpoint and returns label values' do + req_stub = stub_prometheus_request(query_url, body: prometheus_label_values) + expected = prometheus_label_values.deep_stringify_keys['data'] + + expect(subject.label_values('__name__')).to eq(expected) + + expect(req_stub).to have_been_requested + end + end + describe '#query_range' do let(:prometheus_query) { prometheus_memory_query('env-slug') } let(:query_url) { prometheus_query_range_url(prometheus_query) } diff --git a/spec/support/prometheus_helpers.rb b/spec/support/prometheus_helpers.rb index 6b9ebcf2bb3..e49902475da 100644 --- a/spec/support/prometheus_helpers.rb +++ b/spec/support/prometheus_helpers.rb @@ -36,6 +36,19 @@ module PrometheusHelpers "https://prometheus.example.com/api/v1/query_range?#{query}" end + def prometheus_label_values_url(name) + "https://prometheus.example.com/api/v1/label/#{name}/values" + end + + def prometheus_series_url(*matches, start: 8.hours.ago, stop: Time.now) + query = { + match: matches, + start: start.to_f, + end: stop.to_f + }.to_query + "https://prometheus.example.com/api/v1/series?#{query}" + end + def stub_prometheus_request(url, body: {}, status: 200) WebMock.stub_request(:get, url) .to_return({ @@ -140,4 +153,37 @@ module PrometheusHelpers } } end + + def prometheus_label_values + { + 'status': 'success', + 'data': %w(job_adds job_controller_rate_limiter_use job_depth job_queue_latency job_work_duration_sum up) + } + end + + def prometheus_series(name) + { + 'status': 'success', + 'data': [ + { + '__name__': name, + 'container_name': 'gitlab', + 'environment': 'mattermost', + 'id': '/docker/9953982f95cf5010dfc59d7864564d5f188aaecddeda343699783009f89db667', + 'image': 'gitlab/gitlab-ce:8.15.4-ce.1', + 'instance': 'minikube', + 'job': 'kubernetes-nodes', + 'name': 'k8s_gitlab.e6611886_mattermost-4210310111-77z8r_gitlab_2298ae6b-da24-11e6-baee-8e7f67d0eb3a_43536cb6', + 'namespace': 'gitlab', + 'pod_name': 'mattermost-4210310111-77z8r' + }, + { + '__name__': name, + 'id': '/docker', + 'instance': 'minikube', + 'job': 'kubernetes-nodes' + } + ] + } + end end -- cgit v1.2.1 From efb264f70d943196f94132655a50d73fc416f9dc Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 26 May 2017 19:53:48 +0530 Subject: Empty state icon for Metrics --- app/views/shared/icons/_icon_empty_metrics.svg | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 app/views/shared/icons/_icon_empty_metrics.svg diff --git a/app/views/shared/icons/_icon_empty_metrics.svg b/app/views/shared/icons/_icon_empty_metrics.svg new file mode 100644 index 00000000000..24fa353f3ba --- /dev/null +++ b/app/views/shared/icons/_icon_empty_metrics.svg @@ -0,0 +1,5 @@ + + + + + -- cgit v1.2.1 From 2e302e35e798ae171e08c08b81092e12c34dcfc7 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 26 May 2017 19:54:10 +0530 Subject: Prometheus Metrics Panel View --- .../projects/services/prometheus/_show.html.haml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 app/views/projects/services/prometheus/_show.html.haml diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml new file mode 100644 index 00000000000..70f53bffe58 --- /dev/null +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -0,0 +1,21 @@ +.row.prepend-top-default.append-bottom-default.prometheus-metrics-monitoring + .col-lg-3 + %h4.prepend-top-0 + Metrics + %p + Metrics are automatically configured and monitored + based on a library of metrics from popular exporters. + More information + + .col-lg-9 + .panel.panel-default + .panel-heading + %h3.panel-title + Monitored + %span.badge-count.js-monitored-count 0 + .panel-body + .empty-metrics + = custom_icon('icon_empty_metrics') + %p No metrics are being monitored. To start monitoring, deploy to an environment. + = link_to project_environments_path(@project), title: 'View environments', class: 'btn btn-success' do + View environments \ No newline at end of file -- cgit v1.2.1 From 7cef07c948db4aa61ec948ab227c4d42438b87fc Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 26 May 2017 19:55:14 +0530 Subject: Render service extras partial conditionally --- app/views/projects/services/_form.html.haml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index f1a80f1d5e1..f9ce81b729f 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -18,3 +18,7 @@ = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled_class}", title: disabled_title = link_to "Cancel", namespace_project_settings_integrations_path(@project.namespace, @project), class: "btn btn-cancel" + +- if lookup_context.template_exists?('show', "projects/services/#{@service.to_param}", true) + %hr + = render "projects/services/#{@service.to_param}/show" \ No newline at end of file -- cgit v1.2.1 From 33fe2af4a3cfe0d1f1b93b8c6546f796cde141f2 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 26 May 2017 19:55:37 +0530 Subject: Add styles for Prometheus Metrics panel --- app/assets/stylesheets/pages/settings.scss | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 3889deee21a..e1ee566c3a1 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -35,3 +35,28 @@ margin-left: 5px; } } + +.prometheus-metrics-monitoring { + .panel { + .panel-heading .badge-count { + padding: 3px 8px; + color: $white-light; + background: $common-gray-dark; + border-radius: 40%; + } + } + + .empty-metrics { + text-align: center; + padding: 20px 10px; + + p, + .btn { + margin-top: 10px; + } + + p { + color: $gl-gray-light; + } + } +} \ No newline at end of file -- cgit v1.2.1 From 2559a880eebd2b8beb0a29bad6ab9948dca0c010 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 17:08:22 +0530 Subject: Prometheus Metrics Bundle --- app/assets/javascripts/prometheus_metrics/index.js | 7 ++ .../prometheus_metrics/prometheus_metrics.js | 75 ++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 app/assets/javascripts/prometheus_metrics/index.js create mode 100644 app/assets/javascripts/prometheus_metrics/prometheus_metrics.js diff --git a/app/assets/javascripts/prometheus_metrics/index.js b/app/assets/javascripts/prometheus_metrics/index.js new file mode 100644 index 00000000000..9f20161783b --- /dev/null +++ b/app/assets/javascripts/prometheus_metrics/index.js @@ -0,0 +1,7 @@ +import PrometheusMetrics from './prometheus_metrics'; + +$(() => { + const prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); + prometheusMetrics.init(); + prometheusMetrics.loadActiveMetrics(); +}); diff --git a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js new file mode 100644 index 00000000000..0dc6f7727a2 --- /dev/null +++ b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js @@ -0,0 +1,75 @@ +/* eslint-disable class-methods-use-this, promise/catch-or-return */ + +export default class PrometheusMetrics { + constructor(wrapperSelector) { + this.backOffRequestCounter = 0; + + this.$wrapper = $(wrapperSelector); + + this.$monitoredMetricsPanel = this.$wrapper.find('.js-panel-monitored-metrics'); + this.$monitoredMetricsCount = this.$monitoredMetricsPanel.find('.js-monitored-count'); + this.$monitoredMetricsLoading = this.$monitoredMetricsPanel.find('.js-loading-metrics'); + this.$monitoredMetricsEmpty = this.$monitoredMetricsPanel.find('.js-empty-metrics'); + this.$monitoredMetricsList = this.$monitoredMetricsPanel.find('.js-metrics-list'); + + this.$panelToggle = this.$wrapper.find('.js-panel-toggle'); + + this.activeMetricsEndpoint = this.$monitoredMetricsPanel.data('active-metrics'); + } + + init() { + this.$panelToggle.on('click', e => this.handlePanelToggle(e)); + } + + handlePanelToggle(e) { + const $toggleBtn = $(e.currentTarget); + const $currentPanelBody = $toggleBtn.parents('.panel').find('.panel-body'); + if ($currentPanelBody.is(':visible')) { + $currentPanelBody.addClass('hidden'); + $toggleBtn.removeClass('fa-caret-down').addClass('fa-caret-right'); + } else { + $currentPanelBody.removeClass('hidden'); + $toggleBtn.removeClass('fa-caret-right').addClass('fa-caret-down'); + } + } + + populateActiveMetrics(metrics) { + let totalMonitoredMetrics = 0; + metrics.forEach((metric) => { + this.$monitoredMetricsList.append(`
  • ${metric.group}${metric.active_metrics}
  • `); + totalMonitoredMetrics += metric.active_metrics; + }); + + this.$monitoredMetricsCount.text(totalMonitoredMetrics); + this.$monitoredMetricsLoading.addClass('hidden'); + this.$monitoredMetricsList.removeClass('hidden'); + } + + loadActiveMetrics() { + this.$monitoredMetricsLoading.removeClass('hidden'); + gl.utils.backOff((next, stop) => { + $.getJSON(this.activeMetricsEndpoint) + .done((res) => { + if (res && res.success) { + stop(res); + } else { + this.backOffRequestCounter = this.backOffRequestCounter += 1; + if (this.backOffRequestCounter < 3) { + next(); + } else { + stop(res); + } + } + }) + .fail(stop); + }) + .then((res) => { + if (res && res.data && res.data.length) { + this.populateActiveMetrics(res.data); + } else { + this.$monitoredMetricsLoading.addClass('hidden'); + this.$monitoredMetricsEmpty.removeClass('hidden'); + } + }); + } +} -- cgit v1.2.1 From 2742bbd22c6f2330d87b631d7be25f1d7549a1d8 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 17:08:41 +0530 Subject: Add Prometheus Metrics Bundle --- config/webpack.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/config/webpack.config.js b/config/webpack.config.js index c77b1d6334c..38769f3f625 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -51,6 +51,7 @@ var config = { pipelines: './pipelines/index.js', pipelines_details: './pipelines/pipeline_details_bundle.js', profile: './profile/profile_bundle.js', + prometheus_metrics: './prometheus_metrics', protected_branches: './protected_branches/protected_branches_bundle.js', protected_tags: './protected_tags', sidebar: './sidebar/sidebar_bundle.js', -- cgit v1.2.1 From 15908cf29099b994b4495ac864446ef2045d254c Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 17:09:21 +0530 Subject: Add Prometheus Metrics Bundle, panel for Missing environment variables --- .../projects/services/prometheus/_show.html.haml | 25 ++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index 70f53bffe58..2f0f539560a 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -1,4 +1,7 @@ -.row.prepend-top-default.append-bottom-default.prometheus-metrics-monitoring +- content_for :page_specific_javascripts do + = webpack_bundle_tag('prometheus_metrics') + +.row.prepend-top-default.append-bottom-default.prometheus-metrics-monitoring.js-prometheus-metrics-monitoring .col-lg-3 %h4.prepend-top-0 Metrics @@ -8,14 +11,28 @@ More information .col-lg-9 - .panel.panel-default + .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{namespace_project_prometheus_active_metrics_path(@project.namespace, @project)}.json" } } .panel-heading %h3.panel-title Monitored %span.badge-count.js-monitored-count 0 .panel-body - .empty-metrics + .loading-metrics.js-loading-metrics + = icon('spinner spin 3x') + %p Finding and configuring metrics... + .empty-metrics.hidden.js-empty-metrics = custom_icon('icon_empty_metrics') %p No metrics are being monitored. To start monitoring, deploy to an environment. = link_to project_environments_path(@project), title: 'View environments', class: 'btn btn-success' do - View environments \ No newline at end of file + View environments + %ul.metrics-list.hidden.js-metrics-list + + .panel.panel-default.js-panel-missing-env-vars + .panel-heading + %h3.panel-title + = icon('caret-right lg', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel') + Missing environment variable(s) + %span.badge-count.js-env-var-count 0 + .panel-body.hidden + .empty-metrics + %p Nothing to show here at the moment \ No newline at end of file -- cgit v1.2.1 From a2b68693a86493150a68719376cdc990ad93267a Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 17:09:42 +0530 Subject: Add styles for Prometheus Service Metrics --- app/assets/stylesheets/pages/settings.scss | 38 +++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index e1ee566c3a1..3ce17d64182 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -38,14 +38,31 @@ .prometheus-metrics-monitoring { .panel { + .panel-toggle { + width: 12px; + + &.fa-caret-right { + padding-left: 4px; + } + } + .panel-heading .badge-count { - padding: 3px 8px; color: $white-light; background: $common-gray-dark; + } + + .panel-body { + padding: 0; + } + + .badge-count { + margin-left: 3px; + padding: 3px 8px; border-radius: 40%; } } + .loading-metrics, .empty-metrics { text-align: center; padding: 20px 10px; @@ -59,4 +76,23 @@ color: $gl-gray-light; } } + + .metrics-list { + list-style-type: none; + margin: 0; + padding: 0; + + li { + padding: 15px 10px; + + .badge-count { + background: $badge-bg; + } + } + + /* Ensure we don't add border if there's only single li */ + li + li { + border-top: 1px solid $border-color; + } + } } \ No newline at end of file -- cgit v1.2.1 From a57e6a8b760d79a0a2adca8cce0547928599e3cb Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 17:10:03 +0530 Subject: Add support for Prometheus Service --- spec/factories/services.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 3fad4d2d658..412dd82c929 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -25,6 +25,14 @@ FactoryGirl.define do }) end + factory :prometheus_service do + project factory: :empty_project + active true + properties({ + api_url: 'https://prometheus.example.com/' + }) + end + factory :jira_service do project factory: :empty_project active true -- cgit v1.2.1 From c3c644c722dc2f0887067dbba68e43d91e610f62 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 17:10:30 +0530 Subject: Prometheus Service Fixture Generator --- spec/javascripts/fixtures/prometheus_service.rb | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 spec/javascripts/fixtures/prometheus_service.rb diff --git a/spec/javascripts/fixtures/prometheus_service.rb b/spec/javascripts/fixtures/prometheus_service.rb new file mode 100644 index 00000000000..7dfbf885fbd --- /dev/null +++ b/spec/javascripts/fixtures/prometheus_service.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Projects::ServicesController, '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project_empty_repo, namespace: namespace, path: 'services-project') } + let!(:service) { create(:prometheus_service, project: project) } + + + render_views + + before(:all) do + clean_frontend_fixtures('services/') + end + + before(:each) do + sign_in(admin) + end + + it 'services/prometheus_service.html.raw' do |example| + get :edit, + namespace_id: namespace, + project_id: project, + id: service.to_param + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end +end -- cgit v1.2.1 From 5cccbde4c66eaf22fc45da5a8d87a3f05552ecc7 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 19:05:26 +0530 Subject: Populate Missing env var panel --- .../javascripts/prometheus_metrics/prometheus_metrics.js | 16 +++++++++++++++- app/views/projects/services/prometheus/_show.html.haml | 15 +++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js index 0dc6f7727a2..b44fdcc9dc4 100644 --- a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js +++ b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js @@ -12,7 +12,10 @@ export default class PrometheusMetrics { this.$monitoredMetricsEmpty = this.$monitoredMetricsPanel.find('.js-empty-metrics'); this.$monitoredMetricsList = this.$monitoredMetricsPanel.find('.js-metrics-list'); - this.$panelToggle = this.$wrapper.find('.js-panel-toggle'); + this.$missingEnvVarPanel = this.$wrapper.find('.js-panel-missing-env-vars'); + this.$panelToggle = this.$missingEnvVarPanel.find('.js-panel-toggle'); + this.$missingEnvVarMetricCount = this.$missingEnvVarPanel.find('.js-env-var-count'); + this.$missingEnvVarMetricsList = this.$missingEnvVarPanel.find('.js-missing-var-metrics-list'); this.activeMetricsEndpoint = this.$monitoredMetricsPanel.data('active-metrics'); } @@ -35,14 +38,25 @@ export default class PrometheusMetrics { populateActiveMetrics(metrics) { let totalMonitoredMetrics = 0; + let totalMissingEnvVarMetrics = 0; + metrics.forEach((metric) => { this.$monitoredMetricsList.append(`
  • ${metric.group}${metric.active_metrics}
  • `); totalMonitoredMetrics += metric.active_metrics; + if (metric.metrics_missing_requirements > 0) { + this.$missingEnvVarMetricsList.append(`
  • ${metric.group}
  • `); + totalMissingEnvVarMetrics += 1; + } }); this.$monitoredMetricsCount.text(totalMonitoredMetrics); this.$monitoredMetricsLoading.addClass('hidden'); this.$monitoredMetricsList.removeClass('hidden'); + + if (totalMissingEnvVarMetrics > 0) { + this.$missingEnvVarPanel.removeClass('hidden'); + this.$missingEnvVarMetricCount.text(totalMissingEnvVarMetrics); + } } loadActiveMetrics() { diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index 2f0f539560a..3192116d128 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -8,7 +8,7 @@ %p Metrics are automatically configured and monitored based on a library of metrics from popular exporters. - More information + = link_to 'More information', '#' .col-lg-9 .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{namespace_project_prometheus_active_metrics_path(@project.namespace, @project)}.json" } } @@ -27,12 +27,19 @@ View environments %ul.metrics-list.hidden.js-metrics-list - .panel.panel-default.js-panel-missing-env-vars + .panel.panel-default.hidden.js-panel-missing-env-vars .panel-heading %h3.panel-title = icon('caret-right lg', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel') Missing environment variable(s) %span.badge-count.js-env-var-count 0 .panel-body.hidden - .empty-metrics - %p Nothing to show here at the moment \ No newline at end of file + .flash-container + .flash-notice + .flash-text + To set up automatic monitoring, add the environment variable + %code + $CI_ENVIRONMENT_SLUG + to exporter's queries. + = link_to 'More information', '#' + %ul.metrics-list.js-missing-var-metrics-list \ No newline at end of file -- cgit v1.2.1 From b4b16212d9c17231f843db1b59caa5d086490860 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 19:05:54 +0530 Subject: Styles for banner within panel --- app/assets/stylesheets/pages/settings.scss | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 3ce17d64182..7c35690f584 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -60,6 +60,14 @@ padding: 3px 8px; border-radius: 40%; } + + .flash-container { + margin-bottom: 0; + + .flash-notice { + border-radius: 0; + } + } } .loading-metrics, -- cgit v1.2.1 From 85b6b9b6aa3f76bd7e6ea764a49829afbfba9de8 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 31 May 2017 20:17:31 +0530 Subject: Lint: Fix missing new line at EOF --- app/assets/stylesheets/pages/settings.scss | 2 +- app/views/projects/services/_form.html.haml | 2 +- app/views/projects/services/prometheus/_show.html.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 7c35690f584..7c99eef3406 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -103,4 +103,4 @@ border-top: 1px solid $border-color; } } -} \ No newline at end of file +} diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index f9ce81b729f..a9013701591 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -21,4 +21,4 @@ - if lookup_context.template_exists?('show', "projects/services/#{@service.to_param}", true) %hr - = render "projects/services/#{@service.to_param}/show" \ No newline at end of file + = render "projects/services/#{@service.to_param}/show" diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index 3192116d128..bf8bfe4ddcb 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -42,4 +42,4 @@ $CI_ENVIRONMENT_SLUG to exporter's queries. = link_to 'More information', '#' - %ul.metrics-list.js-missing-var-metrics-list \ No newline at end of file + %ul.metrics-list.js-missing-var-metrics-list -- cgit v1.2.1 From b83a40483ad5f87b3e08c3ed2d090b5d87bdac27 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 1 Jun 2017 13:25:16 +0530 Subject: Remove service help, add URL field help --- app/models/project_services/prometheus_service.rb | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index d3f43d66937..d4d6611fd3b 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -28,17 +28,6 @@ class PrometheusService < MonitoringService 'Prometheus monitoring' end - def help - <<-MD.strip_heredoc - Retrieves the Kubernetes node metrics `container_cpu_usage_seconds_total` - and `container_memory_usage_bytes` from the configured Prometheus server. - - If you are not using [Auto-Deploy](https://docs.gitlab.com/ee/ci/autodeploy/index.html) - or have set up your own Prometheus server, an `environment` label is required on each metric to - [identify the Environment](https://docs.gitlab.com/ce/user/project/integrations/prometheus.html#metrics-and-labels). - MD - end - def self.to_param 'prometheus' end @@ -49,7 +38,8 @@ class PrometheusService < MonitoringService type: 'text', name: 'api_url', title: 'API URL', - placeholder: 'Prometheus API Base URL, like http://prometheus.example.com/' + placeholder: 'Prometheus API Base URL, like http://prometheus.example.com/', + help: 'By default, Prometheus listens on ‘http://localhost:9090’. It’s not recommended to change the default address and port as this might affect or conflict with other services running on the GitLab server.' } ] end -- cgit v1.2.1 From 4826ae074f2757e2ca7cddbb201688a11bcf8f8b Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 1 Jun 2017 13:25:48 +0530 Subject: Changes based on UX review --- app/assets/stylesheets/pages/settings.scss | 9 +++++++-- app/views/projects/services/prometheus/_show.html.haml | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 7c99eef3406..deed4501399 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -73,11 +73,12 @@ .loading-metrics, .empty-metrics { text-align: center; - padding: 20px 10px; + padding: 30px 10px; p, .btn { margin-top: 10px; + margin-bottom: 0; } p { @@ -85,13 +86,17 @@ } } + .loading-metrics .fa-spin { + color: $loading-color; + } + .metrics-list { list-style-type: none; margin: 0; padding: 0; li { - padding: 15px 10px; + padding: 16px; .badge-count { background: $badge-bg; diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index bf8bfe4ddcb..82d4dd2488f 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -31,7 +31,7 @@ .panel-heading %h3.panel-title = icon('caret-right lg', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel') - Missing environment variable(s) + Missing environment variable %span.badge-count.js-env-var-count 0 .panel-body.hidden .flash-container @@ -40,6 +40,6 @@ To set up automatic monitoring, add the environment variable %code $CI_ENVIRONMENT_SLUG - to exporter's queries. + to exporter’s queries. = link_to 'More information', '#' %ul.metrics-list.js-missing-var-metrics-list -- cgit v1.2.1 From afc1a67d19c077bc61350a01d6a0f7e676e5a0f6 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 1 Jun 2017 15:05:15 +0530 Subject: Handle response failure case for loadActiveMetrics --- app/assets/javascripts/prometheus_metrics/prometheus_metrics.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js index b44fdcc9dc4..d83e85b2026 100644 --- a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js +++ b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js @@ -1,5 +1,3 @@ -/* eslint-disable class-methods-use-this, promise/catch-or-return */ - export default class PrometheusMetrics { constructor(wrapperSelector) { this.backOffRequestCounter = 0; @@ -24,6 +22,7 @@ export default class PrometheusMetrics { this.$panelToggle.on('click', e => this.handlePanelToggle(e)); } + /* eslint-disable class-methods-use-this */ handlePanelToggle(e) { const $toggleBtn = $(e.currentTarget); const $currentPanelBody = $toggleBtn.parents('.panel').find('.panel-body'); @@ -84,6 +83,10 @@ export default class PrometheusMetrics { this.$monitoredMetricsLoading.addClass('hidden'); this.$monitoredMetricsEmpty.removeClass('hidden'); } + }) + .catch(() => { + this.$monitoredMetricsLoading.addClass('hidden'); + this.$monitoredMetricsEmpty.removeClass('hidden'); }); } } -- cgit v1.2.1 From 4c58fd82df71e1a8a4389cbbb8a5ea3b0186291c Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 1 Jun 2017 15:05:27 +0530 Subject: Tests for `PrometheusMetrics` class --- spec/javascripts/prometheus_metrics/mock_data.js | 41 +++++++ .../prometheus_metrics/prometheus_metrics_spec.js | 125 +++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 spec/javascripts/prometheus_metrics/mock_data.js create mode 100644 spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js diff --git a/spec/javascripts/prometheus_metrics/mock_data.js b/spec/javascripts/prometheus_metrics/mock_data.js new file mode 100644 index 00000000000..3af56df92e2 --- /dev/null +++ b/spec/javascripts/prometheus_metrics/mock_data.js @@ -0,0 +1,41 @@ +export const metrics = [ + { + group: 'Kubernetes', + priority: 1, + active_metrics: 4, + metrics_missing_requirements: 0, + }, + { + group: 'HAProxy', + priority: 2, + active_metrics: 3, + metrics_missing_requirements: 0, + }, + { + group: 'Apache', + priority: 3, + active_metrics: 5, + metrics_missing_requirements: 0, + }, +]; + +export const missingVarMetrics = [ + { + group: 'Kubernetes', + priority: 1, + active_metrics: 4, + metrics_missing_requirements: 0, + }, + { + group: 'HAProxy', + priority: 2, + active_metrics: 3, + metrics_missing_requirements: 1, + }, + { + group: 'Apache', + priority: 3, + active_metrics: 5, + metrics_missing_requirements: 3, + }, +]; diff --git a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js new file mode 100644 index 00000000000..8b4f386af80 --- /dev/null +++ b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js @@ -0,0 +1,125 @@ +import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics'; +import { metrics, missingVarMetrics } from './mock_data'; + +describe('PrometheusMetrics', () => { + const FIXTURE = 'services/prometheus_service.html.raw'; + preloadFixtures(FIXTURE); + + beforeEach(() => { + loadFixtures(FIXTURE); + }); + + describe('constructor', () => { + let prometheusMetrics; + + beforeEach(() => { + prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); + }); + + it('should initialize wrapper element refs on class object', () => { + expect(prometheusMetrics.$wrapper).toBeDefined(); + expect(prometheusMetrics.$monitoredMetricsPanel).toBeDefined(); + expect(prometheusMetrics.$monitoredMetricsCount).toBeDefined(); + expect(prometheusMetrics.$monitoredMetricsLoading).toBeDefined(); + expect(prometheusMetrics.$monitoredMetricsEmpty).toBeDefined(); + expect(prometheusMetrics.$monitoredMetricsList).toBeDefined(); + expect(prometheusMetrics.$missingEnvVarPanel).toBeDefined(); + expect(prometheusMetrics.$panelToggle).toBeDefined(); + expect(prometheusMetrics.$missingEnvVarMetricCount).toBeDefined(); + expect(prometheusMetrics.$missingEnvVarMetricsList).toBeDefined(); + }); + + it('should initialize metadata on class object', () => { + expect(prometheusMetrics.backOffRequestCounter).toEqual(0); + expect(prometheusMetrics.activeMetricsEndpoint).toContain('/test'); + }); + }); + + describe('populateActiveMetrics', () => { + let prometheusMetrics; + + beforeEach(() => { + prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); + }); + + it('should show monitored metrics list', () => { + prometheusMetrics.populateActiveMetrics(metrics); + + const $metricsListLi = prometheusMetrics.$monitoredMetricsList.find('li'); + + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeFalsy(); + + expect(prometheusMetrics.$monitoredMetricsCount.text()).toEqual('12'); + expect($metricsListLi.length).toEqual(metrics.length); + expect($metricsListLi.first().find('.badge-count').text()).toEqual(`${metrics[0].active_metrics}`); + }); + + it('should show missing environment variables list', () => { + prometheusMetrics.populateActiveMetrics(missingVarMetrics); + + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$missingEnvVarPanel.hasClass('hidden')).toBeFalsy(); + + expect(prometheusMetrics.$missingEnvVarMetricCount.text()).toEqual('2'); + expect(prometheusMetrics.$missingEnvVarPanel.find('li').length).toEqual(2); + expect(prometheusMetrics.$missingEnvVarPanel.find('.flash-container')).toBeDefined(); + }); + }); + + describe('loadActiveMetrics', () => { + let prometheusMetrics; + + beforeEach(() => { + prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); + }); + + it('should show loader animation while response is being loaded and hide it when request is complete', (done) => { + const deferred = $.Deferred(); + spyOn($, 'getJSON').and.returnValue(deferred.promise()); + + prometheusMetrics.loadActiveMetrics(); + + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeFalsy(); + expect($.getJSON).toHaveBeenCalledWith(prometheusMetrics.activeMetricsEndpoint); + + deferred.resolve({ data: metrics, success: true }); + + setTimeout(() => { + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy(); + done(); + }); + }); + + it('should show empty state if response failed to load', (done) => { + const deferred = $.Deferred(); + spyOn($, 'getJSON').and.returnValue(deferred.promise()); + spyOn(prometheusMetrics, 'populateActiveMetrics'); + + prometheusMetrics.loadActiveMetrics(); + + deferred.reject(); + + setTimeout(() => { + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeFalsy(); + done(); + }); + }); + + it('should populate metrics list once response is loaded', (done) => { + const deferred = $.Deferred(); + spyOn($, 'getJSON').and.returnValue(deferred.promise()); + spyOn(prometheusMetrics, 'populateActiveMetrics'); + + prometheusMetrics.loadActiveMetrics(); + + deferred.resolve({ data: metrics, success: true }); + + setTimeout(() => { + expect(prometheusMetrics.populateActiveMetrics).toHaveBeenCalledWith(metrics); + done(); + }); + }); + }); +}); -- cgit v1.2.1 From 3f0eff82592f4a30abb6ffd15ac248a5f773c994 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 1 Jun 2017 21:24:56 +0530 Subject: Update as per review feedback --- .../javascripts/prometheus_metrics/constants.js | 5 +++ app/assets/javascripts/prometheus_metrics/index.js | 1 - .../prometheus_metrics/prometheus_metrics.js | 45 +++++++++++++++------- app/assets/stylesheets/pages/settings.scss | 30 +++++---------- .../projects/services/prometheus/_show.html.haml | 14 +++---- .../prometheus_metrics/prometheus_metrics_spec.js | 35 ++++++++++++++++- 6 files changed, 87 insertions(+), 43 deletions(-) create mode 100644 app/assets/javascripts/prometheus_metrics/constants.js diff --git a/app/assets/javascripts/prometheus_metrics/constants.js b/app/assets/javascripts/prometheus_metrics/constants.js new file mode 100644 index 00000000000..50f1248456e --- /dev/null +++ b/app/assets/javascripts/prometheus_metrics/constants.js @@ -0,0 +1,5 @@ +export default { + EMPTY: 'empty', + LOADING: 'loading', + LIST: 'list', +}; diff --git a/app/assets/javascripts/prometheus_metrics/index.js b/app/assets/javascripts/prometheus_metrics/index.js index 9f20161783b..a0c43c5abe1 100644 --- a/app/assets/javascripts/prometheus_metrics/index.js +++ b/app/assets/javascripts/prometheus_metrics/index.js @@ -2,6 +2,5 @@ import PrometheusMetrics from './prometheus_metrics'; $(() => { const prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); - prometheusMetrics.init(); prometheusMetrics.loadActiveMetrics(); }); diff --git a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js index d83e85b2026..ef4d6df5138 100644 --- a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js +++ b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js @@ -1,3 +1,5 @@ +import PANEL_STATE from './constants'; + export default class PrometheusMetrics { constructor(wrapperSelector) { this.backOffRequestCounter = 0; @@ -16,31 +18,48 @@ export default class PrometheusMetrics { this.$missingEnvVarMetricsList = this.$missingEnvVarPanel.find('.js-missing-var-metrics-list'); this.activeMetricsEndpoint = this.$monitoredMetricsPanel.data('active-metrics'); - } - init() { this.$panelToggle.on('click', e => this.handlePanelToggle(e)); } /* eslint-disable class-methods-use-this */ handlePanelToggle(e) { const $toggleBtn = $(e.currentTarget); - const $currentPanelBody = $toggleBtn.parents('.panel').find('.panel-body'); - if ($currentPanelBody.is(':visible')) { - $currentPanelBody.addClass('hidden'); + const $currentPanelBody = $toggleBtn.closest('.panel').find('.panel-body'); + $currentPanelBody.toggleClass('hidden'); + if ($toggleBtn.hasClass('fa-caret-down')) { $toggleBtn.removeClass('fa-caret-down').addClass('fa-caret-right'); } else { - $currentPanelBody.removeClass('hidden'); $toggleBtn.removeClass('fa-caret-right').addClass('fa-caret-down'); } } + showMonitoringMetricsPanelState(stateName) { + switch (stateName) { + case PANEL_STATE.LOADING: + this.$monitoredMetricsLoading.removeClass('hidden'); + this.$monitoredMetricsEmpty.addClass('hidden'); + this.$monitoredMetricsList.addClass('hidden'); + break; + case PANEL_STATE.LIST: + this.$monitoredMetricsLoading.addClass('hidden'); + this.$monitoredMetricsEmpty.addClass('hidden'); + this.$monitoredMetricsList.removeClass('hidden'); + break; + default: + this.$monitoredMetricsLoading.addClass('hidden'); + this.$monitoredMetricsEmpty.removeClass('hidden'); + this.$monitoredMetricsList.addClass('hidden'); + break; + } + } + populateActiveMetrics(metrics) { let totalMonitoredMetrics = 0; let totalMissingEnvVarMetrics = 0; metrics.forEach((metric) => { - this.$monitoredMetricsList.append(`
  • ${metric.group}${metric.active_metrics}
  • `); + this.$monitoredMetricsList.append(`
  • ${metric.group}${metric.active_metrics}
  • `); totalMonitoredMetrics += metric.active_metrics; if (metric.metrics_missing_requirements > 0) { this.$missingEnvVarMetricsList.append(`
  • ${metric.group}
  • `); @@ -49,17 +68,17 @@ export default class PrometheusMetrics { }); this.$monitoredMetricsCount.text(totalMonitoredMetrics); - this.$monitoredMetricsLoading.addClass('hidden'); - this.$monitoredMetricsList.removeClass('hidden'); + this.showMonitoringMetricsPanelState(PANEL_STATE.LIST); if (totalMissingEnvVarMetrics > 0) { this.$missingEnvVarPanel.removeClass('hidden'); + this.$missingEnvVarPanel.find('.flash-container').off('click'); this.$missingEnvVarMetricCount.text(totalMissingEnvVarMetrics); } } loadActiveMetrics() { - this.$monitoredMetricsLoading.removeClass('hidden'); + this.showMonitoringMetricsPanelState(PANEL_STATE.LOADING); gl.utils.backOff((next, stop) => { $.getJSON(this.activeMetricsEndpoint) .done((res) => { @@ -80,13 +99,11 @@ export default class PrometheusMetrics { if (res && res.data && res.data.length) { this.populateActiveMetrics(res.data); } else { - this.$monitoredMetricsLoading.addClass('hidden'); - this.$monitoredMetricsEmpty.removeClass('hidden'); + this.showMonitoringMetricsPanelState(PANEL_STATE.EMPTY); } }) .catch(() => { - this.$monitoredMetricsLoading.addClass('hidden'); - this.$monitoredMetricsEmpty.removeClass('hidden'); + this.showMonitoringMetricsPanelState(PANEL_STATE.EMPTY); }); } } diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index deed4501399..e2ed1239541 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -39,11 +39,11 @@ .prometheus-metrics-monitoring { .panel { .panel-toggle { - width: 12px; + width: 14px; + } - &.fa-caret-right { - padding-left: 4px; - } + .badge { + font-size: inherit; } .panel-heading .badge-count { @@ -55,14 +55,9 @@ padding: 0; } - .badge-count { - margin-left: 3px; - padding: 3px 8px; - border-radius: 40%; - } - .flash-container { margin-bottom: 0; + cursor: default; .flash-notice { border-radius: 0; @@ -80,25 +75,20 @@ margin-top: 10px; margin-bottom: 0; } - - p { - color: $gl-gray-light; - } } - .loading-metrics .fa-spin { + .loading-metrics .metrics-load-spinner { color: $loading-color; } .metrics-list { - list-style-type: none; - margin: 0; - padding: 0; + margin-bottom: 0; li { - padding: 16px; + padding: $gl-padding; - .badge-count { + .badge { + margin-left: 5px; background: $badge-bg; } } diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index 82d4dd2488f..f0bd8c8bee9 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -11,28 +11,28 @@ = link_to 'More information', '#' .col-lg-9 - .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{namespace_project_prometheus_active_metrics_path(@project.namespace, @project)}.json" } } + .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{namespace_project_prometheus_active_metrics_path(@project.namespace, @project, :json)}" } } .panel-heading %h3.panel-title Monitored - %span.badge-count.js-monitored-count 0 + %span.badge.js-monitored-count 0 .panel-body .loading-metrics.js-loading-metrics - = icon('spinner spin 3x') + = icon('spinner spin 3x', class: 'metrics-load-spinner') %p Finding and configuring metrics... .empty-metrics.hidden.js-empty-metrics = custom_icon('icon_empty_metrics') %p No metrics are being monitored. To start monitoring, deploy to an environment. = link_to project_environments_path(@project), title: 'View environments', class: 'btn btn-success' do View environments - %ul.metrics-list.hidden.js-metrics-list + %ul.list-unstyled.metrics-list.hidden.js-metrics-list .panel.panel-default.hidden.js-panel-missing-env-vars .panel-heading %h3.panel-title - = icon('caret-right lg', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel') + = icon('caret-right lg fw', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel') Missing environment variable - %span.badge-count.js-env-var-count 0 + %span.badge.js-env-var-count 0 .panel-body.hidden .flash-container .flash-notice @@ -42,4 +42,4 @@ $CI_ENVIRONMENT_SLUG to exporter’s queries. = link_to 'More information', '#' - %ul.metrics-list.js-missing-var-metrics-list + %ul.list-unstyled.metrics-list.js-missing-var-metrics-list diff --git a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js index 8b4f386af80..e7187a8a5e0 100644 --- a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js +++ b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js @@ -1,4 +1,5 @@ import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics'; +import PANEL_STATE from '~/prometheus_metrics/constants'; import { metrics, missingVarMetrics } from './mock_data'; describe('PrometheusMetrics', () => { @@ -35,6 +36,38 @@ describe('PrometheusMetrics', () => { }); }); + describe('showMonitoringMetricsPanelState', () => { + let prometheusMetrics; + + beforeEach(() => { + prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); + }); + + it('should show loading state when called with `loading`', () => { + prometheusMetrics.showMonitoringMetricsPanelState(PANEL_STATE.LOADING); + + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeFalsy(); + expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeTruthy(); + }); + + it('should show metrics list when called with `list`', () => { + prometheusMetrics.showMonitoringMetricsPanelState(PANEL_STATE.LIST); + + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeFalsy(); + }); + + it('should show empty state when called with `empty`', () => { + prometheusMetrics.showMonitoringMetricsPanelState(PANEL_STATE.EMPTY); + + expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy(); + expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeFalsy(); + expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeTruthy(); + }); + }); + describe('populateActiveMetrics', () => { let prometheusMetrics; @@ -52,7 +85,7 @@ describe('PrometheusMetrics', () => { expect(prometheusMetrics.$monitoredMetricsCount.text()).toEqual('12'); expect($metricsListLi.length).toEqual(metrics.length); - expect($metricsListLi.first().find('.badge-count').text()).toEqual(`${metrics[0].active_metrics}`); + expect($metricsListLi.first().find('.badge').text()).toEqual(`${metrics[0].active_metrics}`); }); it('should show missing environment variables list', () => { -- cgit v1.2.1 From 223abc6df005beba45a514b44af7227b224dadd3 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 2 Jun 2017 17:03:30 +0530 Subject: Use `.text-center` class instead of text-align prop --- app/assets/stylesheets/pages/settings.scss | 1 - app/views/projects/services/prometheus/_show.html.haml | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index e2ed1239541..7f39a99b59d 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -67,7 +67,6 @@ .loading-metrics, .empty-metrics { - text-align: center; padding: 30px 10px; p, diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index f0bd8c8bee9..c4ac384ca1a 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -17,10 +17,10 @@ Monitored %span.badge.js-monitored-count 0 .panel-body - .loading-metrics.js-loading-metrics + .loading-metrics.text-center.js-loading-metrics = icon('spinner spin 3x', class: 'metrics-load-spinner') %p Finding and configuring metrics... - .empty-metrics.hidden.js-empty-metrics + .empty-metrics.text-center.hidden.js-empty-metrics = custom_icon('icon_empty_metrics') %p No metrics are being monitored. To start monitoring, deploy to an environment. = link_to project_environments_path(@project), title: 'View environments', class: 'btn btn-success' do -- cgit v1.2.1 From e74896df0c7d0d88958a3d35b3144361cfdd0594 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 31 May 2017 19:36:07 +0200 Subject: Matched Metrics tests --- config/additional_metrics.yml | 14 ++++-- lib/gitlab/prometheus/metric.rb | 21 ++++----- lib/gitlab/prometheus/metric_group.rb | 2 +- .../prometheus/queries/matched_metrics_query.rb | 49 ++++++++++++--------- .../queries/matched_metrics_query_spec.rb | 50 ++++++++++++++++++++++ 5 files changed, 97 insertions(+), 39 deletions(-) create mode 100644 spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb diff --git a/config/additional_metrics.yml b/config/additional_metrics.yml index 59ca387f055..d84db1530a8 100644 --- a/config/additional_metrics.yml +++ b/config/additional_metrics.yml @@ -3,25 +3,31 @@ metrics: - title: "Memory usage" y_label: "Values" - detect: container_memory_usage_bytes + required_metrics: + - container_memory_usage_bytes weight: 1 queries: - query_range: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' label: Container memory + display_empty: true unit: MiB - title: "Current memory usage" - detect: container_memory_usage_bytes + required_metrics: + - container_memory_usage_bytes weight: 1 queries: - query: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' + display_empty: false unit: MiB - title: "CPU usage" - detect: container_cpu_usage_seconds_total + required_metrics: + - container_cpu_usage_seconds_total weight: 1 queries: - query_range: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' - title: "Current CPU usage" - detect: container_cpu_usage_seconds_total + required_metrics: + - container_cpu_usage_seconds_total weight: 1 queries: - query: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' diff --git a/lib/gitlab/prometheus/metric.rb b/lib/gitlab/prometheus/metric.rb index 777cf030ceb..d7cd4237a7b 100644 --- a/lib/gitlab/prometheus/metric.rb +++ b/lib/gitlab/prometheus/metric.rb @@ -1,29 +1,24 @@ module Gitlab::Prometheus class Metric - attr_reader :group, :title, :detect, :weight, :y_label, :queries + attr_reader :group, :title, :required_metrics, :weight, :y_label, :queries - def initialize(group, title, detect, weight, y_label, queries = []) - @group = group + def initialize(title, required_metrics, weight, y_label, queries = []) @title = title - @detect = detect + @required_metrics = required_metrics @weight = weight @y_label = y_label || 'Values' @queries = queries end - def self.metric_from_entry(group, entry) - missing_fields = [:title, :detect, :weight, :queries].select { |key| !entry.has_key?(key) } + def self.metric_from_entry(entry) + missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.has_key?(key) } raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? - Metric.new(group, entry[:title], entry[:detect], entry[:weight], entry[:y_label],entry[:queries]) + Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries]) end - def self.metrics_from_list(group, list) - list.map { |entry| metric_from_entry(group, entry) } - end - - def self.additional_metrics_raw - @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml')).map(&:deep_symbolize_keys) + def self.metrics_from_list(list) + list.map { |entry| metric_from_entry(entry) } end end end diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb index 9f95525bc0c..0dcd9dc0f36 100644 --- a/lib/gitlab/prometheus/metric_group.rb +++ b/lib/gitlab/prometheus/metric_group.rb @@ -22,7 +22,7 @@ module Gitlab::Prometheus raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? group = MetricGroup.new(entry[:group], entry[:priority]) - group.metrics = Metric.metrics_from_list(group, entry[:metrics]) + group.metrics = Metric.metrics_from_list(entry[:metrics]) group end diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metrics_query.rb index 5a8b0f6c701..a5e1f6a3fde 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metrics_query.rb @@ -16,31 +16,28 @@ module Gitlab::Prometheus::Queries private def groups_data - metrics_series = metrics_with_series(Gitlab::Prometheus::MetricGroup.all) - lookup = active_series_lookup(metrics_series) + metrics_groups = groups_with_active_metrics(Gitlab::Prometheus::MetricGroup.all) + lookup = active_series_lookup(metrics_groups) groups = {} - metrics_series.each do |metrics, series| - groups[metrics.group] ||= { active_metrics: 0, metrics_missing_requirements: 0 } - group = groups[metrics.group] + metrics_groups.each do |group| + groups[group] ||= { active_metrics: 0, metrics_missing_requirements: 0 } + metrics = group.metrics.flat_map(&:required_metrics) + active_metrics = metrics.count(&lookup.method(:has_key?)) - if series.all?(&lookup.method(:has_key?)) - group[:active_metrics] += 1 - else - group[:metrics_missing_requirements] += 1 - end - group + groups[group][:active_metrics] += active_metrics + groups[group][:metrics_missing_requirements] += metrics.count - active_metrics end groups end - def active_series_lookup(metrics) + def active_series_lookup(metric_groups) timeframe_start = 8.hours.ago timeframe_end = Time.now - series = metrics.flat_map { |metrics, series| series }.uniq + series = metric_groups.flat_map(&:metrics).flat_map(&:required_metrics).uniq lookup = series.each_slice(MAX_QUERY_ITEMS).flat_map do |batched_series| client_series(*batched_series, start: timeframe_start, stop: timeframe_end) @@ -54,17 +51,27 @@ module Gitlab::Prometheus::Queries series_info.has_key?('environment') end - def metrics_with_series(metric_groups) - label_values = client_label_values || [] + def available_metrics + @available_metrics ||= client_label_values || [] + end - metrics = metric_groups.flat_map do |group| - group.metrics.map do |metric| - matcher = Regexp.compile(metric.detect) - [metric, label_values.select(&matcher.method(:match))] - end + def filter_active_metrics(metric_group) + metric_group.metrics.select! do |metric| + metric.required_metrics.all?(&available_metrics.method(:include?)) end + metric_group + end - metrics.select { |metric, labels| labels&.any? } + def groups_with_active_metrics(metric_groups) + metric_groups.map(&method(:filter_active_metrics)).select { |group| group.metrics.any? } + end + + def metrics_with_required_series(metric_groups) + metric_groups.flat_map do |group| + group.metrics.select do |metric| + metric.required_metrics.all?(&available_metrics.method(:include?)) + end + end end end end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb new file mode 100644 index 00000000000..0aee9d37889 --- /dev/null +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do + let(:environment) { create(:environment, slug: 'environment-slug') } + let(:deployment) { create(:deployment, environment: environment) } + + let(:client) { double('prometheus_client') } + subject { described_class.new(client) } + + around do |example| + time_without_subsecond_values = Time.local(2008, 9, 1, 12, 0, 0) + Timecop.freeze(time_without_subsecond_values) { example.run } + end + + let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } + let(:metric_class) { Gitlab::Prometheus::Metric } + + let(:simple_metrics) do + [ + metric_class.new('title', ['metrica', 'metricb'], '1', 'y_label', [{ :query_range => 'avg' }]) + ] + end + + let(:simple_metric_group) do + metric_group_class.new('name', 1, simple_metrics) + end + + let(:xx) do + [{ + '__name__': 'metrica', + 'environment': 'mattermost' + }, + { + '__name__': 'metricb', + 'environment': 'mattermost' + }] + end + + before do + allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + + allow(client).to receive(:label_values).and_return(['metrica', 'metricb']) + allow(client).to receive(:series).and_return(xx) + end + + it "something something" do + + expect(subject.query).to eq("asf") + end +end -- cgit v1.2.1 From 6a70509a2763717e592c603249855bfb43519d2f Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 2 Jun 2017 23:32:59 +0200 Subject: Towards Reviewable prometheus --- .../prometheus/queries/matched_metrics_query.rb | 7 +- spec/db/production/settings.rb.orig | 16 +++ .../queries/matched_metrics_query_spec.rb | 125 ++++++++++++++++----- .../prometheus/matched_metrics_query_helper.rb | 33 ++++++ 4 files changed, 146 insertions(+), 35 deletions(-) create mode 100644 spec/db/production/settings.rb.orig create mode 100644 spec/support/prometheus/matched_metrics_query_helper.rb diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metrics_query.rb index a5e1f6a3fde..fc97bca486c 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metrics_query.rb @@ -23,11 +23,10 @@ module Gitlab::Prometheus::Queries metrics_groups.each do |group| groups[group] ||= { active_metrics: 0, metrics_missing_requirements: 0 } - metrics = group.metrics.flat_map(&:required_metrics) - active_metrics = metrics.count(&lookup.method(:has_key?)) + active_metrics = group.metrics.count { |metric| metric.required_metrics.all?(&lookup.method(:has_key?)) } groups[group][:active_metrics] += active_metrics - groups[group][:metrics_missing_requirements] += metrics.count - active_metrics + groups[group][:metrics_missing_requirements] += group.metrics.count - active_metrics end groups @@ -48,7 +47,7 @@ module Gitlab::Prometheus::Queries end def has_matching_label(series_info) - series_info.has_key?('environment') + series_info.key?('environment') end def available_metrics diff --git a/spec/db/production/settings.rb.orig b/spec/db/production/settings.rb.orig new file mode 100644 index 00000000000..3cbb173c4cc --- /dev/null +++ b/spec/db/production/settings.rb.orig @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe 'seed production settings', lib: true do + include StubENV + + context 'GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN is set in the environment' do + before do + stub_env('GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN', '013456789') + end + + it 'writes the token to the database' do + load(File.join(__dir__, '../../../db/fixtures/production/010_settings.rb')) + expect(ApplicationSetting.current.runners_registration_token).to eq('013456789') + end + end +end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb index 0aee9d37889..d46de56f520 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -1,50 +1,113 @@ require 'spec_helper' describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do - let(:environment) { create(:environment, slug: 'environment-slug') } - let(:deployment) { create(:deployment, environment: environment) } + include Prometheus::MatchedMetricsQueryHelper + + let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } + let(:metric_class) { Gitlab::Prometheus::Metric } let(:client) { double('prometheus_client') } + subject { described_class.new(client) } - around do |example| - time_without_subsecond_values = Time.local(2008, 9, 1, 12, 0, 0) - Timecop.freeze(time_without_subsecond_values) { example.run } - end + context 'with one group where two metrics are found' do + before do + allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(client).to receive(:label_values).and_return(metric_names) + end - let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } - let(:metric_class) { Gitlab::Prometheus::Metric } + context 'both metrics in the group pass requirements' do + before do + allow(client).to receive(:series).and_return(series_info_with_environment) + end - let(:simple_metrics) do - [ - metric_class.new('title', ['metrica', 'metricb'], '1', 'y_label', [{ :query_range => 'avg' }]) - ] - end + it 'responds with both metrics as actve' do + expect(subject.query).to eq([{ group: 'name', priority: 1, active_metrics: 2, metrics_missing_requirements: 0 }]) + end + end - let(:simple_metric_group) do - metric_group_class.new('name', 1, simple_metrics) - end + context 'none of the metrics pass requirements' do + before do + allow(client).to receive(:series).and_return(series_info_without_environment) + end - let(:xx) do - [{ - '__name__': 'metrica', - 'environment': 'mattermost' - }, - { - '__name__': 'metricb', - 'environment': 'mattermost' - }] + it 'responds with both metrics missing requirements' do + expect(subject.query).to eq([{ group: 'name', priority: 1, active_metrics: 0, metrics_missing_requirements: 2 }]) + end + end + + context 'no series information found about the metrics' do + before do + allow(client).to receive(:series).and_return(empty_series_info) + end + + it 'responds with both metrics missing requirements' do + expect(subject.query).to eq([{ group: 'name', priority: 1, active_metrics: 0, metrics_missing_requirements: 2 }]) + end + end + + context 'one of the series info was not found' do + before do + allow(client).to receive(:series).and_return(partialy_empty_series_info) + end + it 'responds with one active and one missing metric' do + expect(subject.query).to eq([{ group: 'name', priority: 1, active_metrics: 1, metrics_missing_requirements: 1 }]) + end + end end - before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + context 'with one group where only one metric is found' do + before do + allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(client).to receive(:label_values).and_return('metric_a') + end + + context 'both metrics in the group pass requirements' do + before do + allow(client).to receive(:series).and_return(series_info_with_environment) + end + + it 'responds with one metrics as active and no missing requiremens' do + expect(subject.query).to eq([{ group: 'name', priority: 1, active_metrics: 1, metrics_missing_requirements: 0 }]) + end + end - allow(client).to receive(:label_values).and_return(['metrica', 'metricb']) - allow(client).to receive(:series).and_return(xx) + context 'no metrics in group pass requirements' do + before do + allow(client).to receive(:series).and_return(series_info_without_environment) + end + + it 'responds with one metrics as active and no missing requiremens' do + expect(subject.query).to eq([{ group: 'name', priority: 1, active_metrics: 0, metrics_missing_requirements: 1 }]) + end + end end - it "something something" do + context 'with two groups where only one metric is found' do + before do + allow(metric_group_class).to receive(:all).and_return([simple_metric_group, + simple_metric_group('nameb', simple_metrics('metric_c'))]) + allow(client).to receive(:label_values).and_return('metric_c') + end + + context 'both metrics in the group pass requirements' do + before do + allow(client).to receive(:series).and_return(series_info_with_environment('metric_c')) + end + + it 'responds with one metrics as active and no missing requiremens' do + expect(subject.query).to eq([{ group: 'nameb', priority: 1, active_metrics: 1, metrics_missing_requirements: 0 }]) + end + end + + context 'no metris in group pass requirements' do + before do + allow(client).to receive(:series).and_return(series_info_without_environment) + end - expect(subject.query).to eq("asf") + it 'responds with one metrics as active and no missing requiremens' do + expect(subject.query).to eq([{ group: 'nameb', priority: 1, active_metrics: 0, metrics_missing_requirements: 1 }]) + end + end end end diff --git a/spec/support/prometheus/matched_metrics_query_helper.rb b/spec/support/prometheus/matched_metrics_query_helper.rb new file mode 100644 index 00000000000..ecaf85e3338 --- /dev/null +++ b/spec/support/prometheus/matched_metrics_query_helper.rb @@ -0,0 +1,33 @@ +module Prometheus + module MatchedMetricsQueryHelper + def metric_names + %w{metric_a metric_b} + end + + def simple_metrics(metric_name = 'metric_a') + [metric_class.new('title', %W(#{metric_name} metric_b), nil, nil), + metric_class.new('title', [metric_name], nil, nil)] + end + + def simple_metric_group(name = 'name', metrics = simple_metrics) + metric_group_class.new(name, 1, metrics) + end + + def series_info_with_environment(*more_metrics) + %w{metric_a metric_b}.concat(more_metrics).map { |metric_name| { '__name__' => metric_name, 'environment' => '' } } + end + + def series_info_without_environment + [{ '__name__' => 'metric_a' }, + { '__name__' => 'metric_b' }] + end + + def partialy_empty_series_info + [{ '__name__' => 'metric_a', 'environment' => '' }] + end + + def empty_series_info + [] + end + end +end -- cgit v1.2.1 From ae5268ce8cc533be4086a11d9d89fa726136d59d Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 11:32:56 +0200 Subject: Additional Metrics tests --- config/additional_metrics.yml | 1 - .../prometheus/queries/additional_metrics_query.rb | 35 +++++++++------ .../queries/additional_metrics_query_spec.rb | 49 +++++++++++++++++++++ .../queries/matched_metrics_query_spec.rb | 2 +- .../prometheus/additional_metrics_query_helper.rb | 51 ++++++++++++++++++++++ .../prometheus/matched_metrics_query_helper.rb | 6 +-- 6 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb create mode 100644 spec/support/prometheus/additional_metrics_query_helper.rb diff --git a/config/additional_metrics.yml b/config/additional_metrics.yml index d84db1530a8..daecde49570 100644 --- a/config/additional_metrics.yml +++ b/config/additional_metrics.yml @@ -9,7 +9,6 @@ queries: - query_range: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' label: Container memory - display_empty: true unit: MiB - title: "Current memory usage" required_metrics: diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb index fd7f072834d..d21a3978e25 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_query.rb @@ -17,26 +17,42 @@ module Gitlab::Prometheus::Queries def query_metrics(query_context) query_processor = method(:process_query).curry[query_context] - matched_metrics.map do |group| + groups = matched_metrics.map do |group| metrics = group.metrics.map do |metric| { title: metric.title, weight: metric.weight, y_label: metric.y_label, - queries: metric.queries.map(&query_processor) + queries: metric.queries.map(&query_processor).select(&method(:query_with_result)) } end { group: group.name, priority: group.priority, - metrics: metrics + metrics: metrics.select(&method(:metric_with_any_queries)) } end + + groups.select(&method(:group_with_any_metrics)) end private + def metric_with_any_queries(metric) + metric[:queries]&.count > 0 + end + + def group_with_any_metrics(group) + group[:metrics]&.count > 0 + end + + def query_with_result(query) + query[:result]&.any? do |item| + item&.[](:values)&.any? || item&.[](:value)&.any? + end + end + def process_query(context, query) query_with_result = query.dup query_with_result[:result] = @@ -48,22 +64,15 @@ module Gitlab::Prometheus::Queries query_with_result end - def process_result(query_result) - contains_metrics = query_result.all? do |item| - item&.[](:values)&.any? || item&.[](:value)&.any? - end - contains_metrics + def available_metrics + @available_metrics ||= client_label_values || [] end def matched_metrics - label_values = client_label_values || [] - Gitlab::Prometheus::MetricGroup.all - result = Gitlab::Prometheus::MetricGroup.all.map do |group| group.metrics.select! do |metric| - matcher = Regexp.compile(metric.detect) - label_values.any? &matcher.method(:match) + metric.required_metrics.all?(&available_metrics.method(:include?)) end group end diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb new file mode 100644 index 00000000000..c7e2dbc12ec --- /dev/null +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do + include Prometheus::AdditionalMetricsQueryHelper + + let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } + let(:metric_class) { Gitlab::Prometheus::Metric } + + let(:client) { double('prometheus_client') } + let(:environment) { create(:environment, slug: 'environment-slug') } + + subject(:query_result) { described_class.new(client).query(environment.id) } + + + context 'with one group where two metrics is found' do + before do + allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(client).to receive(:label_values).and_return(metric_names) + end + + context 'some querie return results' do + before do + expect(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + expect(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) + expect(client).to receive(:query_range).with('query_range_empty', any_args).and_return([]) + end + + it 'return results only for queries with results' do + puts query_result + expected = { + group: 'name', + priority: 1, + metrics: + [ + { + title: 'title', weight: nil, y_label: 'Values', queries: + [ + { query_range: 'query_range_a', result: query_range_result }, + { query_range: 'query_range_b', label: 'label', unit: 'unit', result: query_range_result } + ] + } + ] + } + + expect(query_result).to eq([expected]) + end + end + end +end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb index d46de56f520..390fff568cc 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do subject { described_class.new(client) } - context 'with one group where two metrics are found' do + context 'with one group where two metrics is found' do before do allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) allow(client).to receive(:label_values).and_return(metric_names) diff --git a/spec/support/prometheus/additional_metrics_query_helper.rb b/spec/support/prometheus/additional_metrics_query_helper.rb new file mode 100644 index 00000000000..d80beb066ff --- /dev/null +++ b/spec/support/prometheus/additional_metrics_query_helper.rb @@ -0,0 +1,51 @@ +module Prometheus + module AdditionalMetricsQueryHelper + def metric_names + %w{metric_a metric_b} + end + + def simple_queries + [{ query_range: 'query_range_a' }, { query_range: 'query_range_b', label: 'label', unit: 'unit' }] + end + + def simple_query(suffix = 'a') + [{ query_range: "query_range_#{suffix}" }] + end + + def simple_metrics + [ + Gitlab::Prometheus::Metric.new('title', %w(metric_a metric_b), nil, nil, simple_queries), + Gitlab::Prometheus::Metric.new('title', %w{metric_a}, nil, nil, simple_query('empty')), + Gitlab::Prometheus::Metric.new('title', %w{metric_c}, nil, nil) + ] + end + + def simple_metric_group(name = 'name', metrics = simple_metrics) + Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) + end + + def query_result + [ + { + 'metric': {}, + 'value': [ + 1488772511.004, + '0.000041021495238095323' + ] + } + ] + end + + def query_range_result + [ + { + 'metric': {}, + 'values': [ + [1488758662.506, '0.00002996364761904785'], + [1488758722.506, '0.00003090239047619091'] + ] + } + ] + end + end +end diff --git a/spec/support/prometheus/matched_metrics_query_helper.rb b/spec/support/prometheus/matched_metrics_query_helper.rb index ecaf85e3338..86e874fb295 100644 --- a/spec/support/prometheus/matched_metrics_query_helper.rb +++ b/spec/support/prometheus/matched_metrics_query_helper.rb @@ -5,12 +5,12 @@ module Prometheus end def simple_metrics(metric_name = 'metric_a') - [metric_class.new('title', %W(#{metric_name} metric_b), nil, nil), - metric_class.new('title', [metric_name], nil, nil)] + [Gitlab::Prometheus::Metric.new('title', %W(#{metric_name} metric_b), nil, nil), + Gitlab::Prometheus::Metric.new('title', [metric_name], nil, nil)] end def simple_metric_group(name = 'name', metrics = simple_metrics) - metric_group_class.new(name, 1, metrics) + Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) end def series_info_with_environment(*more_metrics) -- cgit v1.2.1 From eaaad702deab2ff73cc204d55056745bf34c703e Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 13:33:11 +0200 Subject: Additional metrics test using multiple groups --- .../queries/additional_metrics_query_spec.rb | 96 +++++++++++++++++++--- .../prometheus/additional_metrics_query_helper.rb | 20 ----- spec/support/prometheus/metric_builders.rb | 27 ++++++ 3 files changed, 111 insertions(+), 32 deletions(-) create mode 100644 spec/support/prometheus/metric_builders.rb diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb index c7e2dbc12ec..617028cde37 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do include Prometheus::AdditionalMetricsQueryHelper + include Prometheus::MetricBuilders let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } let(:metric_class) { Gitlab::Prometheus::Metric } @@ -11,6 +12,9 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do subject(:query_result) { described_class.new(client).query(environment.id) } + around do |example| + Timecop.freeze { example.run } + end context 'with one group where two metrics is found' do before do @@ -18,31 +22,99 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do allow(client).to receive(:label_values).and_return(metric_names) end - context 'some querie return results' do + context 'some queries return results' do before do expect(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) expect(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) expect(client).to receive(:query_range).with('query_range_empty', any_args).and_return([]) end + it 'return results only for queries with results' do + expected = [ + { + group: 'name', + priority: 1, + metrics: [ + { + title: 'title', weight: nil, y_label: 'Values', queries: [ + { query_range: 'query_range_a', result: query_range_result }, + { query_range: 'query_range_b', label: 'label', unit: 'unit', result: query_range_result } + ] + } + ] + } + ] + + expect(query_result).to eq(expected) + end + end + end + + context 'with two groups with one metric each' do + let(:metrics) { [simple_metric(queries: [simple_query])] } + before do + allow(metric_group_class).to receive(:all).and_return( + [ + simple_metric_group('group_a', [simple_metric(queries: [simple_query])]), + simple_metric_group('group_b', [simple_metric(title: 'title_b', queries: [simple_query('b')])]) + ]) + allow(client).to receive(:label_values).and_return(metric_names) + end + + context 'some queries return results' do + before do + expect(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + expect(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) + end + it 'return results only for queries with results' do puts query_result - expected = { - group: 'name', - priority: 1, - metrics: - [ + expected = [ + { + group: 'group_a', + priority: 1, + metrics: [ + { + title: 'title', + weight: nil, + y_label: 'Values', + queries: [ + { + query_range: 'query_range_a', + result: [ + { + metric: {}, + values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] } + ] + } + ] + } + ] + }, + { + group: 'group_b', + priority: 1, + metrics: [ { - title: 'title', weight: nil, y_label: 'Values', queries: - [ - { query_range: 'query_range_a', result: query_range_result }, - { query_range: 'query_range_b', label: 'label', unit: 'unit', result: query_range_result } + title: 'title_b', + weight: nil, + y_label: 'Values', + queries: [ + { + query_range: 'query_range_b', result: [ + { + metric: {}, + values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] + } + ] + } ] } ] - } + } + ] - expect(query_result).to eq([expected]) + expect(query_result).to eq(expected) end end end diff --git a/spec/support/prometheus/additional_metrics_query_helper.rb b/spec/support/prometheus/additional_metrics_query_helper.rb index d80beb066ff..84cbc24a301 100644 --- a/spec/support/prometheus/additional_metrics_query_helper.rb +++ b/spec/support/prometheus/additional_metrics_query_helper.rb @@ -4,26 +4,6 @@ module Prometheus %w{metric_a metric_b} end - def simple_queries - [{ query_range: 'query_range_a' }, { query_range: 'query_range_b', label: 'label', unit: 'unit' }] - end - - def simple_query(suffix = 'a') - [{ query_range: "query_range_#{suffix}" }] - end - - def simple_metrics - [ - Gitlab::Prometheus::Metric.new('title', %w(metric_a metric_b), nil, nil, simple_queries), - Gitlab::Prometheus::Metric.new('title', %w{metric_a}, nil, nil, simple_query('empty')), - Gitlab::Prometheus::Metric.new('title', %w{metric_c}, nil, nil) - ] - end - - def simple_metric_group(name = 'name', metrics = simple_metrics) - Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) - end - def query_result [ { diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb new file mode 100644 index 00000000000..2d54ecdfb5c --- /dev/null +++ b/spec/support/prometheus/metric_builders.rb @@ -0,0 +1,27 @@ +module Prometheus + module MetricBuilders + def simple_query(suffix = 'a', **opts) + { query_range: "query_range_#{suffix}" }.merge(opts) + end + + def simple_queries + [simple_query, simple_query('b', label: 'label', unit: 'unit')] + end + + def simple_metric(title: 'title', required_metrics: [], queries: []) + Gitlab::Prometheus::Metric.new(title, required_metrics, nil, nil, queries) + end + + def simple_metrics + [ + simple_metric(required_metrics: %w(metric_a metric_b), queries: simple_queries), + simple_metric(required_metrics: %w{metric_a}, queries: [simple_query('empty')]), + simple_metric(required_metrics: %w{metric_c}) + ] + end + + def simple_metric_group(name = 'name', metrics = simple_metrics) + Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) + end + end +end \ No newline at end of file -- cgit v1.2.1 From cf4aeafa6f3baaec7652f486cd04b7170dde9fbf Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 14:42:53 +0200 Subject: Test Partial additional query response --- .../queries/additional_metrics_query_spec.rb | 54 ++++++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb index 617028cde37..2291c4d67bb 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb @@ -24,12 +24,12 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do context 'some queries return results' do before do - expect(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) - expect(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) - expect(client).to receive(:query_range).with('query_range_empty', any_args).and_return([]) + allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_empty', any_args).and_return([]) end - it 'return results only for queries with results' do + it 'return group data only for queries with results' do expected = [ { group: 'name', @@ -61,14 +61,13 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do allow(client).to receive(:label_values).and_return(metric_names) end - context 'some queries return results' do + context 'both queries return results' do before do - expect(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) - expect(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) end - it 'return results only for queries with results' do - puts query_result + it 'return group data both queries' do expected = [ { group: 'group_a', @@ -117,5 +116,42 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do expect(query_result).to eq(expected) end end + + context 'one query returns result' do + before do + allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_b', any_args).and_return([]) + end + + it 'queries using specific time' do + expect(client).to receive(:query_range).with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f) + + expect(query_result).not_to be_nil + end + + it 'return group data only for query with results' do + expected = [ + { + group: 'group_a', + priority: 1, + metrics: [ + { + title: 'title', + weight: nil, + y_label: 'Values', + queries: [ + { + query_range: 'query_range_a', + result: query_range_result + } + ] + } + ] + } + ] + + expect(query_result).to eq(expected) + end + end end end -- cgit v1.2.1 From 223d07b38315a68fe39df90a7675f043d8b7acaf Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 18:38:09 +0200 Subject: Environments#additional_metrics tests --- .../projects/environments_controller_spec.rb | 40 ++++++++++ spec/models/environment_spec.rb | 93 ++++++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 20f99b209eb..12efdc700c5 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -311,6 +311,46 @@ describe Projects::EnvironmentsController do end end + describe 'GET #additional_metrics' do + before do + allow(controller).to receive(:environment).and_return(environment) + end + + context 'when environment has no metrics' do + before do + expect(environment).to receive(:additional_metrics).and_return(nil) + end + + context 'when requesting metrics as JSON' do + it 'returns a metrics JSON document' do + get :additional_metrics, environment_params(format: :json) + + expect(response).to have_http_status(204) + expect(json_response).to eq({}) + end + end + end + + context 'when environment has some metrics' do + before do + expect(environment).to receive(:additional_metrics).and_return({ + success: true, + data: {}, + last_update: 42 + }) + end + + it 'returns a metrics JSON document' do + get :additional_metrics, environment_params(format: :json) + + expect(response).to be_ok + expect(json_response['success']).to be(true) + expect(json_response['data']).to eq({}) + expect(json_response['last_update']).to eq(42) + end + end + end + def environment_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project, diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12519de8636..12de9c9111b 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -409,6 +409,99 @@ describe Environment, models: true do end end + describe '#has_metrics?' do + subject { environment.has_metrics? } + + context 'when the enviroment is available' do + context 'with a deployment service' do + let(:project) { create(:prometheus_project) } + + context 'and a deployment' do + let!(:deployment) { create(:deployment, environment: environment) } + it { is_expected.to be_truthy } + end + + context 'but no deployments' do + it { is_expected.to be_falsy } + end + end + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + end + + context 'when the environment is unavailable' do + let(:project) { create(:prometheus_project) } + + before do + environment.stop + end + + it { is_expected.to be_falsy } + end + end + + describe '#additional_metrics' do + let(:project) { create(:prometheus_project) } + subject { environment.additional_metrics } + + context 'when the environment has additional metrics' do + before do + allow(environment).to receive(:has_additional_metrics?).and_return(true) + end + + it 'returns the additional metrics from the deployment service' do + expect(project.monitoring_service).to receive(:reactive_query) + .with(Gitlab::Prometheus::Queries::AdditionalMetricsQuery.name, environment.id) + .and_return(:fake_metrics) + + is_expected.to eq(:fake_metrics) + end + end + + context 'when the environment does not have metrics' do + before do + allow(environment).to receive(:has_additional_metrics?).and_return(false) + end + + it { is_expected.to be_nil } + end + end + + describe '#has_additional_metrics??' do + subject { environment.has_metrics? } + + context 'when the enviroment is available' do + context 'with a deployment service' do + let(:project) { create(:prometheus_project) } + + context 'and a deployment' do + let!(:deployment) { create(:deployment, environment: environment) } + it { is_expected.to be_truthy } + end + + context 'but no deployments' do + it { is_expected.to be_falsy } + end + end + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + end + + context 'when the environment is unavailable' do + let(:project) { create(:prometheus_project) } + + before do + environment.stop + end + + it { is_expected.to be_falsy } + end + end + describe '#slug' do it "is automatically generated" do expect(environment.slug).not_to be_nil -- cgit v1.2.1 From eccc187a70a6745ca02211648814c89ff390dfa3 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 19:26:46 +0200 Subject: Additional Metrics of deployment tests --- .../additional_metrics_deployment_query_spec.rb | 29 +++++ .../queries/additional_metrics_query_spec.rb | 143 +-------------------- .../additional_metrics_shared_examples.rb | 143 +++++++++++++++++++++ spec/support/prometheus/metric_builders.rb | 6 +- 4 files changed, 179 insertions(+), 142 deletions(-) create mode 100644 spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb create mode 100644 spec/support/prometheus/additional_metrics_shared_examples.rb diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb new file mode 100644 index 00000000000..93a9ce38d80 --- /dev/null +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery, lib: true do + include Prometheus::AdditionalMetricsQueryHelper + include Prometheus::MetricBuilders + + let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } + let(:metric_class) { Gitlab::Prometheus::Metric } + + let(:client) { double('prometheus_client') } + let(:environment) { create(:environment, slug: 'environment-slug') } + let(:deployment) { create(:deployment, environment: environment) } + + subject(:query_result) { described_class.new(client).query(deployment.id) } + + around do |example| + Timecop.freeze { example.run } + end + + include_examples 'additional metrics query' do + it 'queries using specific time' do + expect(client).to receive(:query_range).with(anything, + start: (deployment.created_at - 30.minutes).to_f, + stop: (deployment.created_at + 30.minutes).to_f) + + expect(query_result).not_to be_nil + end + end +end diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb index 2291c4d67bb..e5db1326597 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb @@ -4,9 +4,6 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do include Prometheus::AdditionalMetricsQueryHelper include Prometheus::MetricBuilders - let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } - let(:metric_class) { Gitlab::Prometheus::Metric } - let(:client) { double('prometheus_client') } let(:environment) { create(:environment, slug: 'environment-slug') } @@ -16,142 +13,10 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do Timecop.freeze { example.run } end - context 'with one group where two metrics is found' do - before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) - allow(client).to receive(:label_values).and_return(metric_names) - end - - context 'some queries return results' do - before do - allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) - allow(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) - allow(client).to receive(:query_range).with('query_range_empty', any_args).and_return([]) - end - - it 'return group data only for queries with results' do - expected = [ - { - group: 'name', - priority: 1, - metrics: [ - { - title: 'title', weight: nil, y_label: 'Values', queries: [ - { query_range: 'query_range_a', result: query_range_result }, - { query_range: 'query_range_b', label: 'label', unit: 'unit', result: query_range_result } - ] - } - ] - } - ] - - expect(query_result).to eq(expected) - end - end - end - - context 'with two groups with one metric each' do - let(:metrics) { [simple_metric(queries: [simple_query])] } - before do - allow(metric_group_class).to receive(:all).and_return( - [ - simple_metric_group('group_a', [simple_metric(queries: [simple_query])]), - simple_metric_group('group_b', [simple_metric(title: 'title_b', queries: [simple_query('b')])]) - ]) - allow(client).to receive(:label_values).and_return(metric_names) - end - - context 'both queries return results' do - before do - allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) - allow(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) - end - - it 'return group data both queries' do - expected = [ - { - group: 'group_a', - priority: 1, - metrics: [ - { - title: 'title', - weight: nil, - y_label: 'Values', - queries: [ - { - query_range: 'query_range_a', - result: [ - { - metric: {}, - values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] } - ] - } - ] - } - ] - }, - { - group: 'group_b', - priority: 1, - metrics: [ - { - title: 'title_b', - weight: nil, - y_label: 'Values', - queries: [ - { - query_range: 'query_range_b', result: [ - { - metric: {}, - values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] - } - ] - } - ] - } - ] - } - ] - - expect(query_result).to eq(expected) - end - end - - context 'one query returns result' do - before do - allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) - allow(client).to receive(:query_range).with('query_range_b', any_args).and_return([]) - end - - it 'queries using specific time' do - expect(client).to receive(:query_range).with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f) - - expect(query_result).not_to be_nil - end - - it 'return group data only for query with results' do - expected = [ - { - group: 'group_a', - priority: 1, - metrics: [ - { - title: 'title', - weight: nil, - y_label: 'Values', - queries: [ - { - query_range: 'query_range_a', - result: query_range_result - } - ] - } - ] - } - ] - - expect(query_result).to eq(expected) - end + include_examples 'additional metrics query' do + it 'queries using specific time' do + expect(client).to receive(:query_range).with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f) + expect(query_result).not_to be_nil end end end diff --git a/spec/support/prometheus/additional_metrics_shared_examples.rb b/spec/support/prometheus/additional_metrics_shared_examples.rb new file mode 100644 index 00000000000..96a9e1f5049 --- /dev/null +++ b/spec/support/prometheus/additional_metrics_shared_examples.rb @@ -0,0 +1,143 @@ +RSpec.shared_examples 'additional metrics query' do + include Prometheus::MetricBuilders + + before do + allow(client).to receive(:label_values).and_return(metric_names) + allow(metric_group_class).to receive(:all).and_return([simple_metric_group(metrics: [simple_metric])]) + end + + let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } + let(:metric_class) { Gitlab::Prometheus::Metric } + + context 'with one group where two metrics is found' do + before do + allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + end + + context 'some queries return results' do + before do + allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_empty', any_args).and_return([]) + end + + it 'return group data only for queries with results' do + expected = [ + { + group: 'name', + priority: 1, + metrics: [ + { + title: 'title', weight: nil, y_label: 'Values', queries: [ + { query_range: 'query_range_a', result: query_range_result }, + { query_range: 'query_range_b', label: 'label', unit: 'unit', result: query_range_result } + ] + } + ] + } + ] + + expect(query_result).to eq(expected) + end + end + end + + context 'with two groups with one metric each' do + let(:metrics) { [simple_metric(queries: [simple_query])] } + before do + allow(metric_group_class).to receive(:all).and_return( + [ + simple_metric_group(name: 'group_a', metrics: [simple_metric(queries: [simple_query])]), + simple_metric_group(name: 'group_b', metrics: [simple_metric(title: 'title_b', queries: [simple_query('b')])]) + ]) + allow(client).to receive(:label_values).and_return(metric_names) + end + + context 'both queries return results' do + before do + allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_b', any_args).and_return(query_range_result) + end + + it 'return group data both queries' do + expected = [ + { + group: 'group_a', + priority: 1, + metrics: [ + { + title: 'title', + weight: nil, + y_label: 'Values', + queries: [ + { + query_range: 'query_range_a', + result: [ + { + metric: {}, + values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] } + ] + } + ] + } + ] + }, + { + group: 'group_b', + priority: 1, + metrics: [ + { + title: 'title_b', + weight: nil, + y_label: 'Values', + queries: [ + { + query_range: 'query_range_b', result: [ + { + metric: {}, + values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] + } + ] + } + ] + } + ] + } + ] + + expect(query_result).to eq(expected) + end + end + + context 'one query returns result' do + before do + allow(client).to receive(:query_range).with('query_range_a', any_args).and_return(query_range_result) + allow(client).to receive(:query_range).with('query_range_b', any_args).and_return([]) + end + + it 'return group data only for query with results' do + expected = [ + { + group: 'group_a', + priority: 1, + metrics: [ + { + title: 'title', + weight: nil, + y_label: 'Values', + queries: [ + { + query_range: 'query_range_a', + result: query_range_result + } + ] + } + ] + } + ] + + expect(query_result).to eq(expected) + end + end + end +end \ No newline at end of file diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index 2d54ecdfb5c..c57694bf7fd 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -8,7 +8,7 @@ module Prometheus [simple_query, simple_query('b', label: 'label', unit: 'unit')] end - def simple_metric(title: 'title', required_metrics: [], queries: []) + def simple_metric(title: 'title', required_metrics: [], queries: [simple_query]) Gitlab::Prometheus::Metric.new(title, required_metrics, nil, nil, queries) end @@ -20,8 +20,8 @@ module Prometheus ] end - def simple_metric_group(name = 'name', metrics = simple_metrics) + def simple_metric_group(name: 'name', metrics: simple_metrics) Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) end end -end \ No newline at end of file +end -- cgit v1.2.1 From a3eb8264f31a79fc05113df4276d7dcf4e0bad75 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 19:43:30 +0200 Subject: Refactor Metric tests to use more common code --- .../prometheus/queries/matched_metrics_query_spec.rb | 20 +++++++++++++------- .../prometheus/matched_metrics_query_helper.rb | 9 --------- spec/support/prometheus/metric_builders.rb | 6 +++--- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb index 390fff568cc..34f11205878 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do include Prometheus::MatchedMetricsQueryHelper + include Prometheus::MetricBuilders let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } let(:metric_class) { Gitlab::Prometheus::Metric } @@ -83,30 +84,35 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do end end - context 'with two groups where only one metric is found' do + context 'with two groups where metrics are found in each group' do + let(:second_metric_group) { simple_metric_group(name: 'nameb', metrics: simple_metrics(added_metric_name: 'metric_c')) } + before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group, - simple_metric_group('nameb', simple_metrics('metric_c'))]) + allow(metric_group_class).to receive(:all).and_return([simple_metric_group, second_metric_group]) allow(client).to receive(:label_values).and_return('metric_c') end - context 'both metrics in the group pass requirements' do + context 'all metrics in both groups pass requirements' do before do allow(client).to receive(:series).and_return(series_info_with_environment('metric_c')) end it 'responds with one metrics as active and no missing requiremens' do - expect(subject.query).to eq([{ group: 'nameb', priority: 1, active_metrics: 1, metrics_missing_requirements: 0 }]) + expect(subject.query).to eq([ + { group: 'name', priority: 1, active_metrics: 1, metrics_missing_requirements: 0 }, + { group: 'nameb', priority: 1, active_metrics: 2, metrics_missing_requirements: 0 }]) end end - context 'no metris in group pass requirements' do + context 'no metrics in groups pass requirements' do before do allow(client).to receive(:series).and_return(series_info_without_environment) end it 'responds with one metrics as active and no missing requiremens' do - expect(subject.query).to eq([{ group: 'nameb', priority: 1, active_metrics: 0, metrics_missing_requirements: 1 }]) + expect(subject.query).to eq([ + { group: 'name', priority: 1, active_metrics: 0, metrics_missing_requirements: 1 }, + { group: 'nameb', priority: 1, active_metrics: 0, metrics_missing_requirements: 2 }]) end end end diff --git a/spec/support/prometheus/matched_metrics_query_helper.rb b/spec/support/prometheus/matched_metrics_query_helper.rb index 86e874fb295..0471a78e426 100644 --- a/spec/support/prometheus/matched_metrics_query_helper.rb +++ b/spec/support/prometheus/matched_metrics_query_helper.rb @@ -4,15 +4,6 @@ module Prometheus %w{metric_a metric_b} end - def simple_metrics(metric_name = 'metric_a') - [Gitlab::Prometheus::Metric.new('title', %W(#{metric_name} metric_b), nil, nil), - Gitlab::Prometheus::Metric.new('title', [metric_name], nil, nil)] - end - - def simple_metric_group(name = 'name', metrics = simple_metrics) - Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) - end - def series_info_with_environment(*more_metrics) %w{metric_a metric_b}.concat(more_metrics).map { |metric_name| { '__name__' => metric_name, 'environment' => '' } } end diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index c57694bf7fd..4c0e01bd5f6 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -12,10 +12,10 @@ module Prometheus Gitlab::Prometheus::Metric.new(title, required_metrics, nil, nil, queries) end - def simple_metrics + def simple_metrics(added_metric_name: 'metric_a') [ - simple_metric(required_metrics: %w(metric_a metric_b), queries: simple_queries), - simple_metric(required_metrics: %w{metric_a}, queries: [simple_query('empty')]), + simple_metric(required_metrics: %W(#{added_metric_name} metric_b), queries: simple_queries), + simple_metric(required_metrics: [added_metric_name], queries: [simple_query('empty')]), simple_metric(required_metrics: %w{metric_c}) ] end -- cgit v1.2.1 From ffedc52eaa33a7a31d3a7b4893387e81163a3d5f Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 20:11:22 +0200 Subject: Cleanup Additional Metrics tests --- .../additional_metrics_deployment_query_spec.rb | 8 ------ .../queries/additional_metrics_query_spec.rb | 1 - .../queries/matched_metrics_query_spec.rb | 12 ++++++++- .../prometheus/additional_metrics_query_helper.rb | 31 ---------------------- .../additional_metrics_shared_examples.rb | 12 ++++++--- .../prometheus/matched_metrics_query_helper.rb | 24 ----------------- 6 files changed, 20 insertions(+), 68 deletions(-) delete mode 100644 spec/support/prometheus/additional_metrics_query_helper.rb delete mode 100644 spec/support/prometheus/matched_metrics_query_helper.rb diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb index 93a9ce38d80..836f2be629f 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb @@ -1,22 +1,14 @@ require 'spec_helper' describe Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery, lib: true do - include Prometheus::AdditionalMetricsQueryHelper include Prometheus::MetricBuilders - let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } - let(:metric_class) { Gitlab::Prometheus::Metric } - let(:client) { double('prometheus_client') } let(:environment) { create(:environment, slug: 'environment-slug') } let(:deployment) { create(:deployment, environment: environment) } subject(:query_result) { described_class.new(client).query(deployment.id) } - around do |example| - Timecop.freeze { example.run } - end - include_examples 'additional metrics query' do it 'queries using specific time' do expect(client).to receive(:query_range).with(anything, diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb index e5db1326597..6fbd2fd17d6 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do - include Prometheus::AdditionalMetricsQueryHelper include Prometheus::MetricBuilders let(:client) { double('prometheus_client') } diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb index 34f11205878..2395675a247 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -1,12 +1,22 @@ require 'spec_helper' describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do - include Prometheus::MatchedMetricsQueryHelper include Prometheus::MetricBuilders let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } let(:metric_class) { Gitlab::Prometheus::Metric } + def series_info_with_environment(*more_metrics) + %w{metric_a metric_b}.concat(more_metrics).map { |metric_name| { '__name__' => metric_name, 'environment' => '' } } + end + let(:metric_names) { %w{metric_a metric_b} } + let(:series_info_without_environment) do + [{ '__name__' => 'metric_a' }, + { '__name__' => 'metric_b' }] + end + let(:partialy_empty_series_info) { [{ '__name__' => 'metric_a', 'environment' => '' }] } + let(:empty_series_info) { [] } + let(:client) { double('prometheus_client') } subject { described_class.new(client) } diff --git a/spec/support/prometheus/additional_metrics_query_helper.rb b/spec/support/prometheus/additional_metrics_query_helper.rb deleted file mode 100644 index 84cbc24a301..00000000000 --- a/spec/support/prometheus/additional_metrics_query_helper.rb +++ /dev/null @@ -1,31 +0,0 @@ -module Prometheus - module AdditionalMetricsQueryHelper - def metric_names - %w{metric_a metric_b} - end - - def query_result - [ - { - 'metric': {}, - 'value': [ - 1488772511.004, - '0.000041021495238095323' - ] - } - ] - end - - def query_range_result - [ - { - 'metric': {}, - 'values': [ - [1488758662.506, '0.00002996364761904785'], - [1488758722.506, '0.00003090239047619091'] - ] - } - ] - end - end -end diff --git a/spec/support/prometheus/additional_metrics_shared_examples.rb b/spec/support/prometheus/additional_metrics_shared_examples.rb index 96a9e1f5049..449a53664c1 100644 --- a/spec/support/prometheus/additional_metrics_shared_examples.rb +++ b/spec/support/prometheus/additional_metrics_shared_examples.rb @@ -1,14 +1,20 @@ RSpec.shared_examples 'additional metrics query' do include Prometheus::MetricBuilders + let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } + let(:metric_class) { Gitlab::Prometheus::Metric } + + let(:metric_names) { %w{metric_a metric_b} } + + let(:query_range_result) do + [{ 'metric': {}, 'values': [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] }] + end + before do allow(client).to receive(:label_values).and_return(metric_names) allow(metric_group_class).to receive(:all).and_return([simple_metric_group(metrics: [simple_metric])]) end - let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } - let(:metric_class) { Gitlab::Prometheus::Metric } - context 'with one group where two metrics is found' do before do allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) diff --git a/spec/support/prometheus/matched_metrics_query_helper.rb b/spec/support/prometheus/matched_metrics_query_helper.rb deleted file mode 100644 index 0471a78e426..00000000000 --- a/spec/support/prometheus/matched_metrics_query_helper.rb +++ /dev/null @@ -1,24 +0,0 @@ -module Prometheus - module MatchedMetricsQueryHelper - def metric_names - %w{metric_a metric_b} - end - - def series_info_with_environment(*more_metrics) - %w{metric_a metric_b}.concat(more_metrics).map { |metric_name| { '__name__' => metric_name, 'environment' => '' } } - end - - def series_info_without_environment - [{ '__name__' => 'metric_a' }, - { '__name__' => 'metric_b' }] - end - - def partialy_empty_series_info - [{ '__name__' => 'metric_a', 'environment' => '' }] - end - - def empty_series_info - [] - end - end -end -- cgit v1.2.1 From 1b6ab2dffce29df53f57cd857720b9f77ab4a7ca Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Jun 2017 21:00:57 +0200 Subject: Remove orig file + rubocop cleanup --- .../queries/additional_metrics_deployment_query.rb | 2 -- .../prometheus/queries/additional_metrics_query.rb | 5 ++--- lib/gitlab/prometheus_client.rb | 2 +- spec/db/production/settings.rb.orig | 16 ---------------- .../prometheus/queries/matched_metrics_query_spec.rb | 9 +++++++-- .../prometheus/additional_metrics_shared_examples.rb | 5 +++-- 6 files changed, 13 insertions(+), 26 deletions(-) delete mode 100644 spec/db/production/settings.rb.orig diff --git a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb index 382052c298f..7693772bf81 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb @@ -1,6 +1,5 @@ module Gitlab::Prometheus::Queries class AdditionalMetricsDeploymentQuery < AdditionalMetricsQuery - def query(deployment_id) deployment = Deployment.find_by(id: deployment_id) query_context = { @@ -14,4 +13,3 @@ module Gitlab::Prometheus::Queries end end end - diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb index d21a3978e25..7ef4ee3a91a 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_query.rb @@ -40,11 +40,11 @@ module Gitlab::Prometheus::Queries private def metric_with_any_queries(metric) - metric[:queries]&.count > 0 + metric[:queries]&.count&.> 0 end def group_with_any_metrics(group) - group[:metrics]&.count > 0 + group[:metrics]&.count&.> 0 end def query_with_result(query) @@ -64,7 +64,6 @@ module Gitlab::Prometheus::Queries query_with_result end - def available_metrics @available_metrics ||= client_label_values || [] end diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb index f4ef4ff8ba4..aa94614bf18 100644 --- a/lib/gitlab/prometheus_client.rb +++ b/lib/gitlab/prometheus_client.rb @@ -29,7 +29,7 @@ module Gitlab end end - def label_values(name='__name__') + def label_values(name = '__name__') json_api_get("label/#{name}/values") end diff --git a/spec/db/production/settings.rb.orig b/spec/db/production/settings.rb.orig deleted file mode 100644 index 3cbb173c4cc..00000000000 --- a/spec/db/production/settings.rb.orig +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -describe 'seed production settings', lib: true do - include StubENV - - context 'GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN is set in the environment' do - before do - stub_env('GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN', '013456789') - end - - it 'writes the token to the database' do - load(File.join(__dir__, '../../../db/fixtures/production/010_settings.rb')) - expect(ApplicationSetting.current.runners_registration_token).to eq('013456789') - end - end -end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb index 2395675a247..d2796ab72da 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -9,6 +9,7 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do def series_info_with_environment(*more_metrics) %w{metric_a metric_b}.concat(more_metrics).map { |metric_name| { '__name__' => metric_name, 'environment' => '' } } end + let(:metric_names) { %w{metric_a metric_b} } let(:series_info_without_environment) do [{ '__name__' => 'metric_a' }, @@ -110,7 +111,9 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do it 'responds with one metrics as active and no missing requiremens' do expect(subject.query).to eq([ { group: 'name', priority: 1, active_metrics: 1, metrics_missing_requirements: 0 }, - { group: 'nameb', priority: 1, active_metrics: 2, metrics_missing_requirements: 0 }]) + { group: 'nameb', priority: 1, active_metrics: 2, metrics_missing_requirements: 0 } + ] + ) end end @@ -122,7 +125,9 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery, lib: true do it 'responds with one metrics as active and no missing requiremens' do expect(subject.query).to eq([ { group: 'name', priority: 1, active_metrics: 0, metrics_missing_requirements: 1 }, - { group: 'nameb', priority: 1, active_metrics: 0, metrics_missing_requirements: 2 }]) + { group: 'nameb', priority: 1, active_metrics: 0, metrics_missing_requirements: 2 } + ] + ) end end end diff --git a/spec/support/prometheus/additional_metrics_shared_examples.rb b/spec/support/prometheus/additional_metrics_shared_examples.rb index 449a53664c1..0581eab95a0 100644 --- a/spec/support/prometheus/additional_metrics_shared_examples.rb +++ b/spec/support/prometheus/additional_metrics_shared_examples.rb @@ -81,7 +81,8 @@ RSpec.shared_examples 'additional metrics query' do result: [ { metric: {}, - values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] } + values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] + } ] } ] @@ -146,4 +147,4 @@ RSpec.shared_examples 'additional metrics query' do end end end -end \ No newline at end of file +end -- cgit v1.2.1 From 336cef434372244b9f17bd1fd222e54f8b70979e Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 11:07:29 +0200 Subject: Fix transient error in deployment test --- .../prometheus/queries/additional_metrics_deployment_query_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb index 836f2be629f..4909aec5a4d 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb @@ -9,6 +9,10 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery, lib: tru subject(:query_result) { described_class.new(client).query(deployment.id) } + around do |example| + Timecop.freeze(Time.local(2008, 9, 1, 12, 0, 0)) { example.run } + end + include_examples 'additional metrics query' do it 'queries using specific time' do expect(client).to receive(:query_range).with(anything, -- cgit v1.2.1 From c0a66dbd2dd8b716e809938d20e7655d84595176 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 11:16:15 +0200 Subject: Fix prometheus service tests --- spec/models/project_services/prometheus_service_spec.rb | 5 +++-- spec/support/prometheus_helpers.rb | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 1f9d3c07b51..b761567ad76 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -61,7 +61,7 @@ describe PrometheusService, models: true, caching: true do end it 'returns reactive data' do - is_expected.to eq(prometheus_data) + is_expected.to eq(prometheus_metrics_data) end end end @@ -82,7 +82,7 @@ describe PrometheusService, models: true, caching: true do end it 'returns reactive data' do - is_expected.to eq(prometheus_data.merge(deployment_time: deployment.created_at.to_i)) + is_expected.to eq(prometheus_metrics_data.merge(deployment_time: deployment.created_at.to_i)) end end end @@ -112,6 +112,7 @@ describe PrometheusService, models: true, caching: true do end it { expect(subject.to_json).to eq(prometheus_data.to_json) } + it { expect(subject.to_json).to eq(prometheus_data.to_json) } end [404, 500].each do |status| diff --git a/spec/support/prometheus_helpers.rb b/spec/support/prometheus_helpers.rb index e49902475da..4212be2cc88 100644 --- a/spec/support/prometheus_helpers.rb +++ b/spec/support/prometheus_helpers.rb @@ -96,6 +96,19 @@ module PrometheusHelpers end def prometheus_data(last_update: Time.now.utc) + { + success: true, + data: { + memory_values: prometheus_values_body('matrix').dig(:data, :result), + memory_current: prometheus_value_body('vector').dig(:data, :result), + cpu_values: prometheus_values_body('matrix').dig(:data, :result), + cpu_current: prometheus_value_body('vector').dig(:data, :result) + }, + last_update: last_update + } + end + + def prometheus_metrics_data(last_update: Time.now.utc) { success: true, metrics: { -- cgit v1.2.1 From c7a1da800ff6fa16db5de796a8f8d715ddd3b582 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 14:40:03 +0200 Subject: Explicitly require format.json in prometheus_controller + add missing prometheus_controller tests! --- app/controllers/projects/deployments_controller.rb | 5 +- app/controllers/projects/prometheus_controller.rb | 18 +++++-- .../projects/prometheus_controller_spec.rb | 60 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 spec/controllers/projects/prometheus_controller_spec.rb diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index 29d94e2760a..acf5573935a 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -24,10 +24,11 @@ class Projects::DeploymentsController < Projects::ApplicationController def additional_metrics return render_404 unless deployment.has_additional_metrics? + metrics = deployment.additional_metrics - if metrics&.any? - render json: metrics, status: :ok + if metrics.any? + render json: metrics else head :no_content end diff --git a/app/controllers/projects/prometheus_controller.rb b/app/controllers/projects/prometheus_controller.rb index 0402be6f85c..4a39d13881e 100644 --- a/app/controllers/projects/prometheus_controller.rb +++ b/app/controllers/projects/prometheus_controller.rb @@ -3,17 +3,25 @@ class Projects::PrometheusController < Projects::ApplicationController before_action :require_prometheus_metrics! def active_metrics - matched_metrics = prometheus_service.reactive_query(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) + matched_metrics = prometheus_service.reactive_query(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) || {} - if matched_metrics - render json: matched_metrics, status: :ok - else - head :no_content + respond_to do |format| + format.json do + if matched_metrics.any? + render json: matched_metrics + else + head :no_content + end + end end end private + rescue_from(ActionController::UnknownFormat) do |e| + render_404 + end + def prometheus_service project.monitoring_service end diff --git a/spec/controllers/projects/prometheus_controller_spec.rb b/spec/controllers/projects/prometheus_controller_spec.rb new file mode 100644 index 00000000000..5e57b19e042 --- /dev/null +++ b/spec/controllers/projects/prometheus_controller_spec.rb @@ -0,0 +1,60 @@ +require('spec_helper') + +describe Projects::PrometheusController do + let(:user) { create(:user) } + let!(:project) { create(:empty_project) } + + let(:prometheus_service) { double('prometheus_service') } + + before do + allow(controller).to receive(:project).and_return(project) + allow(project).to receive(:monitoring_service).and_return(prometheus_service) + + project.add_master(user) + sign_in(user) + end + + describe 'GET #active_metrics' do + context 'when prometheus metrics are enabled' do + before do + allow(prometheus_service).to receive(:reactive_query) + end + + context 'when data is not present' do + it 'returns no content response' do + get :active_metrics, project_params(format: :json) + + expect(response).to have_http_status(204) + end + end + + context 'when data is available' do + let(:sample_response) { { some_data: 1 } } + + before do + allow(prometheus_service).to receive(:reactive_query).with(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name) + .and_return(sample_response) + end + + it 'returns no content response' do + get :active_metrics, project_params(format: :json) + + expect(response).to have_http_status(200) + expect(json_response).to eq(sample_response.deep_stringify_keys) + end + end + + context 'when requesting non json response' do + it 'returns not found response' do + get :active_metrics, project_params + + expect(response).to have_http_status(404) + end + end + end + end + + def project_params(opts = {}) + opts.reverse_merge(namespace_id: project.namespace, project_id: project) + end +end \ No newline at end of file -- cgit v1.2.1 From ccf89acc7145bb129f5666108854daa71a022827 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 16:07:33 +0200 Subject: expand Namespaces:: and refactoring yaml parsing out of MetricGroup class --- app/models/environment.rb | 2 +- lib/gitlab/prometheus/additional_metrics_parser.rb | 36 +++++++ lib/gitlab/prometheus/metric.rb | 33 +++--- lib/gitlab/prometheus/metric_group.rb | 41 +++----- lib/gitlab/prometheus/parsing_error.rb | 6 +- .../queries/additional_metrics_deployment_query.rb | 28 +++-- .../additional_metrics_environment_query.rb | 21 ++++ .../prometheus/queries/additional_metrics_query.rb | 82 --------------- lib/gitlab/prometheus/queries/deployment_query.rb | 42 ++++---- lib/gitlab/prometheus/queries/environment_query.rb | 34 +++--- .../prometheus/queries/matched_metrics_query.rb | 116 +++++++++++---------- .../prometheus/queries/query_additional_metrics.rb | 72 +++++++++++++ .../projects/prometheus_controller_spec.rb | 2 +- .../additional_metrics_environment_query_spec.rb | 21 ++++ .../queries/additional_metrics_query_spec.rb | 21 ---- spec/models/environment_spec.rb | 2 +- spec/support/prometheus/metric_builders.rb | 2 +- 17 files changed, 302 insertions(+), 259 deletions(-) create mode 100644 lib/gitlab/prometheus/additional_metrics_parser.rb create mode 100644 lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb delete mode 100644 lib/gitlab/prometheus/queries/additional_metrics_query.rb create mode 100644 lib/gitlab/prometheus/queries/query_additional_metrics.rb create mode 100644 spec/lib/gitlab/prometheus/queries/additional_metrics_environment_query_spec.rb delete mode 100644 spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb diff --git a/app/models/environment.rb b/app/models/environment.rb index 7ab4a23ab16..8d98b02c05a 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -159,7 +159,7 @@ class Environment < ActiveRecord::Base def additional_metrics if has_additional_metrics? - project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsQuery.name, self.id, &:itself) + project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, self.id, &:itself) end end diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb new file mode 100644 index 00000000000..eb168ce8f9c --- /dev/null +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -0,0 +1,36 @@ +module Gitlab + module Prometheus + module AdditionalMetricsParser + extend self + + def load_groups_from_yaml + additional_metrics_raw.map(&method(:new)) + end + + private + + def metrics_from_list(list) + list.map { |entry| metric_from_entry(entry) } + end + + def metric_from_entry(entry) + missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.has_key?(key) } + raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? + + Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries]) + end + + def group_from_entry(entry) + missing_fields = [:group, :priority, :metrics].select { |key| !entry.has_key?(key) } + raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? + + group = MetricGroup.new(entry[:group], entry[:priority]) + group.tap { |g| g.metrics = Metric.metrics_from_list(entry[:metrics]) } + end + + def additional_metrics_raw + @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml'))&.map(&:deep_symbolize_keys).freeze + end + end + end +end diff --git a/lib/gitlab/prometheus/metric.rb b/lib/gitlab/prometheus/metric.rb index d7cd4237a7b..5155064317c 100644 --- a/lib/gitlab/prometheus/metric.rb +++ b/lib/gitlab/prometheus/metric.rb @@ -1,24 +1,15 @@ -module Gitlab::Prometheus - class Metric - attr_reader :group, :title, :required_metrics, :weight, :y_label, :queries - - def initialize(title, required_metrics, weight, y_label, queries = []) - @title = title - @required_metrics = required_metrics - @weight = weight - @y_label = y_label || 'Values' - @queries = queries - end - - def self.metric_from_entry(entry) - missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.has_key?(key) } - raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? - - Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries]) - end - - def self.metrics_from_list(list) - list.map { |entry| metric_from_entry(entry) } +module Gitlab + module Prometheus + class Metric + attr_reader :group, :title, :required_metrics, :weight, :y_label, :queries + + def initialize(title, required_metrics, weight, y_label, queries = []) + @title = title + @required_metrics = required_metrics + @weight = weight + @y_label = y_label || 'Values' + @queries = queries + end end end end diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb index 0dcd9dc0f36..c3b24dc1fa5 100644 --- a/lib/gitlab/prometheus/metric_group.rb +++ b/lib/gitlab/prometheus/metric_group.rb @@ -1,33 +1,18 @@ -module Gitlab::Prometheus - class MetricGroup - attr_reader :priority, :name - attr_accessor :metrics +module Gitlab + module Prometheus + class MetricGroup + attr_reader :priority, :name + attr_accessor :metrics - def initialize(name, priority, metrics = []) - @name = name - @priority = priority - @metrics = metrics - end - - def self.all - load_groups_from_yaml - end - - def self.load_groups_from_yaml - additional_metrics_raw.map(&method(:group_from_entry)) - end - - def self.group_from_entry(entry) - missing_fields = [:group, :priority, :metrics].select { |key| !entry.has_key?(key) } - raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? - - group = MetricGroup.new(entry[:group], entry[:priority]) - group.metrics = Metric.metrics_from_list(entry[:metrics]) - group - end + def initialize(name:, priority:, metrics: []) + @name = name + @priority = priority + @metrics = metrics + end - def self.additional_metrics_raw - @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml'))&.map(&:deep_symbolize_keys).freeze + def self.all + AdditionalMetricsParser.load_groups_from_yaml + end end end end diff --git a/lib/gitlab/prometheus/parsing_error.rb b/lib/gitlab/prometheus/parsing_error.rb index 067ea7f878a..49cc0e16080 100644 --- a/lib/gitlab/prometheus/parsing_error.rb +++ b/lib/gitlab/prometheus/parsing_error.rb @@ -1,3 +1,5 @@ -module Gitlab::Prometheus - ParsingError = Class.new(StandardError) +module Gitlab + module Prometheus + ParsingError = Class.new(StandardError) + end end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb index 7693772bf81..cfe6bed188b 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb @@ -1,15 +1,21 @@ -module Gitlab::Prometheus::Queries - class AdditionalMetricsDeploymentQuery < AdditionalMetricsQuery - def query(deployment_id) - deployment = Deployment.find_by(id: deployment_id) - query_context = { - environment_slug: deployment.environment.slug, - environment_filter: %{container_name!="POD",environment="#{deployment.environment.slug}"}, - timeframe_start: (deployment.created_at - 30.minutes).to_f, - timeframe_end: (deployment.created_at + 30.minutes).to_f - } +module Gitlab + module Prometheus + module Queries + class AdditionalMetricsDeploymentQuery < BaseQuery + include QueryAdditionalMetrics - query_metrics(query_context) + def query(deployment_id) + deployment = Deployment.find_by(id: deployment_id) + query_context = { + environment_slug: deployment.environment.slug, + environment_filter: %{container_name!="POD",environment="#{deployment.environment.slug}"}, + timeframe_start: (deployment.created_at - 30.minutes).to_f, + timeframe_end: (deployment.created_at + 30.minutes).to_f + } + + query_metrics(query_context) + end + end end end end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb new file mode 100644 index 00000000000..e261988b234 --- /dev/null +++ b/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb @@ -0,0 +1,21 @@ +module Gitlab + module Prometheus + module Queries + class AdditionalMetricsEnvironmentQuery < BaseQuery + include QueryAdditionalMetrics + + def query(environment_id) + environment = Environment.find_by(id: environment_id) + query_context = { + environment_slug: environment.slug, + environment_filter: %{container_name!="POD",environment="#{environment.slug}"}, + timeframe_start: 8.hours.ago.to_f, + timeframe_end: Time.now.to_f + } + + query_metrics(query_context) + end + end + end + end +end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_query.rb deleted file mode 100644 index 7ef4ee3a91a..00000000000 --- a/lib/gitlab/prometheus/queries/additional_metrics_query.rb +++ /dev/null @@ -1,82 +0,0 @@ -module Gitlab::Prometheus::Queries - class AdditionalMetricsQuery < BaseQuery - def query(environment_id) - environment = Environment.find_by(id: environment_id) - query_context = { - environment_slug: environment.slug, - environment_filter: %{container_name!="POD",environment="#{environment.slug}"}, - timeframe_start: 8.hours.ago.to_f, - timeframe_end: Time.now.to_f - } - - query_metrics(query_context) - end - - protected - - def query_metrics(query_context) - query_processor = method(:process_query).curry[query_context] - - groups = matched_metrics.map do |group| - metrics = group.metrics.map do |metric| - { - title: metric.title, - weight: metric.weight, - y_label: metric.y_label, - queries: metric.queries.map(&query_processor).select(&method(:query_with_result)) - } - end - - { - group: group.name, - priority: group.priority, - metrics: metrics.select(&method(:metric_with_any_queries)) - } - end - - groups.select(&method(:group_with_any_metrics)) - end - - private - - def metric_with_any_queries(metric) - metric[:queries]&.count&.> 0 - end - - def group_with_any_metrics(group) - group[:metrics]&.count&.> 0 - end - - def query_with_result(query) - query[:result]&.any? do |item| - item&.[](:values)&.any? || item&.[](:value)&.any? - end - end - - def process_query(context, query) - query_with_result = query.dup - query_with_result[:result] = - if query.has_key?(:query_range) - client_query_range(query[:query_range] % context, start: context[:timeframe_start], stop: context[:timeframe_end]) - else - client_query(query[:query] % context, time: context[:timeframe_end]) - end - query_with_result - end - - def available_metrics - @available_metrics ||= client_label_values || [] - end - - def matched_metrics - result = Gitlab::Prometheus::MetricGroup.all.map do |group| - group.metrics.select! do |metric| - metric.required_metrics.all?(&available_metrics.method(:include?)) - end - group - end - - result.select { |group| group.metrics.any? } - end - end -end diff --git a/lib/gitlab/prometheus/queries/deployment_query.rb b/lib/gitlab/prometheus/queries/deployment_query.rb index 2cc08731f8d..be3527f0f72 100644 --- a/lib/gitlab/prometheus/queries/deployment_query.rb +++ b/lib/gitlab/prometheus/queries/deployment_query.rb @@ -1,26 +1,30 @@ -module Gitlab::Prometheus::Queries - class DeploymentQuery < BaseQuery - def query(deployment_id) - deployment = Deployment.find_by(id: deployment_id) - environment_slug = deployment.environment.slug +module Gitlab + module Prometheus + module Queries + class DeploymentQuery < BaseQuery + def query(deployment_id) + deployment = Deployment.find_by(id: deployment_id) + environment_slug = deployment.environment.slug - memory_query = raw_memory_usage_query(environment_slug) - memory_avg_query = %{avg(avg_over_time(container_memory_usage_bytes{container_name!="POD",environment="#{environment_slug}"}[30m]))} - cpu_query = raw_cpu_usage_query(environment_slug) - cpu_avg_query = %{avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="#{environment_slug}"}[30m])) * 100} + memory_query = raw_memory_usage_query(environment_slug) + memory_avg_query = %{avg(avg_over_time(container_memory_usage_bytes{container_name!="POD",environment="#{environment_slug}"}[30m]))} + cpu_query = raw_cpu_usage_query(environment_slug) + cpu_avg_query = %{avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="#{environment_slug}"}[30m])) * 100} - timeframe_start = (deployment.created_at - 30.minutes).to_f - timeframe_end = (deployment.created_at + 30.minutes).to_f + timeframe_start = (deployment.created_at - 30.minutes).to_f + timeframe_end = (deployment.created_at + 30.minutes).to_f - { - memory_values: client_query_range(memory_query, start: timeframe_start, stop: timeframe_end), - memory_before: client_query(memory_avg_query, time: deployment.created_at.to_f), - memory_after: client_query(memory_avg_query, time: timeframe_end), + { + memory_values: client_query_range(memory_query, start: timeframe_start, stop: timeframe_end), + memory_before: client_query(memory_avg_query, time: deployment.created_at.to_f), + memory_after: client_query(memory_avg_query, time: timeframe_end), - cpu_values: client_query_range(cpu_query, start: timeframe_start, stop: timeframe_end), - cpu_before: client_query(cpu_avg_query, time: deployment.created_at.to_f), - cpu_after: client_query(cpu_avg_query, time: timeframe_end) - } + cpu_values: client_query_range(cpu_query, start: timeframe_start, stop: timeframe_end), + cpu_before: client_query(cpu_avg_query, time: deployment.created_at.to_f), + cpu_after: client_query(cpu_avg_query, time: timeframe_end) + } + end + end end end end diff --git a/lib/gitlab/prometheus/queries/environment_query.rb b/lib/gitlab/prometheus/queries/environment_query.rb index 01d756d7284..9aa9da6d858 100644 --- a/lib/gitlab/prometheus/queries/environment_query.rb +++ b/lib/gitlab/prometheus/queries/environment_query.rb @@ -1,20 +1,24 @@ -module Gitlab::Prometheus::Queries - class EnvironmentQuery < BaseQuery - def query(environment_id) - environment = Environment.find_by(id: environment_id) - environment_slug = environment.slug - timeframe_start = 8.hours.ago.to_f - timeframe_end = Time.now.to_f +module Gitlab + module Prometheus + module Queries + class EnvironmentQuery < BaseQuery + def query(environment_id) + environment = Environment.find_by(id: environment_id) + environment_slug = environment.slug + timeframe_start = 8.hours.ago.to_f + timeframe_end = Time.now.to_f - memory_query = raw_memory_usage_query(environment_slug) - cpu_query = raw_cpu_usage_query(environment_slug) + memory_query = raw_memory_usage_query(environment_slug) + cpu_query = raw_cpu_usage_query(environment_slug) - { - memory_values: client_query_range(memory_query, start: timeframe_start, stop: timeframe_end), - memory_current: client_query(memory_query, time: timeframe_end), - cpu_values: client_query_range(cpu_query, start: timeframe_start, stop: timeframe_end), - cpu_current: client_query(cpu_query, time: timeframe_end) - } + { + memory_values: client_query_range(memory_query, start: timeframe_start, stop: timeframe_end), + memory_current: client_query(memory_query, time: timeframe_end), + cpu_values: client_query_range(cpu_query, start: timeframe_start, stop: timeframe_end), + cpu_current: client_query(cpu_query, time: timeframe_end) + } + end + end end end end diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metrics_query.rb index fc97bca486c..d4894c87f8d 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metrics_query.rb @@ -1,74 +1,78 @@ -module Gitlab::Prometheus::Queries - class MatchedMetricsQuery < BaseQuery - MAX_QUERY_ITEMS = 40.freeze +module Gitlab + module Prometheus + module Queries + class MatchedMetricsQuery < BaseQuery + MAX_QUERY_ITEMS = 40.freeze - def query - groups_data.map do |group, data| - { - group: group.name, - priority: group.priority, - active_metrics: data[:active_metrics], - metrics_missing_requirements: data[:metrics_missing_requirements] - } - end - end + def query + groups_data.map do |group, data| + { + group: group.name, + priority: group.priority, + active_metrics: data[:active_metrics], + metrics_missing_requirements: data[:metrics_missing_requirements] + } + end + end - private + private - def groups_data - metrics_groups = groups_with_active_metrics(Gitlab::Prometheus::MetricGroup.all) - lookup = active_series_lookup(metrics_groups) + def groups_data + metrics_groups = groups_with_active_metrics(Gitlab::Prometheus::MetricGroup.all) + lookup = active_series_lookup(metrics_groups) - groups = {} + groups = {} - metrics_groups.each do |group| - groups[group] ||= { active_metrics: 0, metrics_missing_requirements: 0 } - active_metrics = group.metrics.count { |metric| metric.required_metrics.all?(&lookup.method(:has_key?)) } + metrics_groups.each do |group| + groups[group] ||= { active_metrics: 0, metrics_missing_requirements: 0 } + active_metrics = group.metrics.count { |metric| metric.required_metrics.all?(&lookup.method(:has_key?)) } - groups[group][:active_metrics] += active_metrics - groups[group][:metrics_missing_requirements] += group.metrics.count - active_metrics - end + groups[group][:active_metrics] += active_metrics + groups[group][:metrics_missing_requirements] += group.metrics.count - active_metrics + end - groups - end + groups + end - def active_series_lookup(metric_groups) - timeframe_start = 8.hours.ago - timeframe_end = Time.now + def active_series_lookup(metric_groups) + timeframe_start = 8.hours.ago + timeframe_end = Time.now - series = metric_groups.flat_map(&:metrics).flat_map(&:required_metrics).uniq + series = metric_groups.flat_map(&:metrics).flat_map(&:required_metrics).uniq - lookup = series.each_slice(MAX_QUERY_ITEMS).flat_map do |batched_series| - client_series(*batched_series, start: timeframe_start, stop: timeframe_end) - .select(&method(:has_matching_label)) - .map { |series_info| [series_info['__name__'], true] } - end - lookup.to_h - end + lookup = series.each_slice(MAX_QUERY_ITEMS).flat_map do |batched_series| + client_series(*batched_series, start: timeframe_start, stop: timeframe_end) + .select(&method(:has_matching_label)) + .map { |series_info| [series_info['__name__'], true] } + end + lookup.to_h + end - def has_matching_label(series_info) - series_info.key?('environment') - end + def has_matching_label(series_info) + series_info.key?('environment') + end - def available_metrics - @available_metrics ||= client_label_values || [] - end + def available_metrics + @available_metrics ||= client_label_values || [] + end - def filter_active_metrics(metric_group) - metric_group.metrics.select! do |metric| - metric.required_metrics.all?(&available_metrics.method(:include?)) - end - metric_group - end + def filter_active_metrics(metric_group) + metric_group.metrics.select! do |metric| + metric.required_metrics.all?(&available_metrics.method(:include?)) + end + metric_group + end - def groups_with_active_metrics(metric_groups) - metric_groups.map(&method(:filter_active_metrics)).select { |group| group.metrics.any? } - end + def groups_with_active_metrics(metric_groups) + metric_groups.map(&method(:filter_active_metrics)).select { |group| group.metrics.any? } + end - def metrics_with_required_series(metric_groups) - metric_groups.flat_map do |group| - group.metrics.select do |metric| - metric.required_metrics.all?(&available_metrics.method(:include?)) + def metrics_with_required_series(metric_groups) + metric_groups.flat_map do |group| + group.metrics.select do |metric| + metric.required_metrics.all?(&available_metrics.method(:include?)) + end + end end end end diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb new file mode 100644 index 00000000000..46029a1889a --- /dev/null +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -0,0 +1,72 @@ +module Gitlab + module Prometheus + module Queries + module QueryAdditionalMetrics + def query_metrics(query_context) + query_processor = method(:process_query).curry[query_context] + + groups = matched_metrics.map do |group| + metrics = group.metrics.map do |metric| + { + title: metric.title, + weight: metric.weight, + y_label: metric.y_label, + queries: metric.queries.map(&query_processor).select(&method(:query_with_result)) + } + end + + { + group: group.name, + priority: group.priority, + metrics: metrics.select(&method(:metric_with_any_queries)) + } + end + + groups.select(&method(:group_with_any_metrics)) + end + + private + + def metric_with_any_queries(metric) + metric[:queries]&.count&.> 0 + end + + def group_with_any_metrics(group) + group[:metrics]&.count&.> 0 + end + + def query_with_result(query) + query[:result]&.any? do |item| + item&.[](:values)&.any? || item&.[](:value)&.any? + end + end + + def process_query(context, query) + query_with_result = query.dup + query_with_result[:result] = + if query.has_key?(:query_range) + client_query_range(query[:query_range] % context, start: context[:timeframe_start], stop: context[:timeframe_end]) + else + client_query(query[:query] % context, time: context[:timeframe_end]) + end + query_with_result + end + + def available_metrics + @available_metrics ||= client_label_values || [] + end + + def matched_metrics + result = Gitlab::Prometheus::MetricGroup.all.map do |group| + group.metrics.select! do |metric| + metric.required_metrics.all?(&available_metrics.method(:include?)) + end + group + end + + result.select { |group| group.metrics.any? } + end + end + end + end +end diff --git a/spec/controllers/projects/prometheus_controller_spec.rb b/spec/controllers/projects/prometheus_controller_spec.rb index 5e57b19e042..7c976cfad83 100644 --- a/spec/controllers/projects/prometheus_controller_spec.rb +++ b/spec/controllers/projects/prometheus_controller_spec.rb @@ -57,4 +57,4 @@ describe Projects::PrometheusController do def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end -end \ No newline at end of file +end diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_environment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_environment_query_spec.rb new file mode 100644 index 00000000000..8e6e3bb5946 --- /dev/null +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_environment_query_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery, lib: true do + include Prometheus::MetricBuilders + + let(:client) { double('prometheus_client') } + let(:environment) { create(:environment, slug: 'environment-slug') } + + subject(:query_result) { described_class.new(client).query(environment.id) } + + around do |example| + Timecop.freeze { example.run } + end + + include_examples 'additional metrics query' do + it 'queries using specific time' do + expect(client).to receive(:query_range).with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f) + expect(query_result).not_to be_nil + end + end +end diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb deleted file mode 100644 index 6fbd2fd17d6..00000000000 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_query_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Prometheus::Queries::AdditionalMetricsQuery, lib: true do - include Prometheus::MetricBuilders - - let(:client) { double('prometheus_client') } - let(:environment) { create(:environment, slug: 'environment-slug') } - - subject(:query_result) { described_class.new(client).query(environment.id) } - - around do |example| - Timecop.freeze { example.run } - end - - include_examples 'additional metrics query' do - it 'queries using specific time' do - expect(client).to receive(:query_range).with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f) - expect(query_result).not_to be_nil - end - end -end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12de9c9111b..e25d2bb6955 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -453,7 +453,7 @@ describe Environment, models: true do it 'returns the additional metrics from the deployment service' do expect(project.monitoring_service).to receive(:reactive_query) - .with(Gitlab::Prometheus::Queries::AdditionalMetricsQuery.name, environment.id) + .with(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index 4c0e01bd5f6..cc733bfe1b4 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -21,7 +21,7 @@ module Prometheus end def simple_metric_group(name: 'name', metrics: simple_metrics) - Gitlab::Prometheus::MetricGroup.new(name, 1, metrics) + Gitlab::Prometheus::MetricGroup.new(name: name, priority: 1, metrics: metrics) end end end -- cgit v1.2.1 From 969b812433b6030b15b591ec5862daae1b707025 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Jun 2017 19:35:28 +0200 Subject: Add schema matcher for non response objects + use schema to test additional metrics compliance --- .../additional_metrics_query_result.json | 58 +++++++++++++++ spec/support/api/schema_matcher.rb | 14 +++- .../additional_metrics_shared_examples.rb | 85 +++++----------------- spec/support/prometheus/metric_builders.rb | 2 +- 4 files changed, 88 insertions(+), 71 deletions(-) create mode 100644 spec/fixtures/api/schemas/prometheus/additional_metrics_query_result.json diff --git a/spec/fixtures/api/schemas/prometheus/additional_metrics_query_result.json b/spec/fixtures/api/schemas/prometheus/additional_metrics_query_result.json new file mode 100644 index 00000000000..47b5d283b8c --- /dev/null +++ b/spec/fixtures/api/schemas/prometheus/additional_metrics_query_result.json @@ -0,0 +1,58 @@ +{ + "items": { + "properties": { + "group": { + "type": "string" + }, + "metrics": { + "items": { + "properties": { + "queries": { + "items": { + "properties": { + "query_range": { + "type": "string" + }, + "query": { + "type": "string" + }, + "result": { + "type": "any" + } + }, + "type": "object" + }, + "type": "array" + }, + "title": { + "type": "string" + }, + "weight": { + "type": "integer" + }, + "y_label": { + "type": "string" + } + }, + "type": "object" + }, + "required": [ + "metrics", + "title", + "weight" + ], + "type": "array" + }, + "priority": { + "type": "integer" + } + }, + "type": "object" + }, + "required": [ + "group", + "priority", + "metrics" + ], + "type": "array" +} \ No newline at end of file diff --git a/spec/support/api/schema_matcher.rb b/spec/support/api/schema_matcher.rb index e42d727672b..dff0dfba675 100644 --- a/spec/support/api/schema_matcher.rb +++ b/spec/support/api/schema_matcher.rb @@ -1,8 +1,16 @@ +def schema_path(schema) + schema_directory = "#{Dir.pwd}/spec/fixtures/api/schemas" + "#{schema_directory}/#{schema}.json" +end + RSpec::Matchers.define :match_response_schema do |schema, **options| match do |response| - schema_directory = "#{Dir.pwd}/spec/fixtures/api/schemas" - schema_path = "#{schema_directory}/#{schema}.json" + JSON::Validator.validate!(schema_path(schema), response.body, options) + end +end - JSON::Validator.validate!(schema_path, response.body, options) +RSpec::Matchers.define :match_schema do |schema, **options| + match do |data| + JSON::Validator.validate!(schema_path(schema), data, options) end end diff --git a/spec/support/prometheus/additional_metrics_shared_examples.rb b/spec/support/prometheus/additional_metrics_shared_examples.rb index 0581eab95a0..016e16fc8d4 100644 --- a/spec/support/prometheus/additional_metrics_shared_examples.rb +++ b/spec/support/prometheus/additional_metrics_shared_examples.rb @@ -34,7 +34,7 @@ RSpec.shared_examples 'additional metrics query' do priority: 1, metrics: [ { - title: 'title', weight: nil, y_label: 'Values', queries: [ + title: 'title', weight: 1, y_label: 'Values', queries: [ { query_range: 'query_range_a', result: query_range_result }, { query_range: 'query_range_b', label: 'label', unit: 'unit', result: query_range_result } ] @@ -43,6 +43,7 @@ RSpec.shared_examples 'additional metrics query' do } ] + expect(query_result).to match_schema('prometheus/additional_metrics_query_result') expect(query_result).to eq(expected) end end @@ -66,53 +67,16 @@ RSpec.shared_examples 'additional metrics query' do end it 'return group data both queries' do - expected = [ - { - group: 'group_a', - priority: 1, - metrics: [ - { - title: 'title', - weight: nil, - y_label: 'Values', - queries: [ - { - query_range: 'query_range_a', - result: [ - { - metric: {}, - values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] - } - ] - } - ] - } - ] - }, - { - group: 'group_b', - priority: 1, - metrics: [ - { - title: 'title_b', - weight: nil, - y_label: 'Values', - queries: [ - { - query_range: 'query_range_b', result: [ - { - metric: {}, - values: [[1488758662.506, '0.00002996364761904785'], [1488758722.506, '0.00003090239047619091']] - } - ] - } - ] - } - ] - } - ] + queries_with_result_a = { queries: [{ query_range: 'query_range_a', result: query_range_result }] } + queries_with_result_b = { queries: [{ query_range: 'query_range_b', result: query_range_result }] } - expect(query_result).to eq(expected) + expect(query_result).to match_schema('prometheus/additional_metrics_query_result') + + expect(query_result.count).to eq(2) + expect(query_result).to all(satisfy { |r| r[:metrics].count == 1 }) + + expect(query_result[0][:metrics].first).to include(queries_with_result_a) + expect(query_result[1][:metrics].first).to include(queries_with_result_b) end end @@ -123,27 +87,14 @@ RSpec.shared_examples 'additional metrics query' do end it 'return group data only for query with results' do - expected = [ - { - group: 'group_a', - priority: 1, - metrics: [ - { - title: 'title', - weight: nil, - y_label: 'Values', - queries: [ - { - query_range: 'query_range_a', - result: query_range_result - } - ] - } - ] - } - ] + queries_with_result = { queries: [{ query_range: 'query_range_a', result: query_range_result }] } - expect(query_result).to eq(expected) + expect(query_result).to match_schema('prometheus/additional_metrics_query_result') + + expect(query_result.count).to eq(1) + expect(query_result).to all(satisfy { |r| r[:metrics].count == 1 }) + + expect(query_result.first[:metrics].first).to include(queries_with_result) end end end diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index cc733bfe1b4..18378ec0145 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -9,7 +9,7 @@ module Prometheus end def simple_metric(title: 'title', required_metrics: [], queries: [simple_query]) - Gitlab::Prometheus::Metric.new(title, required_metrics, nil, nil, queries) + Gitlab::Prometheus::Metric.new(title, required_metrics, 1, nil, queries) end def simple_metrics(added_metric_name: 'metric_a') -- cgit v1.2.1 From a7e1205219387a6d24c8579994f73a33b3028010 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 7 Jun 2017 02:36:59 +0200 Subject: Use explicit instance of prometheus service and add access methods to it --- app/controllers/projects/deployments_controller.rb | 2 +- app/controllers/projects/prometheus_controller.rb | 14 ++--- app/models/deployment.rb | 13 ++--- app/models/environment.rb | 14 +++-- app/models/project_services/prometheus_service.rb | 12 ++++- .../projects/deployments_controller_spec.rb | 62 ++++++++++++++++++++++ .../projects/prometheus_controller_spec.rb | 13 +++-- spec/models/deployment_spec.rb | 29 ++++++++++ spec/models/environment_spec.rb | 10 ++-- 9 files changed, 136 insertions(+), 33 deletions(-) diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index acf5573935a..e7d95e46b06 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -23,7 +23,7 @@ class Projects::DeploymentsController < Projects::ApplicationController end def additional_metrics - return render_404 unless deployment.has_additional_metrics? + return render_404 unless deployment.prometheus_service.present? metrics = deployment.additional_metrics diff --git a/app/controllers/projects/prometheus_controller.rb b/app/controllers/projects/prometheus_controller.rb index 4a39d13881e..9609fa44945 100644 --- a/app/controllers/projects/prometheus_controller.rb +++ b/app/controllers/projects/prometheus_controller.rb @@ -3,10 +3,10 @@ class Projects::PrometheusController < Projects::ApplicationController before_action :require_prometheus_metrics! def active_metrics - matched_metrics = prometheus_service.reactive_query(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) || {} - respond_to do |format| format.json do + matched_metrics = prometheus_service.matched_metrics || {} + if matched_metrics.any? render json: matched_metrics else @@ -18,19 +18,15 @@ class Projects::PrometheusController < Projects::ApplicationController private - rescue_from(ActionController::UnknownFormat) do |e| + rescue_from(ActionController::UnknownFormat) do render_404 end def prometheus_service - project.monitoring_service - end - - def has_prometheus_metrics? - prometheus_service&.respond_to?(:reactive_query) + @prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name) end def require_prometheus_metrics! - render_404 unless has_prometheus_metrics? + render_404 unless prometheus_service.present? end end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index f49410f18ae..fbb97de4af6 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -103,10 +103,6 @@ class Deployment < ActiveRecord::Base project.monitoring_service.present? end - def has_additional_metrics? - has_metrics? && project.monitoring_service&.respond_to?(:reactive_query) - end - def metrics return {} unless has_metrics? @@ -114,11 +110,16 @@ class Deployment < ActiveRecord::Base end def additional_metrics - return {} unless has_additional_metrics? - metrics = project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, id, &:itself) + return {} unless prometheus_service.present? + + metrics = prometheus_service.additional_deployment_metrics(self) metrics&.merge(deployment_time: created_at.to_i) || {} end + def prometheus_service + @prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name) + end + private def ref_path diff --git a/app/models/environment.rb b/app/models/environment.rb index 8d98b02c05a..9b7dc7f807e 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -149,20 +149,24 @@ class Environment < ActiveRecord::Base project.monitoring_service.present? && available? && last_deployment.present? end - def has_additional_metrics? - has_metrics? && project.monitoring_service&.respond_to?(:reactive_query) - end - def metrics project.monitoring_service.environment_metrics(self) if has_metrics? end + def has_additional_metrics? + prometheus_service.present? && available? && last_deployment.present? + end + def additional_metrics if has_additional_metrics? - project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, self.id, &:itself) + prometheus_service.additional_environment_metrics(self) end end + def prometheus_service + @prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name) + end + # An environment name is not necessarily suitable for use in URLs, DNS # or other third-party contexts, so provide a slugified version. A slug has # the following properties: diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index d4d6611fd3b..9d736cb5dec 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -62,8 +62,16 @@ class PrometheusService < MonitoringService metrics&.merge(deployment_time: created_at.to_i) || {} end - def reactive_query(query_class, *args, &block) - with_reactive_cache(query_class, *args, &block) + def additional_environment_metrics(environment) + with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id, &:itself) + end + + def additional_deployment_metrics(deployment) + with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, deployment.id, &:itself) + end + + def matched_metrics + additional_deployment_metrics(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) end # Cache metrics for specific environment diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 4c69443314d..26d92e787c4 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -108,6 +108,68 @@ describe Projects::DeploymentsController do end end + describe 'GET #additional_metrics' do + let(:deployment) { create(:deployment, project: project, environment: environment) } + + before do + allow(controller).to receive(:deployment).and_return(deployment) + end + context 'when metrics are disabled' do + before do + allow(deployment).to receive(:has_metrics?).and_return false + end + + it 'responds with not found' do + get :metrics, deployment_params(id: deployment.id) + + expect(response).to be_not_found + end + end + + context 'when metrics are enabled' do + let(:prometheus_service) { double('prometheus_service') } + + before do + allow(deployment).to receive(:prometheus_service).and_return(prometheus_service) + end + + context 'when environment has no metrics' do + before do + expect(deployment).to receive(:additional_metrics).and_return({}) + end + + it 'returns a empty response 204 response' do + get :additional_metrics, deployment_params(id: deployment.id) + expect(response).to have_http_status(204) + expect(response.body).to eq('') + end + end + + context 'when environment has some metrics' do + let(:empty_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + before do + expect(deployment).to receive(:additional_metrics).and_return(empty_metrics) + end + + it 'returns a metrics JSON document' do + get :additional_metrics, deployment_params(id: deployment.id) + + expect(response).to be_ok + expect(json_response['success']).to be(true) + expect(json_response['metrics']).to eq({}) + expect(json_response['last_update']).to eq(42) + end + end + end + end + def deployment_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project, diff --git a/spec/controllers/projects/prometheus_controller_spec.rb b/spec/controllers/projects/prometheus_controller_spec.rb index 7c976cfad83..a994ac6409f 100644 --- a/spec/controllers/projects/prometheus_controller_spec.rb +++ b/spec/controllers/projects/prometheus_controller_spec.rb @@ -8,7 +8,7 @@ describe Projects::PrometheusController do before do allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:monitoring_service).and_return(prometheus_service) + allow(controller).to receive(:prometheus_service).and_return(prometheus_service) project.add_master(user) sign_in(user) @@ -16,11 +16,11 @@ describe Projects::PrometheusController do describe 'GET #active_metrics' do context 'when prometheus metrics are enabled' do - before do - allow(prometheus_service).to receive(:reactive_query) - end - context 'when data is not present' do + before do + allow(prometheus_service).to receive(:matched_metrics).and_return({}) + end + it 'returns no content response' do get :active_metrics, project_params(format: :json) @@ -32,8 +32,7 @@ describe Projects::PrometheusController do let(:sample_response) { { some_data: 1 } } before do - allow(prometheus_service).to receive(:reactive_query).with(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name) - .and_return(sample_response) + allow(prometheus_service).to receive(:matched_metrics).and_return(sample_response) end it 'returns no content response' do diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 4bda7d4314a..bbb7dbf0922 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -77,6 +77,35 @@ describe Deployment, models: true do end end + describe '#additional_metrics' do + let(:deployment) { create(:deployment) } + + subject { deployment.additional_metrics } + + context 'metrics are disabled' do + it { is_expected.to eq({}) } + end + + context 'metrics are enabled' do + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + let(:prometheus_service) { double('prometheus_service') } + + before do + allow(deployment).to receive(:prometheus_service).and_return(prometheus_service) + allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics) + end + + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + end + end + describe '#stop_action' do let(:build) { create(:ci_build) } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e25d2bb6955..8c9d093fc48 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -452,8 +452,8 @@ describe Environment, models: true do end it 'returns the additional metrics from the deployment service' do - expect(project.monitoring_service).to receive(:reactive_query) - .with(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id) + expect(environment.prometheus_service).to receive(:additional_environment_metrics) + .with(environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -470,7 +470,11 @@ describe Environment, models: true do end describe '#has_additional_metrics??' do - subject { environment.has_metrics? } + subject { environment.has_additional_metrics? } + + before do + + end context 'when the enviroment is available' do context 'with a deployment service' do -- cgit v1.2.1 From a924152219c1367bf494f3f387d050ac3ff2d7d3 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 7 Jun 2017 04:07:47 +0200 Subject: Remove unecessary before block --- spec/models/environment_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8c9d093fc48..c8709409cea 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -472,10 +472,6 @@ describe Environment, models: true do describe '#has_additional_metrics??' do subject { environment.has_additional_metrics? } - before do - - end - context 'when the enviroment is available' do context 'with a deployment service' do let(:project) { create(:prometheus_project) } -- cgit v1.2.1 From 6eb96b2019d392d906a64108dbe83b3ce7cce758 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 7 Jun 2017 12:41:06 +0200 Subject: Use `key?` instead of `has_key?` --- lib/gitlab/prometheus/additional_metrics_parser.rb | 4 ++-- lib/gitlab/prometheus/queries/query_additional_metrics.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index eb168ce8f9c..18eb79a44be 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -14,14 +14,14 @@ module Gitlab end def metric_from_entry(entry) - missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.has_key?(key) } + missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.key?(key) } raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries]) end def group_from_entry(entry) - missing_fields = [:group, :priority, :metrics].select { |key| !entry.has_key?(key) } + missing_fields = [:group, :priority, :metrics].select { |key| !entry.key?(key) } raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? group = MetricGroup.new(entry[:group], entry[:priority]) diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index 46029a1889a..dd888b2bc1d 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -44,7 +44,7 @@ module Gitlab def process_query(context, query) query_with_result = query.dup query_with_result[:result] = - if query.has_key?(:query_range) + if query.key?(:query_range) client_query_range(query[:query_range] % context, start: context[:timeframe_start], stop: context[:timeframe_end]) else client_query(query[:query] % context, time: context[:timeframe_end]) -- cgit v1.2.1 From f78fd3de5d55830c84f25cfd50d399e7ddaddf30 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 8 Jun 2017 12:29:53 +0200 Subject: Fix Additional metrics filtering + remove test button that was leftover after a bad merge --- app/models/project_services/prometheus_service.rb | 6 +++++- app/views/projects/services/_form.html.haml | 1 - lib/gitlab/prometheus/additional_metrics_parser.rb | 4 ++-- lib/gitlab/prometheus/metric_group.rb | 2 +- lib/gitlab/prometheus/queries/query_additional_metrics.rb | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 1c4ae18c6ca..d81fec3514f 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -72,7 +72,11 @@ class PrometheusService < MonitoringService end def matched_metrics - additional_deployment_metrics(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) + with_reactive_cache(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) + end + + def with_reactive_cache(*args, &block) + yield calculate_reactive_cache(*args) end # Cache metrics for specific environment diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index cb3e1543fde..6dffc026392 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -22,7 +22,6 @@ - disabled_class = 'disabled' - disabled_title = @service.disabled_title - = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled_class}", title: disabled_title = link_to 'Cancel', namespace_project_settings_integrations_path(@project.namespace, @project), class: 'btn btn-cancel' - if lookup_context.template_exists?('show', "projects/services/#{@service.to_param}", true) diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index 18eb79a44be..694001605ef 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -4,7 +4,7 @@ module Gitlab extend self def load_groups_from_yaml - additional_metrics_raw.map(&method(:new)) + additional_metrics_raw.map(&method(:group_from_entry)) end private @@ -25,7 +25,7 @@ module Gitlab raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? group = MetricGroup.new(entry[:group], entry[:priority]) - group.tap { |g| g.metrics = Metric.metrics_from_list(entry[:metrics]) } + group.tap { |g| g.metrics = metrics_from_list(entry[:metrics]) } end def additional_metrics_raw diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb index c3b24dc1fa5..12bdf407ce0 100644 --- a/lib/gitlab/prometheus/metric_group.rb +++ b/lib/gitlab/prometheus/metric_group.rb @@ -4,7 +4,7 @@ module Gitlab attr_reader :priority, :name attr_accessor :metrics - def initialize(name:, priority:, metrics: []) + def initialize(name, priority, metrics = []) @name = name @priority = priority @metrics = metrics diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index dd888b2bc1d..a5a401f84cd 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -37,7 +37,7 @@ module Gitlab def query_with_result(query) query[:result]&.any? do |item| - item&.[](:values)&.any? || item&.[](:value)&.any? + item&.[]('values')&.any? || item&.[]('value')&.any? end end -- cgit v1.2.1 From 0e7e7c2f2bd0e9c913cda438826a60e761130271 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 9 Jun 2017 12:43:12 +0200 Subject: Test Additional metrics parser and fix query checking tests --- app/models/project_services/prometheus_service.rb | 4 - lib/gitlab/prometheus/additional_metrics_parser.rb | 15 +- .../prometheus/queries/query_additional_metrics.rb | 5 +- .../prometheus/additional_metrics_parser_spec.rb | 250 +++++++++++++++++++++ spec/support/prometheus/metric_builders.rb | 2 +- 5 files changed, 265 insertions(+), 11 deletions(-) create mode 100644 spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index d81fec3514f..c9df678f97e 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -75,10 +75,6 @@ class PrometheusService < MonitoringService with_reactive_cache(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) end - def with_reactive_cache(*args, &block) - yield calculate_reactive_cache(*args) - end - # Cache metrics for specific environment def calculate_reactive_cache(query_class_name, *args) return unless active? && project && !project.pending_delete? diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index 694001605ef..9bd41b99d81 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -14,22 +14,29 @@ module Gitlab end def metric_from_entry(entry) - missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.key?(key) } + required_fields = [:title, :required_metrics, :weight, :queries] + missing_fields = required_fields.select { |key| entry[key].nil? } raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries]) end def group_from_entry(entry) - missing_fields = [:group, :priority, :metrics].select { |key| !entry.key?(key) } - raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? + required_fields = [:group, :priority, :metrics] + missing_fields = required_fields.select { |key| entry[key].nil? } + + raise ParsingError.new("entry missing required fields #{missing_fields.map(&:to_s)}") unless missing_fields.empty? group = MetricGroup.new(entry[:group], entry[:priority]) group.tap { |g| g.metrics = metrics_from_list(entry[:metrics]) } end def additional_metrics_raw - @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml'))&.map(&:deep_symbolize_keys).freeze + @additional_metrics_raw ||= load_yaml_file&.map(&:deep_symbolize_keys).freeze + end + + def load_yaml_file + YAML.load_file(Rails.root.join('config/additional_metrics.yml')) end end end diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index a5a401f84cd..e44be770544 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -37,18 +37,19 @@ module Gitlab def query_with_result(query) query[:result]&.any? do |item| - item&.[]('values')&.any? || item&.[]('value')&.any? + item&.[](:values)&.any? || item&.[](:value)&.any? end end def process_query(context, query) query_with_result = query.dup - query_with_result[:result] = + result = if query.key?(:query_range) client_query_range(query[:query_range] % context, start: context[:timeframe_start], stop: context[:timeframe_end]) else client_query(query[:query] % context, time: context[:timeframe_end]) end + query_with_result[:result] = result&.map(&:deep_symbolize_keys) query_with_result end diff --git a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb new file mode 100644 index 00000000000..97280de173e --- /dev/null +++ b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb @@ -0,0 +1,250 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do + include Prometheus::MetricBuilders + + let(:parser_error_class) { Gitlab::Prometheus::ParsingError } + + describe '#load_groups_from_yaml' do + subject { described_class.load_groups_from_yaml } + + describe 'parsing sample yaml' do + let(:sample_yaml) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: "title" + required_metrics: [ metric_a, metric_b ] + weight: 1 + queries: [{ query_range: 'query_range_a', label: label, unit: unit }] + - title: "title" + required_metrics: [metric_a] + weight: 1 + queries: [{ query_range: 'query_range_empty' }] + - group: group_b + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: 1 + queries: [{query_range: query_range_a}] + EOF + end + + before do + described_class.instance_variable_set :@additional_metrics_raw, nil + allow(described_class).to receive(:load_yaml_file) { YAML.load(sample_yaml) } + end + + it 'parses to two metric groups with 2 and 1 metric respectively' do + expect(subject.count).to eq(2) + expect(subject[0].metrics.count).to eq(2) + expect(subject[1].metrics.count).to eq(1) + end + + it 'provide group data' do + expect(subject[0]).to have_attributes(name: 'group_a', priority: 1) + expect(subject[1]).to have_attributes(name: 'group_b', priority: 1) + end + + it 'provides metrics data' do + metrics = subject.flat_map(&:metrics) + + expect(metrics.count).to eq(3) + expect(metrics[0]).to have_attributes(title: 'title', required_metrics: %w(metric_a metric_b), weight: 1) + expect(metrics[1]).to have_attributes(title: 'title', required_metrics: %w(metric_a), weight: 1) + expect(metrics[2]).to have_attributes(title: 'title', required_metrics: [], weight: 1) + end + + it 'provides query data' do + queries = subject.flat_map(&:metrics).flat_map(&:queries) + + expect(queries.count).to eq(3) + expect(queries[0]).to eq(query_range: 'query_range_a', label: 'label', unit: 'unit') + expect(queries[1]).to eq(query_range: 'query_range_empty') + expect(queries[2]).to eq(query_range: 'query_range_a') + end + end + + shared_examples 'required field' do |field_name| + before do + described_class.instance_variable_set :@additional_metrics_raw, nil + end + + context "when #{field_name} is nil" do + before do + allow(described_class).to receive(:load_yaml_file) { YAML.load(field_missing) } + end + + it 'throws parsing error' do + expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/) + end + end + + context "when #{field_name} are not specified" do + before do + allow(described_class).to receive(:load_yaml_file) { YAML.load(field_nil) } + end + + it 'throws parsing error' do + expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/) + end + end + end + + describe 'group required fields' do + it_behaves_like 'required field', :metrics do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + EOF + end + end + + it_behaves_like 'required field', :group do + let(:field_nil) do + <<-EOF.strip_heredoc + - priority: 1 + metrics: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - priority: 1 + metrics: [] + EOF + end + end + + it_behaves_like 'required field', :priority do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: + metrics: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + metrics: [] + EOF + end + end + end + + describe 'metrics fields parsing' do + it_behaves_like 'required field', :title do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: + required_metrics: [] + weight: 1 + queries: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - required_metrics: [] + weight: 1 + queries: [] + EOF + end + end + + it_behaves_like 'required field', :required_metrics do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: + weight: 1 + queries: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + weight: 1 + queries: [] + EOF + end + end + + it_behaves_like 'required field', :weight do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: + queries: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + queries: [] + EOF + end + end + + it_behaves_like 'required field', :queries do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: 1 + queries: + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: 1 + EOF + end + end + end + end +end diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index 18378ec0145..e4b55c22acd 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -21,7 +21,7 @@ module Prometheus end def simple_metric_group(name: 'name', metrics: simple_metrics) - Gitlab::Prometheus::MetricGroup.new(name: name, priority: 1, metrics: metrics) + Gitlab::Prometheus::MetricGroup.new( name, 1, metrics) end end end -- cgit v1.2.1 From 56b14667ed502cf62dee984c5754a8ef833ba103 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Thu, 15 Jun 2017 17:46:15 +1000 Subject: hot reloading for .vue files --- config/webpack.config.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/webpack.config.js b/config/webpack.config.js index 3c2455ebf35..8f2c2f332c5 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -249,6 +249,7 @@ if (IS_DEV_SERVER) { port: DEV_SERVER_PORT, headers: { 'Access-Control-Allow-Origin': '*' }, stats: 'errors-only', + hot: DEV_SERVER_LIVERELOAD, inline: DEV_SERVER_LIVERELOAD }; config.output.publicPath = '//' + DEV_SERVER_HOST + ':' + DEV_SERVER_PORT + config.output.publicPath; @@ -256,6 +257,9 @@ if (IS_DEV_SERVER) { // watch node_modules for changes if we encounter a missing module compile error new WatchMissingNodeModulesPlugin(path.join(ROOT_PATH, 'node_modules')) ); + if (DEV_SERVER_LIVERELOAD) { + config.plugins.push(new webpack.HotModuleReplacementPlugin()); + } } if (WEBPACK_REPORT) { -- cgit v1.2.1 From b97d5b65dd40fb5d8753c0677534e82cb5636f2d Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 16 Jun 2017 14:23:33 +0200 Subject: Use include ActiveModel::Model to hold metrics data parsed from yaml. --- lib/gitlab/prometheus/additional_metrics_parser.rb | 23 +++++++--------------- lib/gitlab/prometheus/metric.rb | 15 +++++++------- lib/gitlab/prometheus/metric_group.rb | 10 +++------- .../prometheus/additional_metrics_parser_spec.rb | 23 +++++++++++----------- spec/support/prometheus/metric_builders.rb | 4 ++-- 5 files changed, 32 insertions(+), 43 deletions(-) diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index 9bd41b99d81..70151e8c6ad 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -9,26 +9,17 @@ module Gitlab private - def metrics_from_list(list) - list.map { |entry| metric_from_entry(entry) } - end - - def metric_from_entry(entry) - required_fields = [:title, :required_metrics, :weight, :queries] - missing_fields = required_fields.select { |key| entry[key].nil? } - raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? - - Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries]) + def validate!(obj) + raise ParsingError.new(obj.errors.full_messages.join('\n')) unless obj.valid? end def group_from_entry(entry) - required_fields = [:group, :priority, :metrics] - missing_fields = required_fields.select { |key| entry[key].nil? } - - raise ParsingError.new("entry missing required fields #{missing_fields.map(&:to_s)}") unless missing_fields.empty? + entry[:name] = entry.delete(:group) + entry[:metrics]&.map! do |entry| + Metric.new(entry).tap(&method(:validate!)) + end - group = MetricGroup.new(entry[:group], entry[:priority]) - group.tap { |g| g.metrics = metrics_from_list(entry[:metrics]) } + MetricGroup.new(entry).tap(&method(:validate!)) end def additional_metrics_raw diff --git a/lib/gitlab/prometheus/metric.rb b/lib/gitlab/prometheus/metric.rb index 5155064317c..f54b2c6aaff 100644 --- a/lib/gitlab/prometheus/metric.rb +++ b/lib/gitlab/prometheus/metric.rb @@ -1,14 +1,15 @@ module Gitlab module Prometheus class Metric - attr_reader :group, :title, :required_metrics, :weight, :y_label, :queries + include ActiveModel::Model - def initialize(title, required_metrics, weight, y_label, queries = []) - @title = title - @required_metrics = required_metrics - @weight = weight - @y_label = y_label || 'Values' - @queries = queries + attr_accessor :title, :required_metrics, :weight, :y_label, :queries + + validates :title, :required_metrics, :weight, :y_label, :queries, presence: true + + def initialize(params = {}) + super(params) + @y_label ||= 'Values' end end end diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb index 12bdf407ce0..729fef34b35 100644 --- a/lib/gitlab/prometheus/metric_group.rb +++ b/lib/gitlab/prometheus/metric_group.rb @@ -1,14 +1,10 @@ module Gitlab module Prometheus class MetricGroup - attr_reader :priority, :name - attr_accessor :metrics + include ActiveModel::Model - def initialize(name, priority, metrics = []) - @name = name - @priority = priority - @metrics = metrics - end + attr_accessor :name, :priority, :metrics + validates :name, :priority, :metrics, presence: true def self.all AdditionalMetricsParser.load_groups_from_yaml diff --git a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb index 97280de173e..f8b2746b43d 100644 --- a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb +++ b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do priority: 1 metrics: - title: title - required_metrics: [] + required_metrics: ['metric_a'] weight: 1 queries: [{query_range: query_range_a}] EOF @@ -54,7 +54,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do expect(metrics.count).to eq(3) expect(metrics[0]).to have_attributes(title: 'title', required_metrics: %w(metric_a metric_b), weight: 1) expect(metrics[1]).to have_attributes(title: 'title', required_metrics: %w(metric_a), weight: 1) - expect(metrics[2]).to have_attributes(title: 'title', required_metrics: [], weight: 1) + expect(metrics[2]).to have_attributes(title: 'title', required_metrics: %w{metric_a}, weight: 1) end it 'provides query data' do @@ -78,7 +78,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end it 'throws parsing error' do - expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/) + expect { subject }.to raise_error(parser_error_class, /#{field_name} can't be blank/i) end end @@ -88,13 +88,13 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end it 'throws parsing error' do - expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/) + expect { subject }.to raise_error(parser_error_class, /#{field_name} can't be blank/i) end end end describe 'group required fields' do - it_behaves_like 'required field', :metrics do + it_behaves_like 'required field', 'metrics' do let(:field_nil) do <<-EOF.strip_heredoc - group: group_a @@ -111,10 +111,11 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end end - it_behaves_like 'required field', :group do + it_behaves_like 'required field', 'name' do let(:field_nil) do <<-EOF.strip_heredoc - - priority: 1 + - group: + priority: 1 metrics: [] EOF end @@ -127,7 +128,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end end - it_behaves_like 'required field', :priority do + it_behaves_like 'required field', 'priority' do let(:field_nil) do <<-EOF.strip_heredoc - group: group_a @@ -146,7 +147,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end describe 'metrics fields parsing' do - it_behaves_like 'required field', :title do + it_behaves_like 'required field', 'title' do let(:field_nil) do <<-EOF.strip_heredoc - group: group_a @@ -171,7 +172,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end end - it_behaves_like 'required field', :required_metrics do + it_behaves_like 'required field', 'required metrics' do let(:field_nil) do <<-EOF.strip_heredoc - group: group_a @@ -196,7 +197,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end end - it_behaves_like 'required field', :weight do + it_behaves_like 'required field', 'weight' do let(:field_nil) do <<-EOF.strip_heredoc - group: group_a diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index e4b55c22acd..c8d056d3fc8 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -9,7 +9,7 @@ module Prometheus end def simple_metric(title: 'title', required_metrics: [], queries: [simple_query]) - Gitlab::Prometheus::Metric.new(title, required_metrics, 1, nil, queries) + Gitlab::Prometheus::Metric.new(title: title, required_metrics: required_metrics, weight: 1, queries: queries) end def simple_metrics(added_metric_name: 'metric_a') @@ -21,7 +21,7 @@ module Prometheus end def simple_metric_group(name: 'name', metrics: simple_metrics) - Gitlab::Prometheus::MetricGroup.new( name, 1, metrics) + Gitlab::Prometheus::MetricGroup.new(name: name, priority: 1, metrics: metrics) end end end -- cgit v1.2.1 From 64bb0d37d4ef1f8574355019f198e40bc9b70224 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 16 Jun 2017 16:53:02 +0200 Subject: cleanup wip --- spec/controllers/projects/deployments_controller_spec.rb | 2 ++ spec/controllers/projects/environments_controller_spec.rb | 12 +++++++----- spec/javascripts/fixtures/prometheus_service.rb | 1 - 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 26d92e787c4..c3b4f812db9 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -42,6 +42,7 @@ describe Projects::DeploymentsController do before do allow(controller).to receive(:deployment).and_return(deployment) end + context 'when metrics are disabled' do before do allow(deployment).to receive(:has_metrics?).and_return false @@ -114,6 +115,7 @@ describe Projects::DeploymentsController do before do allow(controller).to receive(:deployment).and_return(deployment) end + context 'when metrics are disabled' do before do allow(deployment).to receive(:has_metrics?).and_return false diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index ab171cde6bf..749b090d6e0 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -338,11 +338,13 @@ describe Projects::EnvironmentsController do context 'when environment has some metrics' do before do - expect(environment).to receive(:additional_metrics).and_return({ - success: true, - data: {}, - last_update: 42 - }) + expect(environment) + .to receive(:additional_metrics) + .and_return({ + success: true, + data: {}, + last_update: 42 + }) end it 'returns a metrics JSON document' do diff --git a/spec/javascripts/fixtures/prometheus_service.rb b/spec/javascripts/fixtures/prometheus_service.rb index 7dfbf885fbd..4aa6ea3cdfc 100644 --- a/spec/javascripts/fixtures/prometheus_service.rb +++ b/spec/javascripts/fixtures/prometheus_service.rb @@ -8,7 +8,6 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle let(:project) { create(:project_empty_repo, namespace: namespace, path: 'services-project') } let!(:service) { create(:prometheus_service, project: project) } - render_views before(:all) do -- cgit v1.2.1 From be5f665557fc4f5b10d34f407545486746af54b8 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 16 Jun 2017 18:26:40 +0200 Subject: Fix prometheus service frontend fixture --- spec/javascripts/fixtures/prometheus_service.rb | 4 ++-- spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/javascripts/fixtures/prometheus_service.rb b/spec/javascripts/fixtures/prometheus_service.rb index 4aa6ea3cdfc..3200577b326 100644 --- a/spec/javascripts/fixtures/prometheus_service.rb +++ b/spec/javascripts/fixtures/prometheus_service.rb @@ -11,14 +11,14 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle render_views before(:all) do - clean_frontend_fixtures('services/') + clean_frontend_fixtures('services/prometheus') end before(:each) do sign_in(admin) end - it 'services/prometheus_service.html.raw' do |example| + it 'services/prometheus/prometheus_service.html.raw' do |example| get :edit, namespace_id: namespace, project_id: project, diff --git a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js index e7187a8a5e0..2b3a821dbd9 100644 --- a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js +++ b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js @@ -3,7 +3,7 @@ import PANEL_STATE from '~/prometheus_metrics/constants'; import { metrics, missingVarMetrics } from './mock_data'; describe('PrometheusMetrics', () => { - const FIXTURE = 'services/prometheus_service.html.raw'; + const FIXTURE = 'services/prometheus/prometheus_service.html.raw'; preloadFixtures(FIXTURE); beforeEach(() => { -- cgit v1.2.1 From 6e4d5334211d73dd731cb8757b2ef10e8ea428b7 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 16 Jun 2017 20:48:34 +0200 Subject: Move Prometheus service to project model --- app/controllers/projects/deployments_controller.rb | 2 +- app/controllers/projects/prometheus_controller.rb | 8 ++------ app/models/deployment.rb | 12 ++++++------ app/models/environment.rb | 8 ++------ app/models/project.rb | 4 ++++ spec/controllers/projects/deployments_controller_spec.rb | 2 +- spec/controllers/projects/prometheus_controller_spec.rb | 2 +- spec/models/deployment_spec.rb | 7 ++++--- 8 files changed, 21 insertions(+), 24 deletions(-) diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index e7d95e46b06..acf5573935a 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -23,7 +23,7 @@ class Projects::DeploymentsController < Projects::ApplicationController end def additional_metrics - return render_404 unless deployment.prometheus_service.present? + return render_404 unless deployment.has_additional_metrics? metrics = deployment.additional_metrics diff --git a/app/controllers/projects/prometheus_controller.rb b/app/controllers/projects/prometheus_controller.rb index 9609fa44945..7b828981dd8 100644 --- a/app/controllers/projects/prometheus_controller.rb +++ b/app/controllers/projects/prometheus_controller.rb @@ -5,7 +5,7 @@ class Projects::PrometheusController < Projects::ApplicationController def active_metrics respond_to do |format| format.json do - matched_metrics = prometheus_service.matched_metrics || {} + matched_metrics = project.prometheus_service.matched_metrics || {} if matched_metrics.any? render json: matched_metrics @@ -22,11 +22,7 @@ class Projects::PrometheusController < Projects::ApplicationController render_404 end - def prometheus_service - @prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name) - end - def require_prometheus_metrics! - render_404 unless prometheus_service.present? + render_404 unless project.prometheus_service.present? end end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 04d08f2cfd5..00bf0c118ae 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -114,17 +114,17 @@ class Deployment < ActiveRecord::Base project.monitoring_service.deployment_metrics(self) end + def has_additional_metrics? + project.prometheus_service.present? + end + def additional_metrics - return {} unless prometheus_service.present? + return {} unless project.prometheus_service.present? - metrics = prometheus_service.additional_deployment_metrics(self) + metrics = project.prometheus_service.additional_deployment_metrics(self) metrics&.merge(deployment_time: created_at.to_i) || {} end - def prometheus_service - @prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name) - end - private def ref_path diff --git a/app/models/environment.rb b/app/models/environment.rb index 62db7e958ab..b391a487b30 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -158,19 +158,15 @@ class Environment < ActiveRecord::Base end def has_additional_metrics? - prometheus_service.present? && available? && last_deployment.present? + project.prometheus_service.present? && available? && last_deployment.present? end def additional_metrics if has_additional_metrics? - prometheus_service.additional_environment_metrics(self) + project.prometheus_service.additional_environment_metrics(self) end end - def prometheus_service - @prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name) - end - # An environment name is not necessarily suitable for use in URLs, DNS # or other third-party contexts, so provide a slugified version. A slug has # the following properties: diff --git a/app/models/project.rb b/app/models/project.rb index 4c394646787..f6338b92b9f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -798,6 +798,10 @@ class Project < ActiveRecord::Base @monitoring_service ||= monitoring_services.reorder(nil).find_by(active: true) end + def prometheus_service + @prometheus_service ||= monitoring_services.find_by(active: true, type: PrometheusService.name) + end + def jira_tracker? issues_tracker.to_param == 'jira' end diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index c3b4f812db9..4a77d4d8289 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -132,7 +132,7 @@ describe Projects::DeploymentsController do let(:prometheus_service) { double('prometheus_service') } before do - allow(deployment).to receive(:prometheus_service).and_return(prometheus_service) + allow(deployment.project).to receive(:prometheus_service).and_return(prometheus_service) end context 'when environment has no metrics' do diff --git a/spec/controllers/projects/prometheus_controller_spec.rb b/spec/controllers/projects/prometheus_controller_spec.rb index a994ac6409f..eddf7275975 100644 --- a/spec/controllers/projects/prometheus_controller_spec.rb +++ b/spec/controllers/projects/prometheus_controller_spec.rb @@ -8,7 +8,7 @@ describe Projects::PrometheusController do before do allow(controller).to receive(:project).and_return(project) - allow(controller).to receive(:prometheus_service).and_return(prometheus_service) + allow(project).to receive(:prometheus_service).and_return(prometheus_service) project.add_master(user) sign_in(user) diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index eba0175c54c..bb84d3fc13d 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -30,7 +30,7 @@ describe Deployment, models: true do end describe '#includes_commit?' do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository) } let(:environment) { create(:environment, project: project) } let(:deployment) do create(:deployment, environment: environment, sha: project.commit.id) @@ -91,7 +91,8 @@ describe Deployment, models: true do end describe '#additional_metrics' do - let(:deployment) { create(:deployment) } + let(:project) { create(:project) } + let(:deployment) { create(:deployment, project: project) } subject { deployment.additional_metrics } @@ -111,7 +112,7 @@ describe Deployment, models: true do let(:prometheus_service) { double('prometheus_service') } before do - allow(deployment).to receive(:prometheus_service).and_return(prometheus_service) + allow(project).to receive(:prometheus_service).and_return(prometheus_service) allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics) end -- cgit v1.2.1 From 8b69523b014c9557bcb03bf0e695331ea9621312 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 16 Jun 2017 20:54:15 +0200 Subject: move additional_metrics.yaml into prometheus/ config folder --- config/additional_metrics.yml | 32 ---------------------- config/prometheus/additional_metrics.yml | 32 ++++++++++++++++++++++ lib/gitlab/prometheus/additional_metrics_parser.rb | 2 +- spec/models/environment_spec.rb | 2 +- 4 files changed, 34 insertions(+), 34 deletions(-) delete mode 100644 config/additional_metrics.yml create mode 100644 config/prometheus/additional_metrics.yml diff --git a/config/additional_metrics.yml b/config/additional_metrics.yml deleted file mode 100644 index daecde49570..00000000000 --- a/config/additional_metrics.yml +++ /dev/null @@ -1,32 +0,0 @@ -- group: Kubernetes - priority: 1 - metrics: - - title: "Memory usage" - y_label: "Values" - required_metrics: - - container_memory_usage_bytes - weight: 1 - queries: - - query_range: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' - label: Container memory - unit: MiB - - title: "Current memory usage" - required_metrics: - - container_memory_usage_bytes - weight: 1 - queries: - - query: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' - display_empty: false - unit: MiB - - title: "CPU usage" - required_metrics: - - container_cpu_usage_seconds_total - weight: 1 - queries: - - query_range: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' - - title: "Current CPU usage" - required_metrics: - - container_cpu_usage_seconds_total - weight: 1 - queries: - - query: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' diff --git a/config/prometheus/additional_metrics.yml b/config/prometheus/additional_metrics.yml new file mode 100644 index 00000000000..daecde49570 --- /dev/null +++ b/config/prometheus/additional_metrics.yml @@ -0,0 +1,32 @@ +- group: Kubernetes + priority: 1 + metrics: + - title: "Memory usage" + y_label: "Values" + required_metrics: + - container_memory_usage_bytes + weight: 1 + queries: + - query_range: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' + label: Container memory + unit: MiB + - title: "Current memory usage" + required_metrics: + - container_memory_usage_bytes + weight: 1 + queries: + - query: 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20' + display_empty: false + unit: MiB + - title: "CPU usage" + required_metrics: + - container_cpu_usage_seconds_total + weight: 1 + queries: + - query_range: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' + - title: "Current CPU usage" + required_metrics: + - container_cpu_usage_seconds_total + weight: 1 + queries: + - query: 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100' diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index 70151e8c6ad..c07afbac9ea 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -27,7 +27,7 @@ module Gitlab end def load_yaml_file - YAML.load_file(Rails.root.join('config/additional_metrics.yml')) + YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index f36c165ef7d..b0635c6a90a 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -475,7 +475,7 @@ describe Environment, models: true do end it 'returns the additional metrics from the deployment service' do - expect(environment.prometheus_service).to receive(:additional_environment_metrics) + expect(project.prometheus_service).to receive(:additional_environment_metrics) .with(environment) .and_return(:fake_metrics) -- cgit v1.2.1 From 13902e40a8c11ca8d19bd7dc7e7c44fc62ee31dc Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 16 Jun 2017 20:58:18 +0200 Subject: Memoize only yaml loading method --- lib/gitlab/prometheus/additional_metrics_parser.rb | 4 ++-- spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index c07afbac9ea..cb95daf2260 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -23,11 +23,11 @@ module Gitlab end def additional_metrics_raw - @additional_metrics_raw ||= load_yaml_file&.map(&:deep_symbolize_keys).freeze + load_yaml_file&.map(&:deep_symbolize_keys).freeze end def load_yaml_file - YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) + @loaded_yaml_file ||= YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) end end end diff --git a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb index f8b2746b43d..61d48b05454 100644 --- a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb +++ b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb @@ -33,7 +33,6 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end before do - described_class.instance_variable_set :@additional_metrics_raw, nil allow(described_class).to receive(:load_yaml_file) { YAML.load(sample_yaml) } end @@ -68,10 +67,6 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do end shared_examples 'required field' do |field_name| - before do - described_class.instance_variable_set :@additional_metrics_raw, nil - end - context "when #{field_name} is nil" do before do allow(described_class).to receive(:load_yaml_file) { YAML.load(field_missing) } -- cgit v1.2.1 From 3b4c07ac5fd173786d3e587a5af9ea27bfd0a707 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 16 Jun 2017 16:56:32 -0500 Subject: refactor AwardsHandler into es class syntax --- app/assets/javascripts/awards_handler.js | 753 +++++++++++++++---------------- 1 file changed, 374 insertions(+), 379 deletions(-) diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index adb45b0606d..ebe722061d7 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -1,3 +1,4 @@ +/* eslint-disable class-methods-use-this */ /* global Flash */ import Cookies from 'js-cookie'; @@ -68,141 +69,140 @@ function renderCategory(name, emojiList, opts = {}) { `; } -function AwardsHandler() { - this.eventListeners = []; - this.aliases = emojiAliases; - // If the user shows intent let's pre-build the menu - this.registerEventListener('one', $(document), 'mouseenter focus', '.js-add-award', 'mouseenter focus', () => { - const $menu = $('.emoji-menu'); - if ($menu.length === 0) { - requestAnimationFrame(() => { - this.createEmojiMenu(); - }); - } - // Prebuild the categoryMap - categoryMap = categoryMap || buildCategoryMap(); - }); - this.registerEventListener('on', $(document), 'click', '.js-add-award', (e) => { - e.stopPropagation(); - e.preventDefault(); - this.showEmojiMenu($(e.currentTarget)); - }); +export default class AwardsHandler { + constructor() { + this.eventListeners = []; + this.aliases = emojiAliases; + // If the user shows intent let's pre-build the menu + this.registerEventListener('one', $(document), 'mouseenter focus', '.js-add-award', 'mouseenter focus', () => { + const $menu = $('.emoji-menu'); + if ($menu.length === 0) { + requestAnimationFrame(() => { + this.createEmojiMenu(); + }); + } + // Prebuild the categoryMap + categoryMap = categoryMap || buildCategoryMap(); + }); + this.registerEventListener('on', $(document), 'click', '.js-add-award', (e) => { + e.stopPropagation(); + e.preventDefault(); + this.showEmojiMenu($(e.currentTarget)); + }); - this.registerEventListener('on', $('html'), 'click', (e) => { - const $target = $(e.target); - if (!$target.closest('.emoji-menu-content').length) { - $('.js-awards-block.current').removeClass('current'); - } - if (!$target.closest('.emoji-menu').length) { - if ($('.emoji-menu').is(':visible')) { - $('.js-add-award.is-active').removeClass('is-active'); - $('.emoji-menu').removeClass('is-visible'); + this.registerEventListener('on', $('html'), 'click', (e) => { + const $target = $(e.target); + if (!$target.closest('.emoji-menu-content').length) { + $('.js-awards-block.current').removeClass('current'); } - } - }); - this.registerEventListener('on', $(document), 'click', '.js-emoji-btn', (e) => { - e.preventDefault(); - const $target = $(e.currentTarget); - const $glEmojiElement = $target.find('gl-emoji'); - const $spriteIconElement = $target.find('.icon'); - const emoji = ($glEmojiElement.length ? $glEmojiElement : $spriteIconElement).data('name'); - - $target.closest('.js-awards-block').addClass('current'); - this.addAward(this.getVotesBlock(), this.getAwardUrl(), emoji); - }); -} + if (!$target.closest('.emoji-menu').length) { + if ($('.emoji-menu').is(':visible')) { + $('.js-add-award.is-active').removeClass('is-active'); + $('.emoji-menu').removeClass('is-visible'); + } + } + }); + this.registerEventListener('on', $(document), 'click', '.js-emoji-btn', (e) => { + e.preventDefault(); + const $target = $(e.currentTarget); + const $glEmojiElement = $target.find('gl-emoji'); + const $spriteIconElement = $target.find('.icon'); + const emoji = ($glEmojiElement.length ? $glEmojiElement : $spriteIconElement).data('name'); + + $target.closest('.js-awards-block').addClass('current'); + this.addAward(this.getVotesBlock(), this.getAwardUrl(), emoji); + }); + } -AwardsHandler.prototype.registerEventListener = function registerEventListener(method = 'on', element, ...args) { - element[method].call(element, ...args); - this.eventListeners.push({ - element, - args, - }); -}; + registerEventListener(method = 'on', element, ...args) { + element[method].call(element, ...args); + this.eventListeners.push({ + element, + args, + }); + } -AwardsHandler.prototype.showEmojiMenu = function showEmojiMenu($addBtn) { - if ($addBtn.hasClass('js-note-emoji')) { - $addBtn.closest('.note').find('.js-awards-block').addClass('current'); - } else { - $addBtn.closest('.js-awards-block').addClass('current'); - } - - const $menu = $('.emoji-menu'); - const $thumbsBtn = $menu.find('[data-name="thumbsup"], [data-name="thumbsdown"]').parent(); - const $userAuthored = this.isUserAuthored($addBtn); - if ($menu.length) { - if ($menu.is('.is-visible')) { - $addBtn.removeClass('is-active'); - $menu.removeClass('is-visible'); - $('.js-emoji-menu-search').blur(); + showEmojiMenu($addBtn) { + if ($addBtn.hasClass('js-note-emoji')) { + $addBtn.closest('.note').find('.js-awards-block').addClass('current'); } else { - $addBtn.addClass('is-active'); - this.positionMenu($menu, $addBtn); - $menu.addClass('is-visible'); - $('.js-emoji-menu-search').focus(); + $addBtn.closest('.js-awards-block').addClass('current'); } - } else { - $addBtn.addClass('is-loading is-active'); - this.createEmojiMenu(() => { - const $createdMenu = $('.emoji-menu'); - $addBtn.removeClass('is-loading'); - this.positionMenu($createdMenu, $addBtn); - return setTimeout(() => { - $createdMenu.addClass('is-visible'); + + const $menu = $('.emoji-menu'); + const $thumbsBtn = $menu.find('[data-name="thumbsup"], [data-name="thumbsdown"]').parent(); + const $userAuthored = this.isUserAuthored($addBtn); + if ($menu.length) { + if ($menu.is('.is-visible')) { + $addBtn.removeClass('is-active'); + $menu.removeClass('is-visible'); + $('.js-emoji-menu-search').blur(); + } else { + $addBtn.addClass('is-active'); + this.positionMenu($menu, $addBtn); + $menu.addClass('is-visible'); $('.js-emoji-menu-search').focus(); - }, 200); - }); + } + } else { + $addBtn.addClass('is-loading is-active'); + this.createEmojiMenu(() => { + const $createdMenu = $('.emoji-menu'); + $addBtn.removeClass('is-loading'); + this.positionMenu($createdMenu, $addBtn); + return setTimeout(() => { + $createdMenu.addClass('is-visible'); + $('.js-emoji-menu-search').focus(); + }, 200); + }); + } + + $thumbsBtn.toggleClass('disabled', $userAuthored); } - $thumbsBtn.toggleClass('disabled', $userAuthored); -}; + // Create the emoji menu with the first category of emojis. + // Then render the remaining categories of emojis one by one to avoid jank. + createEmojiMenu(callback) { + if (this.isCreatingEmojiMenu) { + return; + } + this.isCreatingEmojiMenu = true; -// Create the emoji menu with the first category of emojis. -// Then render the remaining categories of emojis one by one to avoid jank. -AwardsHandler.prototype.createEmojiMenu = function createEmojiMenu(callback) { - if (this.isCreatingEmojiMenu) { - return; - } - this.isCreatingEmojiMenu = true; - - // Render the first category - categoryMap = categoryMap || buildCategoryMap(); - const categoryNameKey = Object.keys(categoryMap)[0]; - const emojisInCategory = categoryMap[categoryNameKey]; - const firstCategory = renderCategory(categoryLabelMap[categoryNameKey], emojisInCategory); - - // Render the frequently used - const frequentlyUsedEmojis = this.getFrequentlyUsedEmojis(); - let frequentlyUsedCatgegory = ''; - if (frequentlyUsedEmojis.length > 0) { - frequentlyUsedCatgegory = renderCategory('Frequently used', frequentlyUsedEmojis, { - menuListClass: 'frequent-emojis', - }); - } + // Render the first category + categoryMap = categoryMap || buildCategoryMap(); + const categoryNameKey = Object.keys(categoryMap)[0]; + const emojisInCategory = categoryMap[categoryNameKey]; + const firstCategory = renderCategory(categoryLabelMap[categoryNameKey], emojisInCategory); + + // Render the frequently used + const frequentlyUsedEmojis = this.getFrequentlyUsedEmojis(); + let frequentlyUsedCatgegory = ''; + if (frequentlyUsedEmojis.length > 0) { + frequentlyUsedCatgegory = renderCategory('Frequently used', frequentlyUsedEmojis, { + menuListClass: 'frequent-emojis', + }); + } - const emojiMenuMarkup = ` -
    - + const emojiMenuMarkup = ` +
    + -
    - ${frequentlyUsedCatgegory} - ${firstCategory} +
    + ${frequentlyUsedCatgegory} + ${firstCategory} +
    -
    - `; + `; - document.body.insertAdjacentHTML('beforeend', emojiMenuMarkup); + document.body.insertAdjacentHTML('beforeend', emojiMenuMarkup); - this.addRemainingEmojiMenuCategories(); - this.setupSearch(); - if (callback) { - callback(); + this.addRemainingEmojiMenuCategories(); + this.setupSearch(); + if (callback) { + callback(); + } } -}; -AwardsHandler - .prototype - .addRemainingEmojiMenuCategories = function addRemainingEmojiMenuCategories() { + addRemainingEmojiMenuCategories() { if (this.isAddingRemainingEmojiMenuCategories) { return; } @@ -243,176 +243,174 @@ AwardsHandler emojiContentElement.insertAdjacentHTML('beforeend', '

    We encountered an error while adding the remaining categories

    '); throw new Error(`Error occurred in addRemainingEmojiMenuCategories: ${err.message}`); }); - }; - -AwardsHandler.prototype.positionMenu = function positionMenu($menu, $addBtn) { - const position = $addBtn.data('position'); - // The menu could potentially be off-screen or in a hidden overflow element - // So we position the element absolute in the body - const css = { - top: `${$addBtn.offset().top + $addBtn.outerHeight()}px`, - }; - if (position === 'right') { - css.left = `${($addBtn.offset().left - $menu.outerWidth()) + 20}px`; - $menu.addClass('is-aligned-right'); - } else { - css.left = `${$addBtn.offset().left}px`; - $menu.removeClass('is-aligned-right'); - } - return $menu.css(css); -}; - -AwardsHandler.prototype.addAward = function addAward( - votesBlock, - awardUrl, - emoji, - checkMutuality, - callback, -) { - const normalizedEmoji = this.normalizeEmojiName(emoji); - const $emojiButton = this.findEmojiIcon(votesBlock, normalizedEmoji).parent(); - this.postEmoji($emojiButton, awardUrl, normalizedEmoji, () => { - this.addAwardToEmojiBar(votesBlock, normalizedEmoji, checkMutuality); - return typeof callback === 'function' ? callback() : undefined; - }); - $('.emoji-menu').removeClass('is-visible'); - $('.js-add-award.is-active').removeClass('is-active'); -}; + } -AwardsHandler.prototype.addAwardToEmojiBar = function addAwardToEmojiBar( - votesBlock, - emoji, - checkForMutuality, -) { - if (checkForMutuality || checkForMutuality === null) { - this.checkMutuality(votesBlock, emoji); - } - this.addEmojiToFrequentlyUsedList(emoji); - const normalizedEmoji = this.normalizeEmojiName(emoji); - const $emojiButton = this.findEmojiIcon(votesBlock, normalizedEmoji).parent(); - if ($emojiButton.length > 0) { - if (this.isActive($emojiButton)) { - this.decrementCounter($emojiButton, normalizedEmoji); + positionMenu($menu, $addBtn) { + const position = $addBtn.data('position'); + // The menu could potentially be off-screen or in a hidden overflow element + // So we position the element absolute in the body + const css = { + top: `${$addBtn.offset().top + $addBtn.outerHeight()}px`, + }; + if (position === 'right') { + css.left = `${($addBtn.offset().left - $menu.outerWidth()) + 20}px`; + $menu.addClass('is-aligned-right'); } else { - const counter = $emojiButton.find('.js-counter'); - counter.text(parseInt(counter.text(), 10) + 1); - $emojiButton.addClass('active'); - this.addYouToUserList(votesBlock, normalizedEmoji); - this.animateEmoji($emojiButton); + css.left = `${$addBtn.offset().left}px`; + $menu.removeClass('is-aligned-right'); } - } else { - votesBlock.removeClass('hidden'); - this.createEmoji(votesBlock, normalizedEmoji); + return $menu.css(css); } -}; -AwardsHandler.prototype.getVotesBlock = function getVotesBlock() { - const currentBlock = $('.js-awards-block.current'); - let resultantVotesBlock = currentBlock; - if (currentBlock.length === 0) { - resultantVotesBlock = $('.js-awards-block').eq(0); + addAward( + votesBlock, + awardUrl, + emoji, + checkMutuality, + callback, + ) { + const normalizedEmoji = this.normalizeEmojiName(emoji); + const $emojiButton = this.findEmojiIcon(votesBlock, normalizedEmoji).parent(); + this.postEmoji($emojiButton, awardUrl, normalizedEmoji, () => { + this.addAwardToEmojiBar(votesBlock, normalizedEmoji, checkMutuality); + return typeof callback === 'function' ? callback() : undefined; + }); + $('.emoji-menu').removeClass('is-visible'); + $('.js-add-award.is-active').removeClass('is-active'); } - return resultantVotesBlock; -}; + addAwardToEmojiBar( + votesBlock, + emoji, + checkForMutuality, + ) { + if (checkForMutuality || checkForMutuality === null) { + this.checkMutuality(votesBlock, emoji); + } + this.addEmojiToFrequentlyUsedList(emoji); + const normalizedEmoji = this.normalizeEmojiName(emoji); + const $emojiButton = this.findEmojiIcon(votesBlock, normalizedEmoji).parent(); + if ($emojiButton.length > 0) { + if (this.isActive($emojiButton)) { + this.decrementCounter($emojiButton, normalizedEmoji); + } else { + const counter = $emojiButton.find('.js-counter'); + counter.text(parseInt(counter.text(), 10) + 1); + $emojiButton.addClass('active'); + this.addYouToUserList(votesBlock, normalizedEmoji); + this.animateEmoji($emojiButton); + } + } else { + votesBlock.removeClass('hidden'); + this.createEmoji(votesBlock, normalizedEmoji); + } + } -AwardsHandler.prototype.getAwardUrl = function getAwardUrl() { - return this.getVotesBlock().data('award-url'); -}; + getVotesBlock() { + const currentBlock = $('.js-awards-block.current'); + let resultantVotesBlock = currentBlock; + if (currentBlock.length === 0) { + resultantVotesBlock = $('.js-awards-block').eq(0); + } + + return resultantVotesBlock; + } -AwardsHandler.prototype.checkMutuality = function checkMutuality(votesBlock, emoji) { - const awardUrl = this.getAwardUrl(); - if (emoji === 'thumbsup' || emoji === 'thumbsdown') { - const mutualVote = emoji === 'thumbsup' ? 'thumbsdown' : 'thumbsup'; - const $emojiButton = votesBlock.find(`[data-name="${mutualVote}"]`).parent(); - const isAlreadyVoted = $emojiButton.hasClass('active'); - if (isAlreadyVoted) { - this.addAward(votesBlock, awardUrl, mutualVote, false); + getAwardUrl() { + return this.getVotesBlock().data('award-url'); + } + + checkMutuality(votesBlock, emoji) { + const awardUrl = this.getAwardUrl(); + if (emoji === 'thumbsup' || emoji === 'thumbsdown') { + const mutualVote = emoji === 'thumbsup' ? 'thumbsdown' : 'thumbsup'; + const $emojiButton = votesBlock.find(`[data-name="${mutualVote}"]`).parent(); + const isAlreadyVoted = $emojiButton.hasClass('active'); + if (isAlreadyVoted) { + this.addAward(votesBlock, awardUrl, mutualVote, false); + } } } -}; -AwardsHandler.prototype.isActive = function isActive($emojiButton) { - return $emojiButton.hasClass('active'); -}; + isActive($emojiButton) { + return $emojiButton.hasClass('active'); + } -AwardsHandler.prototype.isUserAuthored = function isUserAuthored($button) { - return $button.hasClass('js-user-authored'); -}; + isUserAuthored($button) { + return $button.hasClass('js-user-authored'); + } -AwardsHandler.prototype.decrementCounter = function decrementCounter($emojiButton, emoji) { - const counter = $('.js-counter', $emojiButton); - const counterNumber = parseInt(counter.text(), 10); - if (counterNumber > 1) { - counter.text(counterNumber - 1); - this.removeYouFromUserList($emojiButton); - } else if (emoji === 'thumbsup' || emoji === 'thumbsdown') { - $emojiButton.tooltip('destroy'); - counter.text('0'); - this.removeYouFromUserList($emojiButton); - if ($emojiButton.parents('.note').length) { + decrementCounter($emojiButton, emoji) { + const counter = $('.js-counter', $emojiButton); + const counterNumber = parseInt(counter.text(), 10); + if (counterNumber > 1) { + counter.text(counterNumber - 1); + this.removeYouFromUserList($emojiButton); + } else if (emoji === 'thumbsup' || emoji === 'thumbsdown') { + $emojiButton.tooltip('destroy'); + counter.text('0'); + this.removeYouFromUserList($emojiButton); + if ($emojiButton.parents('.note').length) { + this.removeEmoji($emojiButton); + } + } else { this.removeEmoji($emojiButton); } - } else { - this.removeEmoji($emojiButton); + return $emojiButton.removeClass('active'); } - return $emojiButton.removeClass('active'); -}; -AwardsHandler.prototype.removeEmoji = function removeEmoji($emojiButton) { - $emojiButton.tooltip('destroy'); - $emojiButton.remove(); - const $votesBlock = this.getVotesBlock(); - if ($votesBlock.find('.js-emoji-btn').length === 0) { - $votesBlock.addClass('hidden'); + removeEmoji($emojiButton) { + $emojiButton.tooltip('destroy'); + $emojiButton.remove(); + const $votesBlock = this.getVotesBlock(); + if ($votesBlock.find('.js-emoji-btn').length === 0) { + $votesBlock.addClass('hidden'); + } } -}; - -AwardsHandler.prototype.getAwardTooltip = function getAwardTooltip($awardBlock) { - return $awardBlock.attr('data-original-title') || $awardBlock.attr('data-title') || ''; -}; -AwardsHandler.prototype.toSentence = function toSentence(list) { - let sentence; - if (list.length <= 2) { - sentence = list.join(' and '); - } else { - sentence = `${list.slice(0, -1).join(', ')}, and ${list[list.length - 1]}`; + getAwardTooltip($awardBlock) { + return $awardBlock.attr('data-original-title') || $awardBlock.attr('data-title') || ''; } - return sentence; -}; + toSentence(list) { + let sentence; + if (list.length <= 2) { + sentence = list.join(' and '); + } else { + sentence = `${list.slice(0, -1).join(', ')}, and ${list[list.length - 1]}`; + } -AwardsHandler.prototype.removeYouFromUserList = function removeYouFromUserList($emojiButton) { - const awardBlock = $emojiButton; - const originalTitle = this.getAwardTooltip(awardBlock); - const authors = originalTitle.split(FROM_SENTENCE_REGEX); - authors.splice(authors.indexOf('You'), 1); - return awardBlock - .closest('.js-emoji-btn') - .removeData('title') - .removeAttr('data-title') - .removeAttr('data-original-title') - .attr('title', this.toSentence(authors)) - .tooltip('fixTitle'); -}; + return sentence; + } -AwardsHandler.prototype.addYouToUserList = function addYouToUserList(votesBlock, emoji) { - const awardBlock = this.findEmojiIcon(votesBlock, emoji).parent(); - const origTitle = this.getAwardTooltip(awardBlock); - let users = []; - if (origTitle) { - users = origTitle.trim().split(FROM_SENTENCE_REGEX); - } - users.unshift('You'); - return awardBlock - .attr('title', this.toSentence(users)) - .tooltip('fixTitle'); -}; + removeYouFromUserList($emojiButton) { + const awardBlock = $emojiButton; + const originalTitle = this.getAwardTooltip(awardBlock); + const authors = originalTitle.split(FROM_SENTENCE_REGEX); + authors.splice(authors.indexOf('You'), 1); + return awardBlock + .closest('.js-emoji-btn') + .removeData('title') + .removeAttr('data-title') + .removeAttr('data-original-title') + .attr('title', this.toSentence(authors)) + .tooltip('fixTitle'); + } + + addYouToUserList(votesBlock, emoji) { + const awardBlock = this.findEmojiIcon(votesBlock, emoji).parent(); + const origTitle = this.getAwardTooltip(awardBlock); + let users = []; + if (origTitle) { + users = origTitle.trim().split(FROM_SENTENCE_REGEX); + } + users.unshift('You'); + return awardBlock + .attr('title', this.toSentence(users)) + .tooltip('fixTitle'); + } -AwardsHandler - .prototype - .createAwardButtonForVotesBlock = function createAwardButtonForVotesBlock(votesBlock, emojiName) { + createAwardButtonForVotesBlock(votesBlock, emojiName) { const buttonHtml = `
    -