(POOLER-160) Revise connection metrics

The redis pooler connection metric used "metric_prefix" which is
misleading, so split this into connpool_type and connpool_provider.
Also remove some earlier jruby compatibility code to reduce
rebase conflicts when this is rebased on top of Matt's changes.
This commit is contained in:
John O'Connor 2020-05-22 19:21:15 +01:00
parent 5c38ba240a
commit 72564de4b4
7 changed files with 33 additions and 24 deletions

View file

@ -169,7 +169,8 @@ module Vmpooler
def self.redis_connection_pool(host, port, password, size, timeout, metrics) def self.redis_connection_pool(host, port, password, size, timeout, metrics)
Vmpooler::PoolManager::GenericConnectionPool.new( Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics, metrics: metrics,
metric_prefix: 'redis_connection_pool', connpool_type: 'redis_connection_pool',
connpool_provider: 'manager',
size: size, size: size,
timeout: timeout timeout: timeout
) do ) do

View file

@ -11,8 +11,10 @@ module Vmpooler
def initialize(options = {}, &block) def initialize(options = {}, &block)
super(options, &block) super(options, &block)
@metrics = options[:metrics] @metrics = options[:metrics]
@metric_prefix = options[:metric_prefix] @connpool_type = options[:connpool_type]
@metric_prefix = 'connectionpool' if @metric_prefix.nil? || @metric_prefix == '' @connpool_type = 'connectionpool' if @connpool_type.nil? || @connpool_type == ''
@connpool_provider = options[:connpool_provider]
@connpool_provider = 'unknown' if @connpool_provider.nil? || @connpool_provider == ''
end end
def with_metrics(options = {}) def with_metrics(options = {})
@ -20,15 +22,15 @@ module Vmpooler
start = Time.now start = Time.now
conn = checkout(options) conn = checkout(options)
timespan_ms = ((Time.now - start) * 1000).to_i timespan_ms = ((Time.now - start) * 1000).to_i
@metrics&.gauge(@metric_prefix + '.available', @available.length) @metrics&.gauge("connection_available.#{@connpool_type}.#{@connpool_provider}", @available.length)
@metrics&.timing(@metric_prefix + '.waited', timespan_ms) @metrics&.timing("connection_waited.#{@connpool_type}.#{@connpool_provider}", timespan_ms)
begin begin
Thread.handle_interrupt(Exception => :immediate) do Thread.handle_interrupt(Exception => :immediate) do
yield conn yield conn
end end
ensure ensure
checkin checkin
@metrics&.gauge(@metric_prefix + '.available', @available.length) @metrics&.gauge("connection_available.#{@connpool_type}.#{@connpool_provider}", @available.length)
end end
end end
end end

View file

@ -29,7 +29,8 @@ module Vmpooler
logger.log('d', "[#{name}] ConnPool - Creating a connection pool of size #{connpool_size} with timeout #{connpool_timeout}") logger.log('d', "[#{name}] ConnPool - Creating a connection pool of size #{connpool_size} with timeout #{connpool_timeout}")
@connection_pool = Vmpooler::PoolManager::GenericConnectionPool.new( @connection_pool = Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics, metrics: metrics,
metric_prefix: "#{name}_provider_connection_pool", connpool_type: 'provider_connection_pool',
connpool_provider: name,
size: connpool_size, size: connpool_size,
timeout: connpool_timeout timeout: connpool_timeout
) do ) do

View file

@ -25,7 +25,8 @@ module Vmpooler
logger.log('d', "[#{name}] ConnPool - Creating a connection pool of size #{connpool_size} with timeout #{connpool_timeout}") logger.log('d', "[#{name}] ConnPool - Creating a connection pool of size #{connpool_size} with timeout #{connpool_timeout}")
@connection_pool = Vmpooler::PoolManager::GenericConnectionPool.new( @connection_pool = Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics, metrics: metrics,
metric_prefix: "#{name}_provider_connection_pool", connpool_type: 'provider_connection_pool',
connpool_provider: name,
size: connpool_size, size: connpool_size,
timeout: connpool_timeout timeout: connpool_timeout
) do ) do

View file

@ -2,15 +2,17 @@ require 'spec_helper'
describe 'GenericConnectionPool' do describe 'GenericConnectionPool' do
let(:metrics) { Vmpooler::DummyStatsd.new } let(:metrics) { Vmpooler::DummyStatsd.new }
let(:metric_prefix) { 'prefix' } let(:connpool_type) { 'test_connection_pool' }
let(:default_metric_prefix) { 'connectionpool' } let(:connpool_provider) { 'testprovider' }
let(:default_connpool_type) { 'connectionpool' }
let(:connection_object) { double('connection') } let(:connection_object) { double('connection') }
let(:pool_size) { 1 } let(:pool_size) { 1 }
let(:pool_timeout) { 1 } let(:pool_timeout) { 1 }
subject { Vmpooler::PoolManager::GenericConnectionPool.new( subject { Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics, metrics: metrics,
metric_prefix: metric_prefix, connpool_type: connpool_type,
connpool_provider: connpool_provider,
size: pool_size, size: pool_size,
timeout: pool_timeout timeout: pool_timeout
) { connection_object } ) { connection_object }
@ -65,7 +67,7 @@ describe 'GenericConnectionPool' do
context 'When metrics are configured' do context 'When metrics are configured' do
it 'should emit a gauge metric when the connection is grabbed and released' do it 'should emit a gauge metric when the connection is grabbed and released' do
expect(metrics).to receive(:gauge).with(/\.available/,Integer).exactly(2).times expect(metrics).to receive(:gauge).with(/connection_available/,Integer).exactly(2).times
subject.with_metrics do |conn1| subject.with_metrics do |conn1|
# do nothing # do nothing
@ -73,7 +75,7 @@ describe 'GenericConnectionPool' do
end end
it 'should emit a timing metric when the connection is grabbed' do it 'should emit a timing metric when the connection is grabbed' do
expect(metrics).to receive(:timing).with(/\.waited/,Integer).exactly(1).times expect(metrics).to receive(:timing).with(/connection_waited/,Integer).exactly(1).times
subject.with_metrics do |conn1| subject.with_metrics do |conn1|
# do nothing # do nothing
@ -81,8 +83,8 @@ describe 'GenericConnectionPool' do
end end
it 'should emit metrics with the specified prefix' do it 'should emit metrics with the specified prefix' do
expect(metrics).to receive(:gauge).with(/#{metric_prefix}\./,Integer).at_least(1).times expect(metrics).to receive(:gauge).with(/#{connpool_type}\./,Integer).at_least(1).times
expect(metrics).to receive(:timing).with(/#{metric_prefix}\./,Integer).at_least(1).times expect(metrics).to receive(:timing).with(/#{connpool_type}\./,Integer).at_least(1).times
subject.with_metrics do |conn1| subject.with_metrics do |conn1|
# do nothing # do nothing
@ -90,11 +92,11 @@ describe 'GenericConnectionPool' do
end end
context 'Metrix prefix is missing' do context 'Metrix prefix is missing' do
let(:metric_prefix) { nil } let(:connpool_type) { nil }
it 'should emit metrics with default prefix' do it 'should emit metrics with default prefix' do
expect(metrics).to receive(:gauge).with(/#{default_metric_prefix}\./,Integer).at_least(1).times expect(metrics).to receive(:gauge).with(/#{default_connpool_type}\./,Integer).at_least(1).times
expect(metrics).to receive(:timing).with(/#{default_metric_prefix}\./,Integer).at_least(1).times expect(metrics).to receive(:timing).with(/#{default_connpool_type}\./,Integer).at_least(1).times
subject.with_metrics do |conn1| subject.with_metrics do |conn1|
# do nothing # do nothing
@ -103,11 +105,11 @@ describe 'GenericConnectionPool' do
end end
context 'Metrix prefix is empty' do context 'Metrix prefix is empty' do
let(:metric_prefix) { '' } let(:connpool_type) { '' }
it 'should emit metrics with default prefix' do it 'should emit metrics with default prefix' do
expect(metrics).to receive(:gauge).with(/#{default_metric_prefix}\./,Integer).at_least(1).times expect(metrics).to receive(:gauge).with(/#{default_connpool_type}\./,Integer).at_least(1).times
expect(metrics).to receive(:timing).with(/#{default_metric_prefix}\./,Integer).at_least(1).times expect(metrics).to receive(:timing).with(/#{default_connpool_type}\./,Integer).at_least(1).times
subject.with_metrics do |conn1| subject.with_metrics do |conn1|
# do nothing # do nothing

View file

@ -22,7 +22,8 @@ describe 'Pool Manager' do
let(:provider_options) { {} } let(:provider_options) { {} }
let(:redis_connection_pool) { Vmpooler::PoolManager::GenericConnectionPool.new( let(:redis_connection_pool) { Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics, metrics: metrics,
metric_prefix: 'redis_connection_pool', connpool_type: 'redis_connection_pool',
connpool_provider: 'testprovider',
size: 1, size: 1,
timeout: 5 timeout: 5
) { MockRedis.new } ) { MockRedis.new }

View file

@ -79,7 +79,8 @@ EOT
let(:vmname) { 'vm1' } let(:vmname) { 'vm1' }
let(:redis_connection_pool) { Vmpooler::PoolManager::GenericConnectionPool.new( let(:redis_connection_pool) { Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics, metrics: metrics,
metric_prefix: 'redis_connection_pool', connpool_type: 'redis_connection_pool',
connpool_provider: 'testprovider',
size: 1, size: 1,
timeout: 5 timeout: 5
) { MockRedis.new } ) { MockRedis.new }
@ -167,7 +168,7 @@ EOT
end end
it 'should record metrics' do it 'should record metrics' do
expect(metrics).to receive(:timing).with('redis_connection_pool.waited', 0) expect(metrics).to receive(:timing).with('connection_waited.redis_connection_pool.testprovider', 0)
expect(metrics).to receive(:timing).with("destroy.#{pool}", finish) expect(metrics).to receive(:timing).with("destroy.#{pool}", finish)
subject.destroy_vm_and_log(vmname, vm_object, pool, data_ttl) subject.destroy_vm_and_log(vmname, vm_object, pool, data_ttl)