Skip to content

Commit

Permalink
Skip constructing Time for transactions (#575)
Browse files Browse the repository at this point in the history
* Skip constructing Time for transactions

The `time` parameter was never used, but each call constructed the current time
to fill out the default value.

Default is changed to nil to preserve the API / parameter count.

* Remove third parameter usage in tests

* Remove the unused parameter from sampler

Co-authored-by: Michael Lang <mlang@newrelic.com>
  • Loading branch information
viraptor and Michael Lang authored Apr 2, 2021
1 parent 3fb7f4a commit 9af55c2
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 3 deletions.
2 changes: 1 addition & 1 deletion lib/new_relic/agent/sql_sampler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def tl_transaction_data # only used for testing
end

# This is called when we are done with the transaction.
def on_finishing_transaction(state, name, time=Time.now)
def on_finishing_transaction(state, name)
return unless enabled?

data = state.sql_sampler_transaction_data
Expand Down
2 changes: 1 addition & 1 deletion test/new_relic/agent/pipe_channel_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def create_sql_sample(sampler)
state = NewRelic::Agent::Tracer.state
sampler.on_start_transaction(state, Time.now)
sampler.notice_sql("SELECT * FROM table", "ActiveRecord/Widgets/find", nil, 100, state)
sampler.on_finishing_transaction(state, 'noodles', Time.now)
sampler.on_finishing_transaction(state, 'noodles')
end
end

Expand Down
2 changes: 1 addition & 1 deletion test/new_relic/agent/sql_sampler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ def test_merge_with_existing_trace

def test_on_finishing_transaction_with_busted_transaction_state_does_not_crash
state = NewRelic::Agent::Tracer.state
@sampler.on_finishing_transaction(state, "whatever", Time.now)
@sampler.on_finishing_transaction(state, "whatever")
end

def test_caps_collection_of_unique_statements
Expand Down
25 changes: 25 additions & 0 deletions test/new_relic/agent/transaction_sampler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,31 @@ def generate_samples(count, opts = {})
end
end

def run_long_sample_trace(n)
@sampler.on_start_transaction(@state, Time.now)
n.times do |i|
@sampler.notice_push_frame(@state)
yield if block_given?
@sampler.notice_pop_frame(@state, "node#{i}")
end
@sampler.on_finishing_transaction(@state, @txn)
end

def run_sample_trace(start = Time.now.to_f, stop = nil, state = @state)
@sampler.on_start_transaction(state, start)
@sampler.notice_push_frame(state)
@sampler.notice_sql("SELECT * FROM sandwiches WHERE bread = 'wheat'", {}, 0, state)
@sampler.notice_push_frame(state)
@sampler.notice_sql("SELECT * FROM sandwiches WHERE bread = 'white'", {}, 0, state)
yield if block_given?
@sampler.notice_pop_frame(state, "ab")
@sampler.notice_push_frame(state)
@sampler.notice_sql("SELECT * FROM sandwiches WHERE bread = 'french'", {}, 0, state)
@sampler.notice_pop_frame(state, "ac")
@sampler.notice_pop_frame(state, "a")
@sampler.on_finishing_transaction(state, @txn)
end

def intrinsic_attributes_from_last_sample
sample = NewRelic::Agent.agent.transaction_sampler.harvest!.first
attributes_for(sample, :intrinsic)
Expand Down

0 comments on commit 9af55c2

Please sign in to comment.