Merge branch '28075-error-while-fetching-envs' into 'master'

Added configured? method to PrometheusAdapter

Closes #28075 and #36921

See merge request gitlab-org/gitlab!21099

app/models/clusters/applications/prometheus.rb

@@ -84,13 +84,17 @@ module Clusters
         # ensures headers containing auth data are appended to original k8s client options
         options = kube_client.rest_client.options.merge(headers: kube_client.headers)
         Gitlab::PrometheusClient.new(proxy_url, options)
-      rescue Kubeclient::HttpError
+      rescue Kubeclient::HttpError, Errno::ECONNRESET, Errno::ECONNREFUSED
         # If users have mistakenly set parameters or removed the depended clusters,
         # `proxy_url` could raise an exception because gitlab can not communicate with the cluster.
         # Since `PrometheusAdapter#can_query?` is eargely loaded on environement pages in gitlab,
         # we need to silence the exceptions
       end
 
+      def configured?
+        kube_client.present? && available?
+      end
+
       private
 
       def disable_prometheus_integration

app/models/concerns/prometheus_adapter.rb

@@ -16,6 +16,14 @@ module PrometheusAdapter
       raise NotImplementedError
     end
 
+    # This is a light-weight check if a prometheus client is properly configured.
+    def configured?
+      raise NotImplemented
+    end
+
+    # This is a heavy-weight check if a prometheus is properly configured and accesible from GitLab.
+    # This actually sends a request to an external service and often it could take a long time,
+    # Please consider using `configured?` instead if the process is running on unicorn/puma threads.
     def can_query?
       prometheus_client.present?
     end

app/models/environment.rb

@@ -193,11 +193,11 @@ class Environment < ApplicationRecord
   end
 
   def has_metrics?
-    available? && prometheus_adapter&.can_query?
+    available? && prometheus_adapter&.configured?
   end
 
   def metrics
-    prometheus_adapter.query(:environment, self) if has_metrics?
+    prometheus_adapter.query(:environment, self) if has_metrics? && prometheus_adapter.can_query?
   end
 
   def prometheus_status

app/models/project_services/prometheus_service.rb

@@ -95,6 +95,10 @@ class PrometheusService < MonitoringService
     self_monitoring_project? && internal_prometheus_url?
   end
 
+  def configured?
+    should_return_client?
+  end
+
   private
 
   def self_monitoring_project?

changelogs/unreleased/28075-error-while-fetching-envs.yml

+---
+title: Added lightweight check when retrieving Prometheus metrics.
+merge_request: 21099
+author:
+type: performance

spec/models/clusters/applications/prometheus_spec.rb

@@ -53,6 +53,16 @@ describe Clusters::Applications::Prometheus do
   end
 
   describe '#prometheus_client' do
+    shared_examples 'exception caught for prometheus client' do
+      before do
+        allow(kube_client).to receive(:proxy_url).and_raise(exception)
+      end
+
+      it 'returns nil' do
+        expect(subject.prometheus_client).to be_nil
+      end
+    end
+
     context 'cluster is nil' do
       it 'returns nil' do
         expect(subject.cluster).to be_nil
@@ -98,12 +108,18 @@ describe Clusters::Applications::Prometheus do
       end
 
       context 'when cluster is not reachable' do
-        before do
-          allow(kube_client).to receive(:proxy_url).and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil))
+        it_behaves_like 'exception caught for prometheus client' do
+          let(:exception) { Kubeclient::HttpError.new(401, 'Unauthorized', nil) }
+        end
+      end
+
+      context 'when there is a socket error while contacting cluster' do
+        it_behaves_like 'exception caught for prometheus client' do
+          let(:exception) { Errno::ECONNREFUSED }
         end
 
-        it 'returns nil' do
-          expect(subject.prometheus_client).to be_nil
+        it_behaves_like 'exception caught for prometheus client' do
+          let(:exception) { Errno::ECONNRESET }
         end
       end
     end
@@ -289,4 +305,28 @@ describe Clusters::Applications::Prometheus do
       end
     end
   end
+
+  describe '#configured?' do
+    let(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
+
+    subject { prometheus.configured? }
+
+    context 'when a kubenetes client is present' do
+      let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+
+      it { is_expected.to be_truthy }
+
+      context 'when it is not availalble' do
+        let(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) }
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'when a kubenetes client is not present' do
+      let(:cluster) { create(:cluster) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
 end

spec/models/environment_spec.rb

@@ -824,6 +824,14 @@ describe Environment, :use_clean_rails_memory_store_caching do
         context 'and no deployments' do
           it { is_expected.to be_truthy }
         end
+
+        context 'and the prometheus adapter is not configured' do
+          before do
+            allow(environment.prometheus_adapter).to receive(:configured?).and_return(false)
+          end
+
+          it { is_expected.to be_falsy }
+        end
       end
 
       context 'without a monitoring service' do
@@ -858,6 +866,14 @@ describe Environment, :use_clean_rails_memory_store_caching do
 
         is_expected.to eq(:fake_metrics)
       end
+
+      context 'and the prometheus client is not present' do
+        before do
+          allow(environment.prometheus_adapter).to receive(:promethus_client).and_return(nil)
+        end
+
+        it { is_expected.to be_nil }
+      end
     end
 
     context 'when the environment does not have metrics' do