Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize setting parameters for transactions #1158

Merged
merged 1 commit into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changesets/add-block-argument-to-set_params.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
bump: patch
type: add
---

Add a block argument to the `Appsignal.set_params` and `Appsignal::Transaction#set_params` helpers. When `set_params` is called with a block argument, the block is executed when the parameters are stored on the Transaction. This block is only called when the Transaction is sampled. Use this block argument to avoid having to parse parameters for every transaction, even when it's not sampled.

```ruby
Appsignal.set_params do
# Some slow code to parse parameters
JSON.parse('{"param1": "value1"}')
end
```
33 changes: 28 additions & 5 deletions lib/appsignal/helpers/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -532,27 +532,50 @@ def tag_request(tags = {})
# When no parameters are set this way, the transaction will look for
# parameters in its request environment.
#
# @example
# A block can be given to this method to defer the fetching and parsing
# of the parameters until and only when the transaction is sampled.
#
# When both the `given_params` and a block is given to this method, the
# `given_params` argument is leading and the block will _not_ be called.
#
# @example Set parameters
# Appsignal.set_params("param1" => "value1")
#
# @example
# @example Calling `set_params` multiple times will only keep the last call
# Appsignal.set_params("param1" => "value1")
# Appsignal.set_params("param2" => "value2")
# # Parameters are: { "param2" => "value2" }
# # The parameters are: { "param2" => "value2" }
#
# @example Calling `set_params` with a block
# Appsignal.set_params do
# # Some slow code to parse parameters
# JSON.parse('{"param1": "value1"}')
# end
# # The parameters are: { "param1" => "value1" }
#
# @example Calling `set_params` with a parameter and a block
# Appsignal.set_params("argument" => "argument value") do
# # Some slow code to parse parameters
# JSON.parse('{"param1": "value1"}')
# end
# # The parameters are: { "argument" => "argument value" }
#
# @since 3.10.0
# @param given_params [Hash] The parameters to set on the transaction.
# @yield This block is called when the transaction is sampled. The block's
# return value will become the new parameters.
# @see https://docs.appsignal.com/guides/custom-data/sample-data.html
# Sample data guide
# @see https://docs.appsignal.com/guides/filter-data/filter-parameters.html
# Parameter filtering guide
# @see Transaction#set_params
# @return [void]
def set_params(params)
def set_params(params = nil, &block)
return unless active?
return unless Appsignal::Transaction.current?

transaction = Appsignal::Transaction.current
transaction.set_params(params)
transaction.set_params(params, &block)
end

# Add breadcrumbs to the transaction.
Expand Down
2 changes: 1 addition & 1 deletion lib/appsignal/rack/abstract_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def add_transaction_metadata_after(transaction, request)
request_method = request_method_for(request)
transaction.set_metadata("method", request_method) if request_method

transaction.set_params_if_nil(params_for(request))
transaction.set_params_if_nil { params_for(request) }
transaction.set_http_or_background_queue_start
end

Expand Down
25 changes: 20 additions & 5 deletions lib/appsignal/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def initialize(transaction_id, namespace, request, options = {})
@store = Hash.new({})
@options = options
@options[:params_method] ||= :params
@params = nil

@ext = Appsignal::Extension.start_transaction(
@transaction_id,
Expand Down Expand Up @@ -139,9 +140,13 @@ def store(key)
end

def params
return @params if defined?(@params)
parameters = @params || request_params

request_params
if parameters.respond_to? :call
parameters.call
else
parameters
end
end

# Set parameters on the transaction.
Expand All @@ -152,10 +157,17 @@ def params
# The parameters set using {#set_params} are leading over those extracted
# from a request's environment.
#
# When both the `given_params` and a block is given to this method, the
# `given_params` argument is leading and the block will _not_ be called.
#
# @since 3.9.1
# @param given_params [Hash] The parameters to set on the transaction.
# @yield This block is called when the transaction is sampled. The block's
# return value will become the new parameters.
# @return [void]
def set_params(given_params)
# @see {Helpers::Instrumentation#set_params}
def set_params(given_params = nil, &block)
@params = block if block
@params = given_params if given_params
end

Expand All @@ -175,9 +187,12 @@ def params=(given_params)
#
# @since 3.9.1
# @param given_params [Hash] The parameters to set on the transaction if none are already set.
# @yield This block is called when the transaction is sampled. The block's
# return value will become the new parameters.
# @return [void]
def set_params_if_nil(given_params)
set_params(given_params) unless @params
# @see {Helpers::Instrumentation#set_params_if_nil}
def set_params_if_nil(given_params = nil, &block)
set_params(given_params, &block) unless @params
end

# Set tags on the transaction.
Expand Down
70 changes: 70 additions & 0 deletions spec/lib/appsignal/transaction_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,16 @@ def create_transaction(id = transaction_id)
it "returns custom parameters" do
expect(subject).to eq(:foo => "bar")
end

context "when params is a callable object" do
it "calls the params object and sets the return value as parametesr" do
transaction.set_params { { "param1" => "value1" } }

expect(transaction.params).to eq(
"param1" => "value1"
)
end
end
end

context "without custom params set on transaction" do
Expand Down Expand Up @@ -374,6 +384,25 @@ def create_transaction(id = transaction_id)
expect(transaction.params).to eq(params)
expect(transaction).to include_params(params)
end

it "updates the params on the transaction with a block" do
params = { "key" => "value" }
transaction.set_params { params }

transaction._sample
expect(transaction.params).to eq(params)
expect(transaction).to include_params(params)
end

it "updates with the params argument when both an argument and block are given" do
arg_params = { "argument" => "value" }
block_params = { "block" => "value" }
transaction.set_params(arg_params) { block_params }

transaction._sample
expect(transaction.params).to eq(arg_params)
expect(transaction).to include_params(arg_params)
end
end

context "when the given params is nil" do
Expand Down Expand Up @@ -402,6 +431,25 @@ def create_transaction(id = transaction_id)
expect(transaction).to include_params(params)
end

it "updates the params on the transaction with a block" do
params = { "key" => "value" }
transaction.set_params_if_nil { params }

transaction._sample
expect(transaction.params).to eq(params)
expect(transaction).to include_params(params)
end

it "updates with the params argument when both an argument and block are given" do
arg_params = { "argument" => "value" }
block_params = { "block" => "value" }
transaction.set_params_if_nil(arg_params) { block_params }

transaction._sample
expect(transaction.params).to eq(arg_params)
expect(transaction).to include_params(arg_params)
end

context "when the given params is nil" do
it "does not update the params on the transaction" do
params = { "key" => "value" }
Expand All @@ -426,6 +474,17 @@ def create_transaction(id = transaction_id)
expect(transaction.params).to eq(preset_params)
expect(transaction).to include_params(preset_params)
end

it "does not update the params with a block on the transaction" do
preset_params = { "other" => "params" }
params = { "key" => "value" }
transaction.set_params(preset_params)
transaction.set_params_if_nil { params }

transaction._sample
expect(transaction.params).to eq(preset_params)
expect(transaction).to include_params(preset_params)
end
end
end

Expand Down Expand Up @@ -821,6 +880,17 @@ def to_s
kind_of(Integer)
)
end

context "when params is a callable object" do
it "calls the params object and sets the return value as parametesr" do
transaction.set_params { { "param1" => "value1" } }

transaction.sample_data
expect(transaction).to include_params(
"param1" => "value1"
)
end
end
end

describe "#set_error" do
Expand Down
7 changes: 7 additions & 0 deletions spec/lib/appsignal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,13 @@
transaction._sample
expect(transaction).to include_params("param2" => "value2")
end

it "sets parameters with a block on the transaction" do
Appsignal.set_params { { "param1" => "value1" } }

transaction._sample
expect(transaction).to include_params("param1" => "value1")
end
end

context "without transaction" do
Expand Down
Loading