WIP: Allow having all metrics in one file, #143

This was a quick experiment on having all metrics for each process on
the same file, because it seemed like it would be easy. (just do these
things in this commit)

However, even though tests pass, this doesn't actually work. See next commit on why.
If the change were just this, i'd say we should do this.

However, as you'll see in the next commit, it's more involved than that,
and i'm not sure it's worth doing, at least not with this approach...

I'm just pushing this up so it doesn't get lost.

Author: Daniel Magliola

lib/prometheus/client/data_stores/direct_file_store.rb

@@ -29,8 +29,9 @@ class InvalidStoreSettingsError < StandardError; end
         DEFAULT_METRIC_SETTINGS = { aggregation: SUM }
         DEFAULT_GAUGE_SETTINGS = { aggregation: ALL }
 
-        def initialize(dir:)
-          @store_settings = { dir: dir }
+        def initialize(dir:, separate_files_per_metric: true)
+          @store_settings = { dir: dir,
+                              separate_files_per_metric: separate_files_per_metric }
           FileUtils.mkdir_p(dir)
         end
 
@@ -125,6 +126,12 @@ def all_values
                     [k.to_sym, vs.first]
                   end.to_h
 
+                  unless @store_settings[:separate_files_per_metric]
+                    # All metrics are in the same file. Ignore entries for other metrics
+                    next unless label_set[:__metric_name] == metric_name.to_s
+                    label_set.delete(:__metric_name)
+                  end
+
                   stores_data[label_set] << v
                 end
               ensure
@@ -149,6 +156,9 @@ def store_key(labels)
             if @values_aggregation_mode == ALL
               labels[:pid] = process_id
             end
+            unless @store_settings[:separate_files_per_metric]
+              labels[:__metric_name] = metric_name.to_s
+            end
 
             labels.map{|k,v| "#{CGI::escape(k.to_s)}=#{CGI::escape(v.to_s)}"}.join('&')
           end
@@ -164,12 +174,21 @@ def internal_store
 
           # Filename for this metric's PStore (one per process)
           def filemap_filename
-            filename = "metric_#{ metric_name }___#{ process_id }.bin"
+            filename = if @store_settings[:separate_files_per_metric]
+                         "metric_#{ metric_name }___#{ process_id }.bin"
+                       else
+                         "metric___all_metrics___#{ process_id }.bin"
+                       end
             File.join(@store_settings[:dir], filename)
           end
 
           def stores_for_metric
-            Dir.glob(File.join(@store_settings[:dir], "metric_#{ metric_name }___*"))
+            base_filename = if @store_settings[:separate_files_per_metric]
+                              "metric_#{ metric_name }___*"
+                            else
+                              "metric___all_metrics___*"
+                            end
+            Dir.glob(File.join(@store_settings[:dir], base_filename))
           end
 
           def process_id

spec/benchmarks/data_stores.rb

@@ -81,6 +81,12 @@ def all_values; {}; end
   {
     store: Prometheus::Client::DataStores::DirectFileStore.new(dir: TMP_DIR),
     before: -> () { cleanup_dir(TMP_DIR) },
+  },
+  {
+    store: Prometheus::Client::DataStores::DirectFileStore.new(dir: TMP_DIR,
+                                                               separate_files_per_metric: false),
+    before: -> () { cleanup_dir(TMP_DIR) },
+    name: "DirectFileStore Single File"
   }
 ]
 

spec/prometheus/client/data_stores/direct_file_store_spec.rb

@@ -250,4 +250,60 @@
 
     expect(truncate_calls_count).to be >= 3
   end
+
+  context "with all metrics in one single file" do
+    subject { described_class.new(dir: "/tmp/prometheus_test", separate_files_per_metric: false) }
+
+    let(:metric_store1) { subject.for_metric(:metric_name, metric_type: :counter) }
+    let(:metric_store2) { subject.for_metric(:metric_name2, metric_type: :counter) }
+
+    it "stores all metrics into one file" do
+      metric_store1.increment(labels: {a: "x", b: "x"}, by: 1)
+      metric_store1.increment(labels: {a: "x", b: "zzz"}, by: 2)
+      metric_store2.increment(labels: {a: "x", b: "x"}, by: 3)
+
+      expect(Dir.glob('/tmp/prometheus_test/*').length).to eq(1)
+    end
+
+    it "exports values correctly" do
+      metric_store1.increment(labels: {a: "x", b: "x"}, by: 1)
+      metric_store1.increment(labels: {a: "x", b: "zzz"}, by: 2)
+      metric_store2.increment(labels: {a: "x", b: "x"}, by: 3)
+
+      expect(metric_store1.all_values).to eq(
+        {a: "x", b: "x"} => 1.0,
+        {a: "x", b: "zzz"} => 2.0,
+      )
+
+      expect(metric_store2.all_values).to eq(
+        {a: "x", b: "x"} => 3.0,
+      )
+    end
+
+    it "exports values correctly from multiple processes" do
+      metric_store1.increment(labels: {a: "x", b: "x"}, by: 1)
+      metric_store1.increment(labels: {a: "x", b: "zzz"}, by: 2)
+      metric_store2.increment(labels: {a: "x", b: "x"}, by: 3)
+
+      # 2nd process
+      allow(Process).to receive(:pid).and_return(23456)
+      metric_store1.increment(labels: {a: "x", b: "x"}, by: 4)
+      metric_store1.increment(labels: {a: "x", b: "yyy"}, by: 5)
+      metric_store2.increment(labels: {a: "x", b: "x"}, by: 6)
+
+      # Make sure we actually have 2 files to sup together
+      expect(Dir.glob('/tmp/prometheus_test/metric___all_metrics___*').length).to eq(2)
+
+      expect(metric_store1.all_values).to eq(
+        {a: "x", b: "x"} => 5.0,
+        {a: "x", b: "zzz"} => 2.0,
+        {a: "x", b: "yyy"} => 5.0,
+      )
+
+      expect(metric_store2.all_values).to eq(
+        {a: "x", b: "x"} => 9.0,
+      )
+    end
+
+  end
 end