diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-03-06 16:31:04 +0000 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-03-06 16:31:04 +0000 |
commit | 348c16e7d36435b3c3c5577d622f173278c49814 (patch) | |
tree | b55c97195105ac5f8854e0f448fa8a7b4dddf3bd | |
parent | 9a8f5a2b605f85ace3c81a32cf1855f79cabde43 (diff) | |
parent | 4ff8db0d2e8371dfdae2ddef8a8595c1ef80c3d4 (diff) | |
download | gitlab-ce-348c16e7d36435b3c3c5577d622f173278c49814.tar.gz |
Merge branch '5029-support-cluster-metrics-ce' into 'master'
Refactoring changes to support cluster metrics in EE
Closes #42820
See merge request gitlab-org/gitlab-ce!17336
30 files changed, 403 insertions, 370 deletions
diff --git a/app/assets/javascripts/monitoring/utils/multiple_time_series.js b/app/assets/javascripts/monitoring/utils/multiple_time_series.js index 4ce3dad440c..b5b8e3c255d 100644 --- a/app/assets/javascripts/monitoring/utils/multiple_time_series.js +++ b/app/assets/javascripts/monitoring/utils/multiple_time_series.js @@ -76,7 +76,7 @@ function queryTimeSeries(query, graphWidth, graphHeight, graphHeightOffset, xDom metricTag = seriesCustomizationData.value || timeSeriesMetricLabel; [lineColor, areaColor] = pickColor(seriesCustomizationData.color); } else { - metricTag = timeSeriesMetricLabel || `series ${timeSeriesNumber + 1}`; + metricTag = timeSeriesMetricLabel || query.label || `series ${timeSeriesNumber + 1}`; [lineColor, areaColor] = pickColor(); } diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index 1a418d0f15a..b68cdc39cb8 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -24,7 +24,7 @@ class Projects::DeploymentsController < Projects::ApplicationController end def additional_metrics - return render_404 unless deployment.has_additional_metrics? + return render_404 unless deployment.has_metrics? respond_to do |format| format.json do diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb index b739d0f0f90..1dd886409a5 100644 --- a/app/controllers/projects/prometheus/metrics_controller.rb +++ b/app/controllers/projects/prometheus/metrics_controller.rb @@ -2,11 +2,12 @@ module Projects module Prometheus class MetricsController < Projects::ApplicationController before_action :authorize_admin_project! + before_action :require_prometheus_metrics! def active_common respond_to do |format| format.json do - matched_metrics = prometheus_service.matched_metrics || {} + matched_metrics = prometheus_adapter.query(:matched_metrics) || {} if matched_metrics.any? render json: matched_metrics @@ -19,8 +20,12 @@ module Projects private - def prometheus_service - @prometheus_service ||= project.find_or_initialize_service('prometheus') + def prometheus_adapter + @prometheus_adapter ||= ::Prometheus::AdapterService.new(project).prometheus_adapter + end + + def require_prometheus_metrics! + render_404 unless prometheus_adapter.can_query? end end end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index 89ebd63e605..7b25d8c4089 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -1,6 +1,8 @@ module Clusters module Applications class Prometheus < ActiveRecord::Base + include PrometheusAdapter + VERSION = "2.0.0".freeze self.table_name = 'clusters_applications_prometheus' @@ -39,7 +41,7 @@ module Clusters ) end - def proxy_client + def prometheus_client return unless kube_client proxy_url = kube_client.proxy_url('service', service_name, service_port, Gitlab::Kubernetes::Helm::NAMESPACE) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 1c0046107d7..49eb069016a 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -51,9 +51,6 @@ module Clusters scope :enabled, -> { where(enabled: true) } scope :disabled, -> { where(enabled: false) } - scope :for_environment, -> (env) { where(environment_scope: ['*', '', env.slug]) } - scope :for_all_environments, -> { where(environment_scope: ['*', '']) } - def status_name if provider provider.status_name diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb new file mode 100644 index 00000000000..18cbbd871a1 --- /dev/null +++ b/app/models/concerns/prometheus_adapter.rb @@ -0,0 +1,48 @@ +module PrometheusAdapter + extend ActiveSupport::Concern + + included do + include ReactiveCaching + + self.reactive_cache_key = ->(adapter) { [adapter.class.model_name.singular, adapter.id] } + self.reactive_cache_lease_timeout = 30.seconds + self.reactive_cache_refresh_interval = 30.seconds + self.reactive_cache_lifetime = 1.minute + + def prometheus_client + raise NotImplementedError + end + + def prometheus_client_wrapper + Gitlab::PrometheusClient.new(prometheus_client) + end + + def can_query? + prometheus_client.present? + end + + def query(query_name, *args) + return unless can_query? + + query_class = Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query") + + args.map!(&:id) + + with_reactive_cache(query_class.name, *args, &query_class.method(:transform_reactive_result)) + end + + # Cache metrics for specific environment + def calculate_reactive_cache(query_class_name, *args) + return unless prometheus_client + + data = Kernel.const_get(query_class_name).new(prometheus_client_wrapper).query(*args) + { + success: true, + data: data, + last_update: Time.now.utc + } + rescue Gitlab::PrometheusClient::Error => err + { success: false, result: err.message } + end + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index b6cf168d60e..66e61c06765 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -98,28 +98,29 @@ class Deployment < ActiveRecord::Base end def has_metrics? - project.monitoring_service.present? + prometheus_adapter&.can_query? end def metrics return {} unless has_metrics? - project.monitoring_service.deployment_metrics(self) - end - - def has_additional_metrics? - project.prometheus_service.present? + metrics = prometheus_adapter.query(:deployment, self) + metrics&.merge(deployment_time: created_at.to_i) || {} end def additional_metrics - return {} unless project.prometheus_service.present? + return {} unless has_metrics? - metrics = project.prometheus_service.additional_deployment_metrics(self) + metrics = prometheus_adapter.query(:additional_metrics_deployment, self) metrics&.merge(deployment_time: created_at.to_i) || {} end private + def prometheus_adapter + environment.prometheus_adapter + end + def ref_path File.join(environment.ref_path, 'deployments', iid.to_s) end diff --git a/app/models/environment.rb b/app/models/environment.rb index f78c21aebe5..24d4f1d8761 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -146,21 +146,19 @@ class Environment < ActiveRecord::Base end def has_metrics? - project.monitoring_service.present? && available? && last_deployment.present? + prometheus_adapter&.can_query? && available? && last_deployment.present? end def metrics - project.monitoring_service.environment_metrics(self) if has_metrics? + prometheus_adapter.query(:environment, self) if has_metrics? end - def has_additional_metrics? - project.prometheus_service.present? && available? && last_deployment.present? + def additional_metrics + prometheus_adapter.query(:additional_metrics_environment, self) if has_metrics? end - def additional_metrics - if has_additional_metrics? - project.prometheus_service.additional_environment_metrics(self) - end + def prometheus_adapter + @prometheus_adapter ||= Prometheus::AdapterService.new(project, deployment_platform).prometheus_adapter end def slug @@ -226,6 +224,10 @@ class Environment < ActiveRecord::Base self.environment_type || self.name end + def deployment_platform + project.deployment_platform + end + private # Slugifying a name may remove the uniqueness guarantee afforded by it being diff --git a/app/models/project_services/monitoring_service.rb b/app/models/project_services/monitoring_service.rb index ee9cd78327a..9af68b4e821 100644 --- a/app/models/project_services/monitoring_service.rb +++ b/app/models/project_services/monitoring_service.rb @@ -9,11 +9,11 @@ class MonitoringService < Service %w() end - def environment_metrics(environment) + def can_query? raise NotImplementedError end - def deployment_metrics(deployment) + def query(_, *_) raise NotImplementedError end end diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 58731451429..dcaeb65dc32 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -1,9 +1,5 @@ class PrometheusService < MonitoringService - include ReactiveService - - self.reactive_cache_lease_timeout = 30.seconds - self.reactive_cache_refresh_interval = 30.seconds - self.reactive_cache_lifetime = 1.minute + include PrometheusAdapter # Access to prometheus is directly through the API prop_accessor :api_url @@ -13,7 +9,7 @@ class PrometheusService < MonitoringService validates :api_url, url: true end - before_save :synchronize_service_state! + before_save :synchronize_service_state after_save :clear_reactive_cache! @@ -66,63 +62,15 @@ class PrometheusService < MonitoringService # Check we can connect to the Prometheus API def test(*args) - client.ping + Gitlab::PrometheusClient.new(prometheus_client).ping { success: true, result: 'Checked API endpoint' } rescue Gitlab::PrometheusClient::Error => err { success: false, result: err } end - def environment_metrics(environment) - with_reactive_cache(Gitlab::Prometheus::Queries::EnvironmentQuery.name, environment.id, &rename_field(:data, :metrics)) - end - - def deployment_metrics(deployment) - metrics = with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.environment.id, deployment.id, &rename_field(:data, :metrics)) - metrics&.merge(deployment_time: deployment.created_at.to_i) || {} - end - - 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.environment.id, deployment.id, &:itself) - end - - def matched_metrics - with_reactive_cache(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) - end - - # Cache metrics for specific environment - def calculate_reactive_cache(query_class_name, *args) - return unless active? && project && !project.pending_delete? - - environment_id = args.first - client = client(environment_id) - - data = Kernel.const_get(query_class_name).new(client).query(*args) - { - success: true, - data: data, - last_update: Time.now.utc - } - rescue Gitlab::PrometheusClient::Error => err - { success: false, result: err.message } - end - - def client(environment_id = nil) - if manual_configuration? - Gitlab::PrometheusClient.new(RestClient::Resource.new(api_url)) - else - cluster = cluster_with_prometheus(environment_id) - raise Gitlab::PrometheusClient::Error, "couldn't find cluster with Prometheus installed" unless cluster - - rest_client = client_from_cluster(cluster) - raise Gitlab::PrometheusClient::Error, "couldn't create proxy Prometheus client" unless rest_client - - Gitlab::PrometheusClient.new(rest_client) - end + def prometheus_client + RestClient::Resource.new(api_url) if api_url && manual_configuration? && active? end def prometheus_installed? @@ -134,32 +82,7 @@ class PrometheusService < MonitoringService private - def cluster_with_prometheus(environment_id = nil) - clusters = if environment_id - ::Environment.find_by(id: environment_id).try do |env| - # sort results by descending order based on environment_scope being longer - # thus more closely matching environment slug - project.clusters.enabled.for_environment(env).sort_by { |c| c.environment_scope&.length }.reverse! - end - else - project.clusters.enabled.for_all_environments - end - - clusters&.detect { |cluster| cluster.application_prometheus&.installed? } - end - - def client_from_cluster(cluster) - cluster.application_prometheus.proxy_client - end - - def rename_field(old_field, new_field) - -> (metrics) do - metrics[new_field] = metrics.delete(old_field) - metrics - end - end - - def synchronize_service_state! + def synchronize_service_state self.active = prometheus_installed? || manual_configuration? true diff --git a/app/services/prometheus/adapter_service.rb b/app/services/prometheus/adapter_service.rb new file mode 100644 index 00000000000..4504d2ccfe6 --- /dev/null +++ b/app/services/prometheus/adapter_service.rb @@ -0,0 +1,36 @@ +module Prometheus + class AdapterService + def initialize(project, deployment_platform = nil) + @project = project + + @deployment_platform = if deployment_platform + deployment_platform + else + project.deployment_platform + end + end + + attr_reader :deployment_platform, :project + + def prometheus_adapter + @prometheus_adapter ||= if service_prometheus_adapter.can_query? + service_prometheus_adapter + else + cluster_prometheus_adapter + end + end + + def service_prometheus_adapter + project.find_or_initialize_service('prometheus') + end + + def cluster_prometheus_adapter + return unless deployment_platform.respond_to?(:cluster) + + cluster = deployment_platform.cluster + return unless cluster.application_prometheus&.installed? + + cluster.application_prometheus + end + end +end diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index cb95daf2260..bb1172f82a1 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -1,10 +1,12 @@ module Gitlab module Prometheus module AdditionalMetricsParser + CONFIG_ROOT = 'config/prometheus'.freeze + MUTEX = Mutex.new extend self - def load_groups_from_yaml - additional_metrics_raw.map(&method(:group_from_entry)) + def load_groups_from_yaml(file_name = 'additional_metrics.yml') + yaml_metrics_raw(file_name).map(&method(:group_from_entry)) end private @@ -22,13 +24,20 @@ module Gitlab MetricGroup.new(entry).tap(&method(:validate!)) end - def additional_metrics_raw - load_yaml_file&.map(&:deep_symbolize_keys).freeze + def yaml_metrics_raw(file_name) + load_yaml_file(file_name)&.map(&:deep_symbolize_keys).freeze end - def load_yaml_file - @loaded_yaml_file ||= YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) + # rubocop:disable Gitlab/ModuleWithInstanceVariables + def load_yaml_file(file_name) + return YAML.load_file(Rails.root.join(CONFIG_ROOT, file_name)) if Rails.env.development? + + MUTEX.synchronize do + @loaded_yaml_cache ||= {} + @loaded_yaml_cache[file_name] ||= YAML.load_file(Rails.root.join(CONFIG_ROOT, file_name)) + end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables 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 index 972ab75d1d5..e677ec84cd4 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb @@ -4,7 +4,7 @@ module Gitlab class AdditionalMetricsDeploymentQuery < BaseQuery include QueryAdditionalMetrics - def query(environment_id, deployment_id) + def query(deployment_id) Deployment.find_by(id: deployment_id).try do |deployment| query_metrics( deployment.project, diff --git a/lib/gitlab/prometheus/queries/base_query.rb b/lib/gitlab/prometheus/queries/base_query.rb index c60828165bd..29cab6e9c15 100644 --- a/lib/gitlab/prometheus/queries/base_query.rb +++ b/lib/gitlab/prometheus/queries/base_query.rb @@ -20,6 +20,10 @@ module Gitlab def query(*args) raise NotImplementedError end + + def self.transform_reactive_result(result) + result + end end end end diff --git a/lib/gitlab/prometheus/queries/deployment_query.rb b/lib/gitlab/prometheus/queries/deployment_query.rb index 6e6da593178..c2626581897 100644 --- a/lib/gitlab/prometheus/queries/deployment_query.rb +++ b/lib/gitlab/prometheus/queries/deployment_query.rb @@ -2,7 +2,7 @@ module Gitlab module Prometheus module Queries class DeploymentQuery < BaseQuery - def query(environment_id, deployment_id) + def query(deployment_id) Deployment.find_by(id: deployment_id).try do |deployment| environment_slug = deployment.environment.slug @@ -25,6 +25,11 @@ module Gitlab } end end + + def self.transform_reactive_result(result) + result[:metrics] = result.delete :data + result + end end end end diff --git a/lib/gitlab/prometheus/queries/environment_query.rb b/lib/gitlab/prometheus/queries/environment_query.rb index 1d17d3cfd56..b62910c8de6 100644 --- a/lib/gitlab/prometheus/queries/environment_query.rb +++ b/lib/gitlab/prometheus/queries/environment_query.rb @@ -19,6 +19,11 @@ module Gitlab } end end + + def self.transform_reactive_result(result) + result[:metrics] = result.delete :data + result + end end end end diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metric_query.rb index 5710ad47c1a..d920e9a749f 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metric_query.rb @@ -1,7 +1,7 @@ module Gitlab module Prometheus module Queries - class MatchedMetricsQuery < BaseQuery + class MatchedMetricQuery < BaseQuery MAX_QUERY_ITEMS = 40.freeze def query diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index 0c280dc9a3c..aad76e335af 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -3,9 +3,16 @@ module Gitlab module Queries module QueryAdditionalMetrics def query_metrics(project, query_context) + matched_metrics(project).map(&query_group(query_context)) + .select(&method(:group_with_any_metrics)) + end + + protected + + def query_group(query_context) query_processor = method(:process_query).curry[query_context] - groups = matched_metrics(project).map do |group| + lambda do |group| metrics = group.metrics.map do |metric| { title: metric.title, @@ -21,8 +28,6 @@ module Gitlab metrics: metrics.select(&method(:metric_with_any_queries)) } end - - groups.select(&method(:group_with_any_metrics)) end private @@ -72,12 +77,17 @@ module Gitlab end def common_query_context(environment, timeframe_start:, timeframe_end:) - { - timeframe_start: timeframe_start, - timeframe_end: timeframe_end, + base_query_context(timeframe_start, timeframe_end).merge({ ci_environment_slug: environment.slug, kube_namespace: environment.project.deployment_platform&.actual_namespace || '', environment_filter: %{container_name!="POD",environment="#{environment.slug}"} + }) + end + + def base_query_context(timeframe_start, timeframe_end) + { + timeframe_start: timeframe_start, + timeframe_end: timeframe_end } end end diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb index 659021c9ac9..b66253a10e0 100644 --- a/lib/gitlab/prometheus_client.rb +++ b/lib/gitlab/prometheus_client.rb @@ -57,7 +57,11 @@ module Gitlab rescue OpenSSL::SSL::SSLError raise PrometheusClient::Error, "#{rest_client.url} contains invalid SSL data" rescue RestClient::ExceptionWithResponse => ex - handle_exception_response(ex.response) + if ex.response + handle_exception_response(ex.response) + else + raise PrometheusClient::Error, "Network connection error" + end rescue RestClient::Exception raise PrometheusClient::Error, "Network connection error" end diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 73e7921fab7..6c67dfde63a 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -129,10 +129,10 @@ describe Projects::DeploymentsController do end context 'when metrics are enabled' do - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(deployment.project).to receive(:prometheus_service).and_return(prometheus_service) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) end context 'when environment has no metrics' do diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index f17f819feee..b2b245dba90 100644 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -4,21 +4,22 @@ describe Projects::Prometheus::MetricsController do let(:user) { create(:user) } let(:project) { create(:project) } - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:find_or_initialize_service).with('prometheus').and_return(prometheus_service) - project.add_master(user) sign_in(user) end describe 'GET #active_common' do + before do + allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) + end + context 'when prometheus metrics are enabled' do context 'when data is not present' do before do - allow(prometheus_service).to receive(:matched_metrics).and_return({}) + allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return({}) end it 'returns no content response' do @@ -32,7 +33,7 @@ describe Projects::Prometheus::MetricsController do let(:sample_response) { { some_data: 1 } } before do - allow(prometheus_service).to receive(:matched_metrics).and_return(sample_response) + allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return(sample_response) end it 'returns no content response' do @@ -53,6 +54,18 @@ describe Projects::Prometheus::MetricsController do end end + describe '#prometheus_adapter' do + before do + allow(controller).to receive(:project).and_return(project) + end + + it 'calls prometheus adapter service' do + expect_any_instance_of(::Prometheus::AdapterService).to receive(:prometheus_adapter) + + subject.__send__(:prometheus_adapter) + end + end + def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end 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 0697cb2def6..c7169717fc1 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 @@ -7,7 +7,7 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery do include_examples 'additional metrics query' do let(:deployment) { create(:deployment, environment: environment) } - let(:query_params) { [environment.id, deployment.id] } + let(:query_params) { [deployment.id] } it 'queries using specific time' do expect(client).to receive(:query_range).with(anything, diff --git a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb index 84dc31d9732..ffe3ad85baa 100644 --- a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb @@ -31,7 +31,7 @@ describe Gitlab::Prometheus::Queries::DeploymentQuery do expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment-slug"}[30m])) * 100', time: stop_time) - expect(subject.query(environment.id, deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil, - cpu_values: nil, cpu_before: nil, cpu_after: nil) + expect(subject.query(deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil, + cpu_values: nil, cpu_before: nil, cpu_after: nil) end end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb index c95719eff1d..420218a695a 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Prometheus::Queries::MatchedMetricsQuery do +describe Gitlab::Prometheus::Queries::MatchedMetricQuery do include Prometheus::MetricBuilders let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index df8a508e021..2905b58066b 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -22,11 +22,11 @@ describe Clusters::Applications::Prometheus do end end - describe '#proxy_client' do + describe '#prometheus_client' do context 'cluster is nil' do it 'returns nil' do expect(subject.cluster).to be_nil - expect(subject.proxy_client).to be_nil + expect(subject.prometheus_client).to be_nil end end @@ -35,7 +35,7 @@ describe Clusters::Applications::Prometheus do subject { create(:clusters_applications_prometheus, cluster: cluster) } it 'returns nil' do - expect(subject.proxy_client).to be_nil + expect(subject.prometheus_client).to be_nil end end @@ -63,15 +63,15 @@ describe Clusters::Applications::Prometheus do end it 'creates proxy prometheus rest client' do - expect(subject.proxy_client).to be_instance_of(RestClient::Resource) + expect(subject.prometheus_client).to be_instance_of(RestClient::Resource) end it 'creates proper url' do - expect(subject.proxy_client.url).to eq('http://example.com/api/v1/proxy/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80') + expect(subject.prometheus_client.url).to eq('http://example.com/api/v1/proxy/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80') end it 'copies options and headers from kube client to proxy client' do - expect(subject.proxy_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) + expect(subject.prometheus_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) end end end diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb new file mode 100644 index 00000000000..f4b9c57e71a --- /dev/null +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -0,0 +1,121 @@ +require 'spec_helper' + +describe PrometheusAdapter, :use_clean_rails_memory_store_caching do + include PrometheusHelpers + include ReactiveCachingHelpers + + class TestClass + include PrometheusAdapter + end + + let(:project) { create(:prometheus_project) } + let(:service) { project.prometheus_service } + + let(:described_class) { TestClass } + let(:environment_query) { Gitlab::Prometheus::Queries::EnvironmentQuery } + + describe '#query' do + describe 'environment' do + let(:environment) { build_stubbed(:environment, slug: 'env-slug') } + + around do |example| + Timecop.freeze { example.run } + end + + context 'with valid data' do + subject { service.query(:environment, environment) } + + before do + stub_reactive_cache(service, prometheus_data, environment_query, environment.id) + end + + it 'returns reactive data' do + is_expected.to eq(prometheus_metrics_data) + end + end + end + + describe 'matched_metrics' do + let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricQuery } + let(:prometheus_client_wrapper) { double(:prometheus_client_wrapper, label_values: nil) } + + context 'with valid data' do + subject { service.query(:matched_metrics) } + + before do + allow(service).to receive(:prometheus_client_wrapper).and_return(prometheus_client_wrapper) + synchronous_reactive_cache(service) + end + + it 'returns reactive data' do + expect(subject[:success]).to be_truthy + expect(subject[:data]).to eq([]) + end + end + end + + describe 'deployment' do + let(:deployment) { build_stubbed(:deployment) } + let(:deployment_query) { Gitlab::Prometheus::Queries::DeploymentQuery } + + around do |example| + Timecop.freeze { example.run } + end + + context 'with valid data' do + subject { service.query(:deployment, deployment) } + + before do + stub_reactive_cache(service, prometheus_data, deployment_query, deployment.id) + end + + it 'returns reactive data' do + expect(subject).to eq(prometheus_metrics_data) + end + end + end + end + + describe '#calculate_reactive_cache' do + let(:environment) { create(:environment, slug: 'env-slug') } + before do + service.manual_configuration = true + service.active = true + end + + subject do + service.calculate_reactive_cache(environment_query.name, environment.id) + end + + around do |example| + Timecop.freeze { example.run } + end + + context 'when service is inactive' do + before do + service.active = false + end + + it { is_expected.to be_nil } + end + + context 'when Prometheus responds with valid data' do + before do + stub_all_prometheus_requests(environment.slug) + 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| + context "when Prometheus responds with #{status}" do + before do + stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") + end + + it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } + end + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index ba8aa13d5ad..ac30cd27e0c 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -64,6 +64,7 @@ describe Deployment do describe '#metrics' do let(:deployment) { create(:deployment) } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } subject { deployment.metrics } @@ -76,17 +77,16 @@ describe Deployment do { success: true, metrics: {}, - last_update: 42, - deployment_time: 1494408956 + last_update: 42 } end before do - allow(deployment.project).to receive_message_chain(:monitoring_service, :deployment_metrics) - .with(any_args).and_return(simple_metrics) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) end - it { is_expected.to eq(simple_metrics) } + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } end end @@ -109,11 +109,11 @@ describe Deployment do } end - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(project).to receive(:prometheus_service).and_return(prometheus_service) - allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) end it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6f24a039998..ceb570ac777 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Environment do - set(:project) { create(:project) } + let(:project) { create(:project) } subject(:environment) { create(:environment, project: project) } it { is_expected.to belong_to(:project) } @@ -452,8 +452,8 @@ describe Environment do end it 'returns the metrics from the deployment service' do - expect(project.monitoring_service) - .to receive(:environment_metrics).with(environment) + expect(environment.prometheus_adapter) + .to receive(:query).with(:environment, environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -508,12 +508,12 @@ describe Environment do context 'when the environment has additional metrics' do before do - allow(environment).to receive(:has_additional_metrics?).and_return(true) + allow(environment).to receive(:has_metrics?).and_return(true) end it 'returns the additional metrics from the deployment service' do - expect(project.prometheus_service).to receive(:additional_environment_metrics) - .with(environment) + expect(environment.prometheus_adapter).to receive(:query) + .with(:additional_metrics_environment, environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -522,46 +522,13 @@ describe Environment do context 'when the environment does not have metrics' do before do - allow(environment).to receive(:has_additional_metrics?).and_return(false) + allow(environment).to receive(:has_metrics?).and_return(false) end it { is_expected.to be_nil } end end - describe '#has_additional_metrics??' do - subject { environment.has_additional_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 @@ -654,4 +621,12 @@ describe Environment do end end end + + describe '#prometheus_adapter' do + it 'calls prometheus adapter service' do + expect_any_instance_of(Prometheus::AdapterService).to receive(:prometheus_adapter) + + subject.prometheus_adapter + end + end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 6693e5783a5..7afb1b4a8e3 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -6,7 +6,6 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do let(:project) { create(:prometheus_project) } let(:service) { project.prometheus_service } - let(:environment_query) { Gitlab::Prometheus::Queries::EnvironmentQuery } describe "Associations" do it { is_expected.to belong_to :project } @@ -55,197 +54,31 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do end end - describe '#environment_metrics' do - let(:environment) { build_stubbed(:environment, slug: 'env-slug') } - - around do |example| - Timecop.freeze { example.run } - end - - context 'with valid data' do - subject { service.environment_metrics(environment) } - - before do - stub_reactive_cache(service, prometheus_data, environment_query, environment.id) - end - - it 'returns reactive data' do - is_expected.to eq(prometheus_metrics_data) - end - end - end - - describe '#matched_metrics' do - let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricsQuery } - let(:client) { double(:client, label_values: nil) } - - context 'with valid data' do - subject { service.matched_metrics } - - before do - allow(service).to receive(:client).and_return(client) - synchronous_reactive_cache(service) - end - - it 'returns reactive data' do - expect(subject[:success]).to be_truthy - expect(subject[:data]).to eq([]) - end - end - end - - describe '#deployment_metrics' do - let(:deployment) { build_stubbed(:deployment) } - let(:deployment_query) { Gitlab::Prometheus::Queries::DeploymentQuery } - - around do |example| - Timecop.freeze { example.run } - end - - context 'with valid data' do - subject { service.deployment_metrics(deployment) } - let(:fake_deployment_time) { 10 } - - before do - stub_reactive_cache(service, prometheus_data, deployment_query, deployment.environment.id, deployment.id) - end - - it 'returns reactive data' do - expect(deployment).to receive(:created_at).and_return(fake_deployment_time) - - expect(subject).to eq(prometheus_metrics_data.merge(deployment_time: fake_deployment_time)) - end - end - end - - describe '#calculate_reactive_cache' do - let(:environment) { create(:environment, slug: 'env-slug') } - before do - service.manual_configuration = true - service.active = true - end - - subject do - service.calculate_reactive_cache(environment_query.name, environment.id) - end - - around do |example| - Timecop.freeze { example.run } - end - - context 'when service is inactive' do - before do - service.active = false - end - - it { is_expected.to be_nil } - end - - context 'when Prometheus responds with valid data' do - before do - stub_all_prometheus_requests(environment.slug) - 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| - context "when Prometheus responds with #{status}" do - before do - stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") - end - - it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } - end - end - end - - describe '#client' do + describe '#prometheus_client' do context 'manual configuration is enabled' do let(:api_url) { 'http://some_url' } + before do + subject.active = true subject.manual_configuration = true subject.api_url = api_url end - it 'returns simple rest client from api_url' do - expect(subject.client).to be_instance_of(Gitlab::PrometheusClient) - expect(subject.client.rest_client.url).to eq(api_url) + it 'returns rest client from api_url' do + expect(subject.prometheus_client.url).to eq(api_url) end end context 'manual configuration is disabled' do - let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } - let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } - - let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } - let(:proxy_client) { double('proxy_client') } + let(:api_url) { 'http://some_url' } before do - service.manual_configuration = false - end - - context 'with cluster for all environments with prometheus installed' do - let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } - - context 'without environment supplied' do - it 'returns client handling all environments' do - expect(service).to receive(:client_from_cluster).with(cluster_for_all).and_return(proxy_client).twice - - expect(service.client).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client.rest_client).to eq(proxy_client) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_dev).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_all).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end + subject.manual_configuration = false + subject.api_url = api_url end - context 'with cluster for all environments without prometheus installed' do - context 'without environment supplied' do - it 'raises PrometheusClient::Error because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusClient::Error, /couldn't find cluster with Prometheus installed/) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_dev).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'raises PrometheusClient::Error because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusClient::Error, /couldn't find cluster with Prometheus installed/) - end - end + it 'no client provided' do + expect(subject.prometheus_client).to be_nil end end end @@ -284,7 +117,7 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do end end - describe '#synchronize_service_state! before_save callback' do + describe '#synchronize_service_state before_save callback' do context 'no clusters with prometheus are installed' do context 'when service is inactive' do before do diff --git a/spec/services/prometheus/adapter_service_spec.rb b/spec/services/prometheus/adapter_service_spec.rb new file mode 100644 index 00000000000..335fc5844aa --- /dev/null +++ b/spec/services/prometheus/adapter_service_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Prometheus::AdapterService do + let(:project) { create(:project) } + subject { described_class.new(project) } + + describe '#prometheus_adapter' do + let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [project]) } + + context 'prometheus service can execute queries' do + let(:prometheus_service) { double(:prometheus_service, can_query?: true) } + + before do + allow(project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it 'return prometheus service as prometheus adapter' do + expect(subject.prometheus_adapter).to eq(prometheus_service) + end + end + + context "prometheus service can't execute queries" do + let(:prometheus_service) { double(:prometheus_service, can_query?: false) } + + context 'with cluster with prometheus installed' do + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + + it 'returns application handling all environments' do + expect(subject.prometheus_adapter).to eq(prometheus) + end + end + + context 'with cluster without prometheus installed' do + it 'returns nil' do + expect(subject.prometheus_adapter).to be_nil + end + end + end + end +end |