Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One file per process instead of per metric/process for DirectFileStore [DOESNT WORK] #161

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions lib/prometheus/client/data_stores/direct_file_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -66,6 +67,11 @@ def validate_metric_settings(metric_settings)
class MetricStore
attr_reader :metric_name, :store_settings

class << self
attr_accessor :shared_store_opened_by_pid
attr_accessor :shared_internal_store
end

def initialize(metric_name:, store_settings:, metric_settings:)
@metric_name = metric_name
@store_settings = store_settings
Expand Down Expand Up @@ -125,6 +131,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
Expand All @@ -149,11 +161,22 @@ 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

def internal_store
if @store_settings[:separate_files_per_metric]
individual_metric_internal_store
else
all_metrics_shared_internal_store
end
end

def individual_metric_internal_store
if @store_opened_by_pid != process_id
@store_opened_by_pid = process_id
@internal_store = FileMappedDict.new(filemap_filename)
Expand All @@ -162,14 +185,32 @@ def internal_store
end
end

def all_metrics_shared_internal_store
if self.class.shared_store_opened_by_pid != process_id
self.class.shared_store_opened_by_pid = process_id
self.class.shared_internal_store = FileMappedDict.new(filemap_filename)
else
self.class.shared_internal_store
end
end

# 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
Expand Down
6 changes: 6 additions & 0 deletions spec/benchmarks/data_stores.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]

Expand Down
58 changes: 58 additions & 0 deletions spec/prometheus/client/data_stores/direct_file_store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
# Reset the PStores
before do
Dir.glob('/tmp/prometheus_test/*').each { |file| File.delete(file) }
# This doesn't actually work, btw, but it's what would have to be done.
Prometheus::Client::DataStores::DirectFileStore::MetricStore.shared_store_opened_by_pid = nil
end

it_behaves_like Prometheus::Client::DataStores
Expand Down Expand Up @@ -250,4 +252,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