Commit 0051b5fb authored by Pawel Chojnacki's avatar Pawel Chojnacki

Use only real duration to measure method call performance via Prometheus

parent efe4cab9
...@@ -13,4 +13,3 @@ class AddPrometheusInstrumentationToApplicationSettings < ActiveRecord::Migratio ...@@ -13,4 +13,3 @@ class AddPrometheusInstrumentationToApplicationSettings < ActiveRecord::Migratio
remove_column(:application_settings, :prometheus_metrics_method_instrumentation_enabled) remove_column(:application_settings, :prometheus_metrics_method_instrumentation_enabled)
end end
end end
...@@ -6,29 +6,15 @@ module Gitlab ...@@ -6,29 +6,15 @@ module Gitlab
BASE_LABELS = { module: nil, method: nil }.freeze BASE_LABELS = { module: nil, method: nil }.freeze
attr_reader :real_time, :cpu_time, :call_count, :labels attr_reader :real_time, :cpu_time, :call_count, :labels
def self.call_real_duration_histogram def self.call_duration_histogram
return @call_real_duration_histogram if @call_real_duration_histogram return @call_duration_histogram if @call_duration_histogram
MUTEX.synchronize do
@call_real_duration_histogram ||= Gitlab::Metrics.histogram(
:gitlab_method_call_real_duration_seconds,
'Method calls real duration',
Transaction::BASE_LABELS.merge(BASE_LABELS),
[0.1, 0.2, 0.5, 1, 2, 5, 10]
)
end
end
def self.call_cpu_duration_histogram
return @call_cpu_duration_histogram if @call_cpu_duration_histogram
MUTEX.synchronize do MUTEX.synchronize do
@call_duration_histogram ||= Gitlab::Metrics.histogram( @call_duration_histogram ||= Gitlab::Metrics.histogram(
:gitlab_method_call_cpu_duration_seconds, :gitlab_method_call_duration_seconds,
'Method calls cpu duration', 'Method calls real duration',
Transaction::BASE_LABELS.merge(BASE_LABELS), Transaction::BASE_LABELS.merge(BASE_LABELS),
[0.1, 0.2, 0.5, 1, 2, 5, 10] [0.01, 0.05, 0.1, 0.5, 1])
)
end end
end end
...@@ -60,8 +46,7 @@ module Gitlab ...@@ -60,8 +46,7 @@ module Gitlab
@call_count += 1 @call_count += 1
if prometheus_enabled? && above_threshold? if prometheus_enabled? && above_threshold?
self.class.call_real_duration_histogram.observe(@transaction.labels.merge(labels), real_time / 1000.0) self.class.call_duration_histogram.observe(@transaction.labels.merge(labels), real_time / 1000.0)
self.class.call_cpu_duration_histogram.observe(@transaction.labels.merge(labels), cpu_time / 1000.0)
end end
retval retval
......
...@@ -25,11 +25,7 @@ describe Gitlab::Metrics::MethodCall do ...@@ -25,11 +25,7 @@ describe Gitlab::Metrics::MethodCall do
end end
it 'observes the performance of the supplied block' do it 'observes the performance of the supplied block' do
expect(described_class.call_real_duration_histogram) expect(described_class.call_duration_histogram)
.to receive(:observe)
.with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric))
expect(described_class.call_cpu_duration_histogram)
.to receive(:observe) .to receive(:observe)
.with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric)) .with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric))
...@@ -44,10 +40,7 @@ describe Gitlab::Metrics::MethodCall do ...@@ -44,10 +40,7 @@ describe Gitlab::Metrics::MethodCall do
end end
it 'does not observe the performance' do it 'does not observe the performance' do
expect(described_class.call_real_duration_histogram) expect(described_class.call_duration_histogram)
.not_to receive(:observe)
expect(described_class.call_cpu_duration_histogram)
.not_to receive(:observe) .not_to receive(:observe)
method_call.measure { 'foo' } method_call.measure { 'foo' }
...@@ -64,10 +57,7 @@ describe Gitlab::Metrics::MethodCall do ...@@ -64,10 +57,7 @@ describe Gitlab::Metrics::MethodCall do
end end
it 'does not observe the performance' do it 'does not observe the performance' do
expect(described_class.call_real_duration_histogram) expect(described_class.call_duration_histogram)
.not_to receive(:observe)
expect(described_class.call_cpu_duration_histogram)
.not_to receive(:observe) .not_to receive(:observe)
method_call.measure { 'foo' } method_call.measure { 'foo' }
...@@ -92,7 +82,13 @@ describe Gitlab::Metrics::MethodCall do ...@@ -92,7 +82,13 @@ describe Gitlab::Metrics::MethodCall do
end end
describe '#above_threshold?' do describe '#above_threshold?' do
before do
allow(Gitlab::Metrics).to receive(:method_call_threshold).and_return(100)
end
it 'returns false when the total call time is not above the threshold' do it 'returns false when the total call time is not above the threshold' do
expect(method_call).to receive(:real_time).and_return(9)
expect(method_call.above_threshold?).to eq(false) expect(method_call.above_threshold?).to eq(false)
end end
......
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