Commit e62d289f authored by Etienne Baqué's avatar Etienne Baqué Committed by Thong Kuah

Fixed AdapterService class based on review

Initializing instance with cluster instead of deployment object.
Adjusted rspec file accordingly.
Fixed changelog content.
parent 1ca9d9fd
...@@ -13,18 +13,18 @@ class DeploymentMetrics ...@@ -13,18 +13,18 @@ class DeploymentMetrics
end end
def has_metrics? def has_metrics?
deployment.success? && prometheus_adapter&.can_query? deployment.success? && prometheus_adapter&.configured?
end end
def metrics def metrics
return {} unless has_metrics? return {} unless has_metrics_and_can_query?
metrics = prometheus_adapter.query(:deployment, deployment) metrics = prometheus_adapter.query(:deployment, deployment)
metrics&.merge(deployment_time: deployment.finished_at.to_i) || {} metrics&.merge(deployment_time: deployment.finished_at.to_i) || {}
end end
def additional_metrics def additional_metrics
return {} unless has_metrics? return {} unless has_metrics_and_can_query?
metrics = prometheus_adapter.query(:additional_metrics_deployment, deployment) metrics = prometheus_adapter.query(:additional_metrics_deployment, deployment)
metrics&.merge(deployment_time: deployment.finished_at.to_i) || {} metrics&.merge(deployment_time: deployment.finished_at.to_i) || {}
...@@ -47,4 +47,8 @@ class DeploymentMetrics ...@@ -47,4 +47,8 @@ class DeploymentMetrics
def cluster_prometheus def cluster_prometheus
cluster.application_prometheus if cluster&.application_prometheus_available? cluster.application_prometheus if cluster&.application_prometheus_available?
end end
def has_metrics_and_can_query?
has_metrics? && prometheus_adapter.can_query?
end
end end
...@@ -208,7 +208,7 @@ class Environment < ApplicationRecord ...@@ -208,7 +208,7 @@ class Environment < ApplicationRecord
end end
def metrics def metrics
prometheus_adapter.query(:environment, self) if has_metrics? && prometheus_adapter.can_query? prometheus_adapter.query(:environment, self) if has_metrics_and_can_query?
end end
def prometheus_status def prometheus_status
...@@ -216,7 +216,7 @@ class Environment < ApplicationRecord ...@@ -216,7 +216,7 @@ class Environment < ApplicationRecord
end end
def additional_metrics(*args) def additional_metrics(*args)
return unless has_metrics? return unless has_metrics_and_can_query?
prometheus_adapter.query(:additional_metrics_environment, self, *args.map(&:to_f)) prometheus_adapter.query(:additional_metrics_environment, self, *args.map(&:to_f))
end end
...@@ -285,6 +285,10 @@ class Environment < ApplicationRecord ...@@ -285,6 +285,10 @@ class Environment < ApplicationRecord
private private
def has_metrics_and_can_query?
has_metrics? && prometheus_adapter.can_query?
end
def generate_slug def generate_slug
self.slug = Gitlab::Slug::Environment.new(name).generate self.slug = Gitlab::Slug::Environment.new(name).generate
end end
......
---
title: Prevent MergeRequestsController#ci_environment_status.json from making HTTP requests
merge_request: 21812
author:
type: fixed
...@@ -20,7 +20,7 @@ describe DeploymentMetrics do ...@@ -20,7 +20,7 @@ describe DeploymentMetrics do
end end
context 'with a Prometheus Service' do context 'with a Prometheus Service' do
let(:prometheus_service) { instance_double(PrometheusService, can_query?: true) } let(:prometheus_service) { instance_double(PrometheusService, can_query?: true, configured?: true) }
before do before do
allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service
...@@ -30,7 +30,17 @@ describe DeploymentMetrics do ...@@ -30,7 +30,17 @@ describe DeploymentMetrics do
end end
context 'with a Prometheus Service that cannot query' do context 'with a Prometheus Service that cannot query' do
let(:prometheus_service) { instance_double(PrometheusService, can_query?: false) } let(:prometheus_service) { instance_double(PrometheusService, configured?: true, can_query?: false) }
before do
allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service
end
it { is_expected.to be_falsy }
end
context 'with a Prometheus Service that is not configured' do
let(:prometheus_service) { instance_double(PrometheusService, configured?: false, can_query?: false) }
before do before do
allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service
...@@ -44,7 +54,7 @@ describe DeploymentMetrics do ...@@ -44,7 +54,7 @@ describe DeploymentMetrics do
let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) } let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) }
before do before do
expect(deployment.cluster.application_prometheus).to receive(:can_query?).and_return(true) expect(deployment.cluster.application_prometheus).to receive(:configured?).and_return(true)
end end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
...@@ -54,7 +64,7 @@ describe DeploymentMetrics do ...@@ -54,7 +64,7 @@ describe DeploymentMetrics do
describe '#metrics' do describe '#metrics' do
let(:deployment) { create(:deployment, :success) } let(:deployment) { create(:deployment, :success) }
let(:prometheus_adapter) { instance_double(PrometheusService, can_query?: true) } let(:prometheus_adapter) { instance_double(PrometheusService, can_query?: true, configured?: true) }
let(:deployment_metrics) { described_class.new(deployment.project, deployment) } let(:deployment_metrics) { described_class.new(deployment.project, deployment) }
subject { deployment_metrics.metrics } subject { deployment_metrics.metrics }
...@@ -101,7 +111,7 @@ describe DeploymentMetrics do ...@@ -101,7 +111,7 @@ describe DeploymentMetrics do
} }
end end
let(:prometheus_adapter) { instance_double('prometheus_adapter', can_query?: true) } let(:prometheus_adapter) { instance_double('prometheus_adapter', can_query?: true, configured?: true) }
before do before do
allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter)
......
...@@ -45,7 +45,7 @@ describe EnvironmentStatusEntity do ...@@ -45,7 +45,7 @@ describe EnvironmentStatusEntity do
end end
context 'when deployment has metrics' do context 'when deployment has metrics' do
let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true, configured?: true) }
let(:simple_metrics) do let(:simple_metrics) do
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment