Commit 1434b92e authored by Thong Kuah's avatar Thong Kuah

Merge branch '31475-fix-metrics-for-env-status' into 'master'

Fixing has_metrics  in DeploymentMetrics class

Closes #31475

See merge request gitlab-org/gitlab!21812
parents 1ca9d9fd e62d289f
...@@ -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