Skip to content

Commit

Permalink
Ability to choose style of instrumentation (#228)
Browse files Browse the repository at this point in the history
Allow consumers to choose how methods get patched for instrumentation (prepend vs alias_method). 

Default remains alias_method.
  • Loading branch information
bcharna authored Mar 2, 2022
1 parent 52e01d9 commit 1cf7757
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 35 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@ Ensure you run the exporter in a monitored background process:
$ bundle exec prometheus_exporter
```

#### Choosing the style of method patching

By default, `prometheus_exporter` uses `alias_method` to instrument methods used by SQL and Redis as it is the fastest approach (see [this article](https://samsaffron.com/archive/2017/10/18/fastest-way-to-profile-a-method-in-ruby)). You may desire to add additional instrumentation libraries beyond `prometheus_exporter` to your app. This can become problematic if these other libraries instead use `prepend` to instrument methods. To resolve this, you can tell the middleware to instrument using `prepend` by passing an `instrument` option like so:

```ruby
Rails.application.middleware.unshift PrometheusExporter::Middleware, instrument: :prepend
```

#### Metrics collected by Rails integration middleware

| Type | Name | Description |
Expand Down
85 changes: 61 additions & 24 deletions lib/prometheus_exporter/instrumentation/method_profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,14 @@
module PrometheusExporter::Instrumentation; end

class PrometheusExporter::Instrumentation::MethodProfiler
def self.patch(klass, methods, name)
patch_source_line = __LINE__ + 3
patches = methods.map do |method_name|
<<~RUBY
unless defined?(#{method_name}__mp_unpatched)
alias_method :#{method_name}__mp_unpatched, :#{method_name}
def #{method_name}(*args, &blk)
unless prof = Thread.current[:_method_profiler]
return #{method_name}__mp_unpatched(*args, &blk)
end
begin
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
#{method_name}__mp_unpatched(*args, &blk)
ensure
data = (prof[:#{name}] ||= {duration: 0.0, calls: 0})
data[:duration] += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
data[:calls] += 1
end
end
end
RUBY
end.join("\n")

klass.class_eval patches, __FILE__, patch_source_line
def self.patch(klass, methods, name, instrument:)
if instrument == :alias_method
patch_using_alias_method(klass, methods, name)
elsif instrument == :prepend
patch_using_prepend(klass, methods, name)
else
raise ArgumentError, "instrument must be :alias_method or :prepend"
end
end

def self.transfer
Expand Down Expand Up @@ -55,4 +39,57 @@ def self.stop
end
data
end

private

def self.patch_using_prepend(klass, methods, name)
prepend_instument = Module.new
patch_source_line = __LINE__ + 3
patches = methods.map do |method_name|
<<~RUBY
def #{method_name}(*args, &blk)
unless prof = Thread.current[:_method_profiler]
return super
end
begin
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
super
ensure
data = (prof[:#{name}] ||= {duration: 0.0, calls: 0})
data[:duration] += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
data[:calls] += 1
end
end
RUBY
end.join("\n")

prepend_instument.module_eval patches, __FILE__, patch_source_line
klass.prepend(prepend_instument)
end

def self.patch_using_alias_method(klass, methods, name)
patch_source_line = __LINE__ + 3
patches = methods.map do |method_name|
<<~RUBY
unless defined?(#{method_name}__mp_unpatched)
alias_method :#{method_name}__mp_unpatched, :#{method_name}
def #{method_name}(*args, &blk)
unless prof = Thread.current[:_method_profiler]
return #{method_name}__mp_unpatched(*args, &blk)
end
begin
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
#{method_name}__mp_unpatched(*args, &blk)
ensure
data = (prof[:#{name}] ||= {duration: 0.0, calls: 0})
data[:duration] += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
data[:calls] += 1
end
end
end
RUBY
end.join("\n")

klass.class_eval patches, __FILE__, patch_source_line
end
end
14 changes: 8 additions & 6 deletions lib/prometheus_exporter/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,25 @@
class PrometheusExporter::Middleware
MethodProfiler = PrometheusExporter::Instrumentation::MethodProfiler

def initialize(app, config = { instrument: true, client: nil })
def initialize(app, config = { instrument: :alias_method, client: nil })
@app = app
@client = config[:client] || PrometheusExporter::Client.default

if config[:instrument]
if defined? Redis::Client
MethodProfiler.patch(Redis::Client, [:call, :call_pipeline], :redis)
MethodProfiler.patch(Redis::Client, [
:call, :call_pipeline
], :redis, instrument: config[:instrument])
end
if defined? PG::Connection
MethodProfiler.patch(PG::Connection, [
:exec, :async_exec, :exec_prepared, :send_query_prepared, :query
], :sql)
], :sql, instrument: config[:instrument])
end
if defined? Mysql2::Client
MethodProfiler.patch(Mysql2::Client, [:query], :sql)
MethodProfiler.patch(Mysql2::Statement, [:execute], :sql)
MethodProfiler.patch(Mysql2::Result, [:each], :sql)
MethodProfiler.patch(Mysql2::Client, [:query], :sql, instrument: config[:instrument])
MethodProfiler.patch(Mysql2::Statement, [:execute], :sql, instrument: config[:instrument])
MethodProfiler.patch(Mysql2::Result, [:each], :sql, instrument: config[:instrument])
end
end
end
Expand Down
20 changes: 16 additions & 4 deletions test/instrumentation/method_profiler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,32 @@
require 'prometheus_exporter/instrumentation'

class PrometheusInstrumentationMethodProfilerTest < Minitest::Test
class SomeClass
class SomeClassPatchedUsingAliasMethod
def some_method
"Hello, world"
end
end

class SomeClassPatchedUsingPrepend
def some_method
"Hello, world"
end
end

def setup
PrometheusExporter::Instrumentation::MethodProfiler.patch SomeClass, [:some_method], :test
PrometheusExporter::Instrumentation::MethodProfiler.patch SomeClassPatchedUsingAliasMethod, [:some_method], :test, instrument: :alias_method
PrometheusExporter::Instrumentation::MethodProfiler.patch SomeClassPatchedUsingPrepend, [:some_method], :test, instrument: :prepend
end

def test_source_location
file, line = SomeClass.instance_method(:some_method).source_location
def test_alias_method_source_location
file, line = SomeClassPatchedUsingAliasMethod.instance_method(:some_method).source_location
source = File.read(file).lines[line - 1].strip
assert_equal 'def #{method_name}(*args, &blk)', source
end

def test_prepend_source_location
file, line = SomeClassPatchedUsingPrepend.instance_method(:some_method).source_location
source = File.read(file).lines[line - 1].strip
assert_equal 'def #{method_name}(*args, &blk)', source
end
end
76 changes: 75 additions & 1 deletion test/middleware_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require 'minitest/stub_const'
require_relative 'test_helper'
require 'rack/test'
require 'prometheus_exporter/middleware'
Expand Down Expand Up @@ -32,7 +33,7 @@ def now
end

def configure_middleware(overrides = {})
config = { client: client, instrument: true }.merge(overrides)
config = { client: client, instrument: :alias_method }.merge(overrides)
@app = PrometheusExporter::Middleware.new(inner_app, config)
def @app.request_start
1234567891.123
Expand Down Expand Up @@ -89,4 +90,77 @@ def test_amzn_trace_id_in_wrong_format
assert_invalid_headers_response
end

def test_patch_called_with_prepend_instrument
Object.stub_const(:Redis, Module) do
::Redis.stub_const(:Client) do
mock = Minitest::Mock.new
mock.expect :call, nil, [Redis::Client, Array, :redis, { instrument: :prepend }]
::PrometheusExporter::Instrumentation::MethodProfiler.stub(:patch, mock) do
configure_middleware(instrument: :prepend)
end
mock.verify
end
end

Object.stub_const(:PG, Module) do
::PG.stub_const(:Connection) do
mock = Minitest::Mock.new
mock.expect :call, nil, [PG::Connection, Array, :sql, { instrument: :prepend }]
::PrometheusExporter::Instrumentation::MethodProfiler.stub(:patch, mock) do
configure_middleware(instrument: :prepend)
end
mock.verify
end
end

Object.stub_const(:Mysql2, Module) do
::Mysql2.stub_consts({ Client: nil, Statement: nil, Result: nil }) do
mock = Minitest::Mock.new
mock.expect :call, nil, [Mysql2::Client, Array, :sql, { instrument: :prepend }]
mock.expect :call, nil, [Mysql2::Statement, Array, :sql, { instrument: :prepend }]
mock.expect :call, nil, [Mysql2::Result, Array, :sql, { instrument: :prepend }]
::PrometheusExporter::Instrumentation::MethodProfiler.stub(:patch, mock) do
configure_middleware(instrument: :prepend)
end
mock.verify
end
end
end

def test_patch_called_with_alias_method_instrument
Object.stub_const(:Redis, Module) do
::Redis.stub_const(:Client) do
mock = Minitest::Mock.new
mock.expect :call, nil, [Redis::Client, Array, :redis, { instrument: :alias_method }]
::PrometheusExporter::Instrumentation::MethodProfiler.stub(:patch, mock) do
configure_middleware
end
mock.verify
end
end

Object.stub_const(:PG, Module) do
::PG.stub_const(:Connection) do
mock = Minitest::Mock.new
mock.expect :call, nil, [PG::Connection, Array, :sql, { instrument: :alias_method }]
::PrometheusExporter::Instrumentation::MethodProfiler.stub(:patch, mock) do
configure_middleware
end
mock.verify
end
end

Object.stub_const(:Mysql2, Module) do
::Mysql2.stub_consts({ Client: nil, Statement: nil, Result: nil }) do
mock = Minitest::Mock.new
mock.expect :call, nil, [Mysql2::Client, Array, :sql, { instrument: :alias_method }]
mock.expect :call, nil, [Mysql2::Statement, Array, :sql, { instrument: :alias_method }]
mock.expect :call, nil, [Mysql2::Result, Array, :sql, { instrument: :alias_method }]
::PrometheusExporter::Instrumentation::MethodProfiler.stub(:patch, mock) do
configure_middleware
end
mock.verify
end
end
end
end

0 comments on commit 1cf7757

Please sign in to comment.