Define custom metric methods even if port is zero

[slemrmartin]

If metrics_port is zero then custom metric methods needs to be defined too.

Cought in Ingress API with disabled metrics

[slemrmartin]

@lindgrenj6 it seems there are some values kept between two specs. Any idea?

[lindgrenj6]

Hmm yeah we're back to the old "Can't modify frozen array" error that I ran into when initially writing them. Heres the changes I made to get back to that:

diff --git a/spec/lib/insights/api/common/metrics_spec.rb b/spec/lib/insights/api/common/metrics_spec.rb
index 4d47ddf..263bd7e 100644
--- a/spec/lib/insights/api/common/metrics_spec.rb
+++ b/spec/lib/insights/api/common/metrics_spec.rb
@@ -1,3 +1,6 @@
+require 'prometheus_exporter'
+require 'prometheus_exporter/client'
+
 describe Insights::API::Common::Metrics do
   let(:custom_metrics) do
     {
@@ -19,21 +22,26 @@ describe Insights::API::Common::Metrics do
   let(:metrics) { Insights::API::Common::Metrics }
   let(:prometheus) { PrometheusExporter::Client.default }

-  before do
-    # Dummy app doesn't listen here.
-    allow(Rails).to receive_message_chain(:application, :middleware, :unshift)
-  end

   describe ".setup_custom_metrics" do
+    around do |example|
+      PrometheusExporter::Client.default = PrometheusExporter::Client.new
+      metrics.instance_variable_set(:@metrics_port, port)
+      metrics.activate(nil, "dummy_prefix", custom_metrics)
+
+      example.call
+
+      metrics.instance_variables.each{|e|metrics.remove_instance_variable(e)}
+      PrometheusExporter::Client.default = nil
+    end
+
     context "with metrics port set" do
       before do
-        metrics.instance_variable_set(:@metrics_port, 9394)
-        metrics.activate(nil, "dummy_prefix", custom_metrics)
+        # Dummy app doesn't listen here.
+        allow(Rails).to receive_message_chain(:application, :middleware, :unshift)
       end

-      after do
-        metrics.instance_variable_set(:@metrics_port, nil)
-      end
+      let(:port) { 9394 }

       it "creates the singleton methods on the Metrics object" do
         expect(metrics.respond_to?(:message_on_queue)).to be_truthy
@@ -51,14 +59,7 @@ describe Insights::API::Common::Metrics do
     end

     context "with metrics port zero" do
-      before do
-        metrics.instance_variable_set(:@metrics_port, 0)
-        metrics.activate(nil, "dummy_prefix", custom_metrics)
-      end
-
-      after do
-        metrics.instance_variable_set(:@metrics_port, nil)
-      end
+      let(:port) { 0 }

       it "creates the singleton methods on the Metrics object" do
         expect(metrics.respond_to?(:message_on_queue)).to be_truthy

I'll look into it some more later, but here something that gets it along further.

[lindgrenj6]

Worked on this a bit more today @slemrmartin and I can't seem to get it consistent. This is the "most consistent" way I've found, it fails once every 5-10 test runs for some reason. It's annoying!

diff --git a/spec/lib/insights/api/common/metrics_spec.rb b/spec/lib/insights/api/common/metrics_spec.rb
index 4d47ddf..12b6a6b 100644
--- a/spec/lib/insights/api/common/metrics_spec.rb
+++ b/spec/lib/insights/api/common/metrics_spec.rb
@@ -1,14 +1,17 @@
+require 'prometheus_exporter'
+require 'prometheus_exporter/client'
+
 describe Insights::API::Common::Metrics do
   let(:custom_metrics) do
     {
       :custom_metrics => [
         {
-          :name      => "message_on_queue",
+          :name        => "message_on_queue",
           :type        => :counter,
           :description => "total number of messages put on the queue"
         },
         {
-          :name      => "error_processing_payload",
+          :name        => "error_processing_payload",
           :type        => :counter,
           :description => "total number of errors while attempting to messages put on the queue"
         }
@@ -19,21 +22,21 @@ describe Insights::API::Common::Metrics do
   let(:metrics) { Insights::API::Common::Metrics }
   let(:prometheus) { PrometheusExporter::Client.default }

-  before do
-    # Dummy app doesn't listen here.
-    allow(Rails).to receive_message_chain(:application, :middleware, :unshift)
-  end
-
   describe ".setup_custom_metrics" do
-    context "with metrics port set" do
-      before do
-        metrics.instance_variable_set(:@metrics_port, 9394)
-        metrics.activate(nil, "dummy_prefix", custom_metrics)
-      end
+    before do
+      allow(Rails).to receive_message_chain(:application, :middleware, :unshift)
+      metrics.instance_variable_set(:@metrics_port, port)

-      after do
-        metrics.instance_variable_set(:@metrics_port, nil)
-      end
+      metrics.activate(nil, "dummy_prefix", custom_metrics)
+    end
+
+    after do
+      metrics.instance_variables.each { |e| metrics.remove_instance_variable(e) }
+      PrometheusExporter::Client.default = nil
+    end
+
+    context "with metrics port set" do
+      let(:port) { 9394 }

       it "creates the singleton methods on the Metrics object" do
         expect(metrics.respond_to?(:message_on_queue)).to be_truthy
@@ -51,14 +54,7 @@ describe Insights::API::Common::Metrics do
     end

     context "with metrics port zero" do
-      before do
-        metrics.instance_variable_set(:@metrics_port, 0)
-        metrics.activate(nil, "dummy_prefix", custom_metrics)
-      end
-
-      after do
-        metrics.instance_variable_set(:@metrics_port, nil)
-      end
+      let(:port) { 0 }

       it "creates the singleton methods on the Metrics object" do
         expect(metrics.respond_to?(:message_on_queue)).to be_truthy

[dippy-bot]

Checked commits https://github.com/slemrmartin/insights-api-common-rails/compare/b3c078323574f92a3d8c809f75756b20506b17e9~...87a018f962743370111d6c420c3db5841caa2ea5 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.35.0, and yamllint 2 files checked, 0 offenses detected Everything looks fine. :cake:

[slemrmartin]

@lindgrenj6 I tried current specs version 30times in a line without error. Can you try it on your localhost pls, to be sure? It's hard to say what exactly caused random errors, but probably setting default client to different instance than the one in let