Permalink
Browse files

Use optional hash for metric arguments instead of argument list.

  • Loading branch information...
1 parent 44328fa commit cd33e1b6e8e3bbce074b7c182b19dcd143c9fde0 @wvanbergen wvanbergen committed Feb 7, 2014
Showing with 61 additions and 62 deletions.
  1. +34 −40 lib/statsd/instrument.rb
  2. +13 −13 test/statsd_instrumentation_test.rb
  3. +14 −9 test/statsd_test.rb
View
@@ -33,15 +33,15 @@ def self.generate_metric_name(metric_name, callee, *args)
metric_name.respond_to?(:call) ? metric_name.call(callee, args).gsub('::', '.') : metric_name.gsub('::', '.')
end
- def statsd_measure(method, name, sample_rate = StatsD.default_sample_rate)
+ def statsd_measure(method, name, *metric_options)
add_to_method(method, name, :measure) do |old_method, new_method, metric_name, *args|
define_method(new_method) do |*args, &block|
- StatsD.measure(StatsD::Instrument.generate_metric_name(metric_name, self, *args), nil, sample_rate) { send(old_method, *args, &block) }
+ StatsD.measure(StatsD::Instrument.generate_metric_name(metric_name, self, *args), nil, *metric_options) { send(old_method, *args, &block) }
end
end
end
- def statsd_count_success(method, name, sample_rate = StatsD.default_sample_rate)
+ def statsd_count_success(method, name, *metric_options)
add_to_method(method, name, :count_success) do |old_method, new_method, metric_name|
define_method(new_method) do |*args, &block|
begin
@@ -54,13 +54,13 @@ def statsd_count_success(method, name, sample_rate = StatsD.default_sample_rate)
result
ensure
suffix = truthiness == false ? 'failure' : 'success'
- StatsD.increment("#{StatsD::Instrument.generate_metric_name(metric_name, self, *args)}.#{suffix}", 1, sample_rate)
+ StatsD.increment("#{StatsD::Instrument.generate_metric_name(metric_name, self, *args)}.#{suffix}", 1, *metric_options)
end
end
end
end
- def statsd_count_if(method, name, sample_rate = StatsD.default_sample_rate)
+ def statsd_count_if(method, name, *metric_options)
add_to_method(method, name, :count_if) do |old_method, new_method, metric_name|
define_method(new_method) do |*args, &block|
begin
@@ -72,16 +72,16 @@ def statsd_count_if(method, name, sample_rate = StatsD.default_sample_rate)
truthiness = (yield(result) rescue false) if block_given?
result
ensure
- StatsD.increment(StatsD::Instrument.generate_metric_name(metric_name, self, *args), sample_rate) if truthiness
+ StatsD.increment(StatsD::Instrument.generate_metric_name(metric_name, self, *args), *metric_options) if truthiness
end
end
end
end
- def statsd_count(method, name, sample_rate = StatsD.default_sample_rate)
+ def statsd_count(method, name, *metric_options)
add_to_method(method, name, :count) do |old_method, new_method, metric_name|
define_method(new_method) do |*args, &block|
- StatsD.increment(StatsD::Instrument.generate_metric_name(metric_name, self, *args), 1, sample_rate)
+ StatsD.increment(StatsD::Instrument.generate_metric_name(metric_name, self, *args), 1, *metric_options)
send(old_method, *args, &block)
end
end
@@ -132,57 +132,50 @@ def remove_from_method(method, name, action)
end
# glork:320|ms
- def self.measure(key, milli = nil, sample_rate = default_sample_rate, tags = nil)
+ def self.measure(key, milli = nil, *metric_options)
result = nil
ms = milli || 1000 * Benchmark.realtime do
result = yield
end
- collect(key, ms, :ms, sample_rate, tags)
+ collect(:ms, key, ms, hash_argument(metric_options))
result
end
# gorets:1|c
- def self.increment(key, *args)
- collect(key, *unpack_arguments(:incr, nil, args))
+ def self.increment(key, value = 1, *metric_options)
+ collect(:incr, key, value, hash_argument(metric_options))
end
# gaugor:333|g
# guagor:1234|kv|@1339864935 (statsite)
- def self.gauge(key, value, *args)
- collect(key, *unpack_arguments(:g, value, args))
+ def self.gauge(key, value, *metric_options)
+ collect(:g, key, value, hash_argument(metric_options))
end
# histogram:123.45|h
- def self.histogram(key, value, *args)
- collect(key, *unpack_arguments(:h, value, args))
+ def self.histogram(key, value, *metric_options)
+ collect(:h, key, value, hash_argument(metric_options))
end
# uniques:765|s
- def self.set(key, value, *args)
- collect(key, *unpack_arguments(:s, value, args))
+ def self.set(key, value, *metric_options)
+ collect(:s, key, value, hash_argument(metric_options))
end
private
- def self.unpack_arguments(operation, value, optional_arguments)
- sample_rate, tags, delta = nil
+ def self.hash_argument(args)
+ return {} if args.length == 0
+ return args.first if args.length == 1 && args.first.is_a?(Hash)
- if optional_arguments.is_a?(Hash)
- sample_rate = optional_arguments[:sample_rate] || default_sample_rate
- tags = optional_arguments[:tags] || nil
- delta = optional_arguments[:delta] || 1
- else
- if operation == :incr && optional_arguments.empty?
- # sometimes we wanna pass in delta (for incr) other times we want to pass in value instead
- end
- delta = optional_arguments.shift || 1
- sample_rate = optional_arguments.first || default_sample_rate
- tags = optional_arguments[:tags] || nil
- end
-
- # yea this generic approach below won't do it like that
- [delta || value, operation, sample_rate, tags]
+ order = [:sample_rate, :tags]
+ hash = {}
+ args.each_with_index do |value, index|
+ hash[order[index]] = value
+ end
+
+ return hash
end
def self.invalidate_socket
@@ -197,12 +190,13 @@ def self.socket
@socket
end
- def self.collect(k, v, op, sample_rate = default_sample_rate, tags = nil)
+ def self.collect(type, k, v, options = {})
return unless enabled
+ sample_rate = options[:sample_rate] || StatsD.default_sample_rate
return if sample_rate < 1 && rand > sample_rate
- command = generate_packet(k, v, op, sample_rate, tags)
- write_packet(command)
+ packet = generate_packet(type, k, v, sample_rate, options[:tags])
+ write_packet(packet)
end
def self.write_packet(command)
@@ -224,9 +218,9 @@ def self.clean_tags(tags)
end
end
- def self.generate_packet(k, v, op, sample_rate = default_sample_rate, tags = nil)
+ def self.generate_packet(type, k, v, sample_rate = default_sample_rate, tags = nil)
command = "#{self.prefix + '.' if self.prefix}#{k}:#{v}"
- case op
+ case type
when :incr
command << '|c'
when :ms
@@ -51,7 +51,7 @@ class StatsDInstrumentationTest < Test::Unit::TestCase
def test_statsd_count_if
ActiveMerchant::Gateway.statsd_count_if :ssl_post, 'ActiveMerchant.Gateway.if'
- StatsD.expects(:increment).with('ActiveMerchant.Gateway.if', 1).once
+ StatsD.expects(:increment).with('ActiveMerchant.Gateway.if').once
ActiveMerchant::Gateway.new.purchase(true)
ActiveMerchant::Gateway.new.purchase(false)
@@ -63,7 +63,7 @@ def test_statsd_count_if_with_method_receiving_block
result == 'true'
end
- StatsD.expects(:collect).with('ActiveMerchant.Base.post_with_block', 1, :incr, 1.0, nil).once
+ StatsD.expects(:collect).with(:incr, 'ActiveMerchant.Base.post_with_block', 1, {}).once
assert_equal 'true', ActiveMerchant::Base.new.post_with_block { 'true' }
assert_equal 'false', ActiveMerchant::Base.new.post_with_block { 'false' }
@@ -75,7 +75,7 @@ def test_statsd_count_if_with_block
result[:success]
end
- StatsD.expects(:increment).with('ActiveMerchant.Gateway.block', 1).once
+ StatsD.expects(:increment).with('ActiveMerchant.Gateway.block').once
ActiveMerchant::UniqueGateway.new.purchase(true)
ActiveMerchant::UniqueGateway.new.purchase(false)
@@ -99,8 +99,8 @@ def test_statsd_count_success_with_method_receiving_block
result == 'successful'
end
- StatsD.expects(:collect).with('ActiveMerchant.Base.post_with_block.success', 1, :incr, 1.0, nil).once
- StatsD.expects(:collect).with('ActiveMerchant.Base.post_with_block.failure', 1, :incr, 1.0, nil).once
+ StatsD.expects(:collect).with(:incr, 'ActiveMerchant.Base.post_with_block.success', 1, {}).once
+ StatsD.expects(:collect).with(:incr, 'ActiveMerchant.Base.post_with_block.failure', 1, {}).once
assert_equal 'successful', ActiveMerchant::Base.new.post_with_block { 'successful' }
assert_equal 'not so successful', ActiveMerchant::Base.new.post_with_block { 'not so successful' }
@@ -113,10 +113,10 @@ def test_statsd_count_success_with_block
result[:success]
end
- StatsD.expects(:increment).with('ActiveMerchant.Gateway.success', 1, StatsD.default_sample_rate)
+ StatsD.expects(:increment).with('ActiveMerchant.Gateway.success', 1)
ActiveMerchant::UniqueGateway.new.purchase(true)
- StatsD.expects(:increment).with('ActiveMerchant.Gateway.failure', 1, StatsD.default_sample_rate)
+ StatsD.expects(:increment).with('ActiveMerchant.Gateway.failure', 1)
ActiveMerchant::UniqueGateway.new.purchase(false)
ActiveMerchant::UniqueGateway.statsd_remove_count_success :ssl_post, 'ActiveMerchant.Gateway'
@@ -125,7 +125,7 @@ def test_statsd_count_success_with_block
def test_statsd_count
ActiveMerchant::Gateway.statsd_count :ssl_post, 'ActiveMerchant.Gateway.ssl_post'
- StatsD.expects(:increment).with('ActiveMerchant.Gateway.ssl_post', 1, StatsD.default_sample_rate)
+ StatsD.expects(:increment).with('ActiveMerchant.Gateway.ssl_post', 1)
ActiveMerchant::Gateway.new.purchase(true)
ActiveMerchant::Gateway.statsd_remove_count :ssl_post, 'ActiveMerchant.Gateway.ssl_post'
@@ -135,7 +135,7 @@ def test_statsd_count_with_name_as_lambda
metric_namer = lambda { |object, args| object.class.to_s.downcase + ".insert." + args.first.to_s }
ActiveMerchant::Gateway.statsd_count(:ssl_post, metric_namer)
- StatsD.expects(:increment).with('gatewaysubclass.insert.true', 1, StatsD.default_sample_rate)
+ StatsD.expects(:increment).with('gatewaysubclass.insert.true', 1)
GatewaySubClass.new.purchase(true)
ActiveMerchant::Gateway.statsd_remove_count(:ssl_post, metric_namer)
@@ -144,7 +144,7 @@ def test_statsd_count_with_name_as_lambda
def test_statsd_count_with_method_receiving_block
ActiveMerchant::Base.statsd_count :post_with_block, 'ActiveMerchant.Base.post_with_block'
- StatsD.expects(:collect).with('ActiveMerchant.Base.post_with_block', 1, :incr, 1.0, nil)
+ StatsD.expects(:collect).with(:incr, 'ActiveMerchant.Base.post_with_block', 1, {})
assert_equal 'block called', ActiveMerchant::Base.new.post_with_block { 'block called' }
ActiveMerchant::Base.statsd_remove_count :post_with_block, 'ActiveMerchant.Base.post_with_block'
@@ -153,7 +153,7 @@ def test_statsd_count_with_method_receiving_block
def test_statsd_measure_with_nested_modules
ActiveMerchant::UniqueGateway.statsd_measure :ssl_post, 'ActiveMerchant::Gateway.ssl_post'
- StatsD.expects(:measure).with('ActiveMerchant.Gateway.ssl_post', nil, StatsD.default_sample_rate)
+ StatsD.expects(:measure).with('ActiveMerchant.Gateway.ssl_post', nil)
ActiveMerchant::UniqueGateway.new.purchase(true)
ActiveMerchant::UniqueGateway.statsd_remove_measure :ssl_post, 'ActiveMerchant::Gateway.ssl_post'
@@ -171,7 +171,7 @@ def test_statsd_measure
def test_statsd_measure_with_method_receiving_block
ActiveMerchant::Base.statsd_measure :post_with_block, 'ActiveMerchant.Base.post_with_block'
- StatsD.expects(:collect).with('ActiveMerchant.Base.post_with_block', is_a(Float), :ms, 1.0, nil)
+ StatsD.expects(:collect).with(:ms, 'ActiveMerchant.Base.post_with_block', is_a(Float), {})
assert_equal 'block called', ActiveMerchant::Base.new.post_with_block { 'block called' }
ActiveMerchant::Base.statsd_remove_measure :post_with_block, 'ActiveMerchant.Base.post_with_block'
@@ -181,7 +181,7 @@ def test_instrumenting_class_method
ActiveMerchant::Gateway.singleton_class.extend StatsD::Instrument
ActiveMerchant::Gateway.singleton_class.statsd_count :sync, 'ActiveMerchant.Gateway.sync'
- StatsD.expects(:increment).with('ActiveMerchant.Gateway.sync', 1, StatsD.default_sample_rate)
+ StatsD.expects(:increment).with('ActiveMerchant.Gateway.sync', 1)
ActiveMerchant::Gateway.sync
ActiveMerchant::Gateway.singleton_class.statsd_remove_count :sync, 'ActiveMerchant.Gateway.sync'
View
@@ -16,42 +16,47 @@ def setup
end
def test_statsd_measure_with_explicit_value
- StatsD.expects(:collect).with('values.foobar', 42, :ms, 1.0, nil)
+ StatsD.expects(:collect).with(:ms, 'values.foobar', 42, {})
StatsD.measure('values.foobar', 42)
end
def test_statsd_measure_with_explicit_value_and_sample_rate
- StatsD.expects(:collect).with('values.foobar', 42, :ms, 0.1, nil)
- StatsD.measure('values.foobar', 42, 0.1)
+ StatsD.expects(:collect).with(:ms, 'values.foobar', 42, :sample_rate => 0.1)
+ StatsD.measure('values.foobar', 42, :sample_rate => 0.1)
end
def test_statsd_measure_with_benchmarked_value
Benchmark.stubs(:realtime).returns(1.12)
- StatsD.expects(:collect).with('values.foobar', 1120.0, :ms, 1.0, nil)
+ StatsD.expects(:collect).with(:ms, 'values.foobar', 1120.0, {})
StatsD.measure('values.foobar', nil) do
#noop
end
end
def test_statsd_increment_with_hash_argument
- StatsD.expects(:collect).with('values.foobar', 1, :incr, 1.0, ['test'])
+ StatsD.expects(:collect).with(:g, 'values.foobar', 12, :tags => ['test'])
StatsD.gauge('values.foobar', 12, :tags => ['test'])
end
+ def test_statsd_increment_with_multiple_arguments
+ StatsD.expects(:collect).with(:g, 'values.foobar', 12, :sample_rate => nil, :tags => ['test'])
+ StatsD.gauge('values.foobar', 12, nil, ['test'])
+ end
+
def test_statsd_gauge
- StatsD.expects(:collect).with('values.foobar', 12, :g, 1.0, nil)
+ StatsD.expects(:collect).with(:g, 'values.foobar', 12, {})
StatsD.gauge('values.foobar', 12)
end
def test_statsd_set
- StatsD.expects(:collect).with('values.foobar', 12, :s, 1.0, nil)
+ StatsD.expects(:collect).with(:s, 'values.foobar', 12, {})
StatsD.set('values.foobar', 12)
end
def test_statsd_histogram_on_datadog
StatsD.stubs(:implementation).returns(:datadog)
- StatsD.expects(:collect).with('values.hg', 12.33, :h, 0.2, ['tag_123', 'key-name:value123'])
- StatsD.histogram('values.hg', 12.33, 0.2, ['tag_123', 'key-name:value123'])
+ StatsD.expects(:collect).with(:h, 'values.hg', 12.33, :sample_rate => 0.2, :tags => ['tag_123', 'key-name:value123'])
+ StatsD.histogram('values.hg', 12.33, :sample_rate => 0.2, :tags => ['tag_123', 'key-name:value123'])
end
def test_collect_respects_enabled

0 comments on commit cd33e1b

Please sign in to comment.