Skip to content

Commit

Permalink
Finish refactoring.
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Feb 13, 2010
1 parent e0a4696 commit 0d84308
Show file tree
Hide file tree
Showing 15 changed files with 102 additions and 82 deletions.
4 changes: 2 additions & 2 deletions app/controllers/rails_metrics_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ def scope_store(store)
@by_name = params[:by_name].presence
store = store.by_name(@by_name) if @by_name

@by_instrumenter_id = params[:by_instrumenter_id].presence
store = store.by_instrumenter_id(@by_instrumenter_id) if @by_instrumenter_id
@by_request_id = params[:by_request_id].presence
store = store.by_request_id(@by_request_id) if @by_request_id

@order_by = (valid_order_by? ? params[:order_by] : :latest).to_sym
store = store.send(@order_by)
Expand Down
4 changes: 2 additions & 2 deletions app/views/rails_metrics/_table_row.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% content_tag_for :tr, metric, :class => cycle("odd", "even") do %>
<td class="instrumenter_id">
<%= link_to_set_by_scope metric, :instrumenter_id %>
<td class="request_id">
<%= link_to_set_by_scope metric, :request_id %>
</td>

<td class="name">
Expand Down
2 changes: 1 addition & 1 deletion app/views/rails_metrics/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

<table id="rails_metrics_table" class="index">
<tr>
<th>Instrumenter<br /><%= link_to_clear_by_scope(:instrumenter_id) %></th>
<th>Request<br /><%= link_to_clear_by_scope(:request_id) %></th>
<th>Name<br /><%= link_to_clear_by_scope(:name) %></th>
<th>Duration<br /><%= link_to_order_by_scopes(:slowest, :fastest) %></th>
<th>Payload</th>
Expand Down
4 changes: 2 additions & 2 deletions app/views/rails_metrics/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

<table id="rails_metrics_table" class="show">
<tr class="odd">
<td>Instrumenter</td>
<td><%= link_to_set_by_scope @metric, :instrumenter_id %></td>
<td>Request</td>
<td><%= link_to_set_by_scope @metric, :request_id %></td>
</tr>

<tr class="even">
Expand Down
6 changes: 5 additions & 1 deletion lib/rails_metrics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ def self.async_consumer
next if events.empty?
root_node = nil

if RailsMetrics.store.respond_to?(:verify_active_connections!)
RailsMetrics.store.verify_active_connections!
end

metrics = events.map do |event|
metric = RailsMetrics.store.new
metric.configure(event)
Expand All @@ -104,7 +108,7 @@ def self.async_consumer

# Wait until the async queue is consumed.
def self.wait
sleep(0.05) until async_consumer.finished?
sleep(0.1) until async_consumer.finished?
end

# A notification is valid for storing if two conditions are met:
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_metrics/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def call(env)
@app.call(env)
else
RailsMetrics.listen do
response = notifications.instrument "rack.middlewares",
response = notifications.instrument "rack.request",
:path => env["PATH_INFO"], :method => env["REQUEST_METHOD"],
:instrumenter_id => notifications.instrumenter.id do
@app.call(env)
Expand Down
10 changes: 8 additions & 2 deletions lib/rails_metrics/orm/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ module ActiveRecord
serialize :payload

# Select scopes
scope :by_name, lambda { |name| where(:name => name) }
scope :by_instrumenter_id, lambda { |instrumenter_id| where(:instrumenter_id => instrumenter_id) }
scope :by_name, lambda { |name| where(:name => name) }
scope :by_request_id, lambda { |request_id| where(:request_id => request_id) }

# Order scopes
# We need to add the id in the earliest and latest scope since the database
Expand All @@ -54,6 +54,12 @@ def connections_ids
self.connection_pool.connections.map(&:object_id)
end
end

protected

def save_metric!
save!
end
end
end
end
19 changes: 15 additions & 4 deletions lib/rails_metrics/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,24 @@ def child_of?(node)
node.parent_of?(self)
end

def save_metrics!(parent_id=nil)
self.instrumenter_id = parent_id
save!
def save_metrics!(request_id=nil, parent_id=nil)
self.request_id, self.parent_id = request_id, parent_id
save_metric!

children.each do |child|
child.save_metrics!(self.id)
child.save_metrics!(request_id || self.id, self.id)
end

unless self.request_id
self.request_id ||= self.id
save_metric!
end
end

protected

def save_metric!
raise NotImplementedError
end
end
end
3 changes: 2 additions & 1 deletion test/dummy/db/migrate/20100106152343_create_metrics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ class CreateMetrics < ActiveRecord::Migration
def self.up
create_table :metrics do |t|
t.string :name
t.integer :request_id
t.integer :parent_id
t.integer :duration
t.integer :instrumenter_id
t.text :payload
t.datetime :started_at
t.datetime :created_at
Expand Down
25 changes: 15 additions & 10 deletions test/integration/instrumentation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ class InstrumentationTest < ActionController::IntegrationTest

test "rails metrics request is added to notifications" do
get "/users"
wait!
wait

request = Metric.first

assert_equal "rack.middlewares", request.name
assert_equal "rack.request", request.name
assert (request.duration >= 0)
assert_kind_of Time, request.started_at
assert_equal Hash[:path => "/users", :method => "GET",
Expand All @@ -20,7 +20,7 @@ class InstrumentationTest < ActionController::IntegrationTest

test "processed actions are added to RailsMetrics" do
get "/users"
wait!
wait

assert_equal 4, Metric.count
request, action, sql, template = Metric.all
Expand Down Expand Up @@ -49,27 +49,32 @@ class InstrumentationTest < ActionController::IntegrationTest

test "instrumentations are saved nested in the database" do
get "/users"
wait!
wait

assert_equal 4, Metric.count
request, action, sql, template = Metric.all

assert_nil request.instrumenter_id
assert_equal action.instrumenter_id, request.id
assert_equal sql.instrumenter_id, action.id
assert_equal template.instrumenter_id, action.id
assert_nil request.parent_id
assert_equal action.parent_id, request.id
assert_equal sql.parent_id, action.id
assert_equal template.parent_id, action.id

assert_equal request.id, request.request_id
assert_equal request.id, action.request_id
assert_equal request.id, sql.request_id
assert_equal request.id, template.request_id
end

test "does not create metrics when accessing /rails_metrics" do
assert_no_difference "Metric.count" do
get "/rails_metrics"
wait!
wait
end
end

test "fragment cache are added to RailsMetrics" do
get "/users/new"
wait!
wait

assert_equal 6, Metric.count
request, action, template, partial, exist, write = Metric.all
Expand Down
11 changes: 3 additions & 8 deletions test/integration/navigation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
class NagivationTest < ActionController::IntegrationTest
setup do
Metric.delete_all
get "/users"
wait
end

test "can navigate notifications" do
get "/users" # set up metrics
get "/rails_metrics"

assert_contain "action_view.render_template"
Expand All @@ -31,8 +32,6 @@ class NagivationTest < ActionController::IntegrationTest
end

test "can navigate with pagination" do
get "/users" # set up metrics

get "/rails_metrics"
assert_contain "Showing 1 - 4 of 4 metrics"

Expand All @@ -55,7 +54,6 @@ class NagivationTest < ActionController::IntegrationTest
end

test "can nagivate with by scopes" do
get "/users" # set up metrics
get "/rails_metrics"

click_link "active_record.sql"
Expand All @@ -66,8 +64,6 @@ class NagivationTest < ActionController::IntegrationTest
end

test "can nagivate with order by scopes" do
get "/users" # set up metrics

get "/rails_metrics"
click_link "Order by latest"
assert_contain "ordered by latest"
Expand All @@ -80,7 +76,7 @@ class NagivationTest < ActionController::IntegrationTest
assert_contain "ordered by earliest"

click_link "Show"
assert_contain "rack.middlewares"
assert_contain "rack.request"

get "/rails_metrics"
click_link "Order by fastest"
Expand All @@ -91,7 +87,6 @@ class NagivationTest < ActionController::IntegrationTest
end

test "can destroy all notifications in a given scope" do
get "/users" # set up metrics
get "/rails_metrics"

click_link "active_record.sql"
Expand Down
55 changes: 27 additions & 28 deletions test/rails_metrics_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,34 @@
class RailsMetricsTest < ActiveSupport::TestCase
setup do
RailsMetrics.set_store { MockStore }
RailsMetrics.request_root_node = RailsMetrics::RootNode.new
MockStore.instances.clear
end

# test "send instrumentation event to the specified store" do
# instrument "rails_metrics.something"
# wait
#
# assert_equal "rails_metrics.something", MockStore.instances.last.args[0]
# end
#
# test "does not send an event to the store if it matches an ignored pattern" do
# RailsMetrics.ignore_patterns << /rails_metrics/
#
# begin
# instrument "rails_metrics.something"
# wait
# assert MockStore.instances.empty?
# ensure
# RailsMetrics.ignore_patterns.pop
# end
# end
#
# test "does not send an event to the store if it was generated inside it" do
# instrument "rails_metrics.kicker"
# wait
#
# assert_equal 1, MockStore.instances.size
# assert_equal :kicked!, MockStore.instances.last.args.last
# assert_equal "rails_metrics.kicker", MockStore.instances.last.args[0]
# end
test "send instrumentation event to the specified store" do
instrument "rails_metrics.something"
wait

assert_equal "rails_metrics.something", MockStore.instances.last.args[0]
end

test "does not send an event to the store if it matches an ignored pattern" do
RailsMetrics.ignore_patterns << /rails_metrics/

begin
instrument "rails_metrics.something"
wait
assert MockStore.instances.empty?
ensure
RailsMetrics.ignore_patterns.pop
end
end

test "does not send an event to the store if it was generated inside it" do
instrument "rails_metrics.kicker"
wait

assert_equal 1, MockStore.instances.size
assert_equal "rails_metrics.kicker", MockStore.instances.last.args[0]
assert MockStore.instances.last.kicked?
end
end
4 changes: 0 additions & 4 deletions test/store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,4 @@ def store!(args=sample_args)
test "sets the payload" do
assert_equal Hash[:some => :info], store!.payload
end

test "does not set the instrumenter id from args" do
assert_nil store!.instrumenter_id
end
end
15 changes: 3 additions & 12 deletions test/support/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,9 @@ def wait
RailsMetrics.wait
end

# Sometimes we need to wait until RailsMetrics push reaches the Queue.
def wait!
sleep(0.05)
wait
end

def instrument(*args, &block)
RailsMetrics.request_root_node = root_node = RailsMetrics::RootNode.new
result = ActiveSupport::Notifications.instrument(*args, &block)
RailsMetrics.async_consumer.push root_node
result
ensure
RailsMetrics.request_root_node = nil
RailsMetrics.listen do
ActiveSupport::Notifications.instrument(*args, &block)
end
end
end
20 changes: 16 additions & 4 deletions test/support/mock_store.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class ActiveSupport::TestCase
class MockStore
attr_accessor :args
include RailsMetrics::Store

attr_accessor :args, :id, :parent_id, :request_id

def self.instances
@instances ||= []
Expand All @@ -10,11 +12,21 @@ def initialize
self.class.instances << self
end

def store!(args)
def configure(args)
@args = args
end

def kicked?
@kicked || false
end

protected

def save_metric!
self.id = (rand * 1000).to_i

if args[0] == "rails_metrics.kicker"
args << :kicked!
if @args[0] == "rails_metrics.kicker"
@kicked = true
ActiveSupport::Notifications.instrument("rails_metrics.inside_store")
end
end
Expand Down

0 comments on commit 0d84308

Please sign in to comment.