Deprecate Transaction.create/new arguments

Remove the transaction ID argument. Have the Transaction generate its own ID. Previously, we used request/job IDs for Transaction IDs, but that's been refactored out to use the `request_id` tag. No longer require the request argument. When none is given it will use the fallback InternalGenericRequest, but this is backwards compatibility step until we remove the request from the Transaction entirely. Transaction metadata will need to be set using the `Appsignal.set_*` helpers.
appsignal · Jul 11, 2024 · 2fc2c61 · 2fc2c61
1 parent 1c69d3f
commit 2fc2c61
Show file tree

Hide file tree

Showing 20 changed files with 1,662 additions and 1,485 deletions.
diff --git a/.changesets/deprecate-arguments-for-transaction-create-and-transaction-new.md b/.changesets/deprecate-arguments-for-transaction-create-and-transaction-new.md
@@ -0,0 +1,18 @@
+---
+bump: patch
+type: deprecate
+---
+
+Deprecate the 'ID', 'request', and 'options' arguments for the `Transaction.create` and `Transaction.new` methods. To add metadata to the transaction, use the `Appsignal.set_*` helpers. Read our [sample data guide](https://docs.appsignal.com/guides/custom-data/sample-data.html) for more information on how to set metadata on transactions.
+
+```ruby
+# Before
+Appsignal::Transaction.create(
+ SecureRandom.uuid,
+ "my_namespace",
+ Appsignal::Transaction::GenericRequest.new(env) # env is a request env Hash
+)
+
+# After
+Appsignal::Transaction.create("my_namespace")
+```
diff --git a/benchmark.rake b/benchmark.rake
@@ -67,11 +67,9 @@ def start_agent
 end
 
 def monitor_transaction(transaction_id)
- request = Appsignal::Transaction::InternalGenericRequest.new({})
  transaction = Appsignal::Transaction.create(
  transaction_id,
- Appsignal::Transaction::HTTP_REQUEST,
- request
+ Appsignal::Transaction::HTTP_REQUEST
  )
  transaction.set_action("HomeController#show")
  transaction.set_params(:id => 1)

diff --git a/lib/appsignal/demo.rb b/lib/appsignal/demo.rb
@@ -39,11 +39,7 @@ def transmit
  private
 
  def create_example_error_request
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::HTTP_REQUEST)
  begin
  raise TestError,
  "Hello world! This is an error used for demonstration purposes."
@@ -60,11 +56,7 @@ def create_example_error_request
  end
 
  def create_example_performance_request
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::HTTP_REQUEST)
  Appsignal.instrument "action_view.render", "Render hello.html.erb",
  "<h1>Hello world!</h1>" do
  sleep 2

diff --git a/lib/appsignal/helpers/instrumentation.rb b/lib/appsignal/helpers/instrumentation.rb
@@ -237,8 +237,7 @@ def send_error(
  end
  transaction = Appsignal::Transaction.new(
  SecureRandom.uuid,
- namespace || Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
+ namespace || Appsignal::Transaction::HTTP_REQUEST
  )
  transaction.set_tags(tags) if tags
  transaction.set_error(error)
@@ -392,8 +391,7 @@ def report_error(exception)
  else
  Appsignal::Transaction.new(
  SecureRandom.uuid,
- Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
+ Appsignal::Transaction::HTTP_REQUEST
  )
  end
 

diff --git a/lib/appsignal/hooks/action_cable.rb b/lib/appsignal/hooks/action_cable.rb
@@ -36,11 +36,8 @@ def install_subscribe_callback
  request_id = request.request_id || SecureRandom.uuid
  env[Appsignal::Hooks::ActionCableHook::REQUEST_ID] ||= request_id
 
- transaction = Appsignal::Transaction.create(
- env[Appsignal::Hooks::ActionCableHook::REQUEST_ID],
- Appsignal::Transaction::ACTION_CABLE,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction =
+ Appsignal::Transaction.create(Appsignal::Transaction::ACTION_CABLE)
 
  begin
  Appsignal.instrument "subscribed.action_cable" do
@@ -75,11 +72,8 @@ def install_unsubscribe_callback
  request_id = request.request_id || SecureRandom.uuid
  env[Appsignal::Hooks::ActionCableHook::REQUEST_ID] ||= request_id
 
- transaction = Appsignal::Transaction.create(
- env[Appsignal::Hooks::ActionCableHook::REQUEST_ID],
- Appsignal::Transaction::ACTION_CABLE,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction =
+ Appsignal::Transaction.create(Appsignal::Transaction::ACTION_CABLE)
 
  begin
  Appsignal.instrument "unsubscribed.action_cable" do

diff --git a/lib/appsignal/hooks/active_job.rb b/lib/appsignal/hooks/active_job.rb
@@ -54,11 +54,7 @@ def execute(job)
  # we do for Sidekiq.
  #
  # Prefer job_id from provider, instead of ActiveJob's internal ID.
- Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
  end
 
  if transaction

diff --git a/lib/appsignal/integrations/action_cable.rb b/lib/appsignal/integrations/action_cable.rb
@@ -11,11 +11,7 @@ def perform_action(*args, &block)
  request_id = request.request_id || SecureRandom.uuid
  env[Appsignal::Hooks::ActionCableHook::REQUEST_ID] ||= request_id
 
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::ACTION_CABLE,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::ACTION_CABLE)
 
  begin
  super

diff --git a/lib/appsignal/integrations/delayed_job_plugin.rb b/lib/appsignal/integrations/delayed_job_plugin.rb
@@ -17,11 +17,8 @@ class DelayedJobPlugin < ::Delayed::Plugin
  end
 
  def self.invoke_with_instrumentation(job, block)
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction =
+ Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
 
  Appsignal.instrument("perform_job.delayed_job") do
  block.call(job)

diff --git a/lib/appsignal/integrations/que.rb b/lib/appsignal/integrations/que.rb
@@ -5,11 +5,8 @@ module Integrations
  # @api private
  module QuePlugin
  def _run(*)
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction =
+ Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
 
  begin
  Appsignal.instrument("perform_job.que") { super }

diff --git a/lib/appsignal/integrations/rake.rb b/lib/appsignal/integrations/rake.rb
@@ -33,11 +33,7 @@ def execute(*args)
  private
 
  def _appsignal_create_transaction
- Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
  end
  end
 

diff --git a/lib/appsignal/integrations/resque.rb b/lib/appsignal/integrations/resque.rb
@@ -5,11 +5,7 @@ module Integrations
  # @api private
  module ResqueIntegration
  def perform
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
 
  Appsignal.instrument "perform.resque" do
  super

diff --git a/lib/appsignal/integrations/shoryuken.rb b/lib/appsignal/integrations/shoryuken.rb
@@ -5,11 +5,7 @@ module Integrations
  # @api private
  class ShoryukenMiddleware
  def call(worker_instance, queue, sqs_msg, body, &block)
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
 
  Appsignal.instrument("perform_job.shoryuken", &block)
  rescue Exception => error # rubocop:disable Lint/RescueException

diff --git a/lib/appsignal/integrations/sidekiq.rb b/lib/appsignal/integrations/sidekiq.rb
@@ -38,12 +38,7 @@ def call(exception, sidekiq_context, _sidekiq_config = nil)
  # Sidekiq error outside of the middleware scope.
  # Can be a job JSON parse error or some other error happening in
  # Sidekiq.
- transaction =
- Appsignal::Transaction.create(
- SecureRandom.uuid, # Newly generated job id
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
  transaction.set_action_if_nil("SidekiqInternal")
  transaction.set_metadata("sidekiq_error", sidekiq_context[:context])
  transaction.set_params_if_nil(:jobstr => sidekiq_context[:jobstr])
@@ -65,11 +60,7 @@ class SidekiqMiddleware
 
  def call(_worker, item, _queue, &block)
  job_status = nil
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::BACKGROUND_JOB,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::BACKGROUND_JOB)
  transaction.set_action_if_nil(formatted_action_name(item))
 
  formatted_metadata(item).each do |key, value|

diff --git a/lib/appsignal/integrations/webmachine.rb b/lib/appsignal/integrations/webmachine.rb
@@ -10,11 +10,7 @@ def run
  if has_parent_transaction
  Appsignal::Transaction.current
  else
- Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ Appsignal::Transaction.create(Appsignal::Transaction::HTTP_REQUEST)
  end
 
  Appsignal.instrument("process_action.webmachine") do

diff --git a/lib/appsignal/rack/abstract_middleware.rb b/lib/appsignal/rack/abstract_middleware.rb
@@ -34,11 +34,7 @@ def call(env)
  if wrapped_instrumentation
  env[Appsignal::Rack::APPSIGNAL_TRANSACTION]
  else
- Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ Appsignal::Transaction.create(Appsignal::Transaction::HTTP_REQUEST)
  end
 
  unless wrapped_instrumentation

diff --git a/lib/appsignal/rack/event_handler.rb b/lib/appsignal/rack/event_handler.rb
@@ -61,11 +61,7 @@ def on_start(request, _response)
  request.env[APPSIGNAL_EVENT_HANDLER_ID] ||= id
  return unless request_handler?(request.env[APPSIGNAL_EVENT_HANDLER_ID])
 
- transaction = Appsignal::Transaction.create(
- SecureRandom.uuid,
- Appsignal::Transaction::HTTP_REQUEST,
- Appsignal::Transaction::InternalGenericRequest.new({})
- )
+ transaction = Appsignal::Transaction.create(Appsignal::Transaction::HTTP_REQUEST)
  request.env[APPSIGNAL_TRANSACTION] = transaction
 
  request.env[RACK_AFTER_REPLY] ||= []

diff --git a/lib/appsignal/transaction.rb b/lib/appsignal/transaction.rb
@@ -24,15 +24,57 @@ class Transaction
  class << self
  # Create a new transaction and set it as the currently active
  # transaction.
- def create(id, namespace, request, options = {})
+ def create(id_or_namespace, arg_namespace = nil, request = nil, options = {})
+ if arg_namespace
+ id = id_or_namespace
+ namespace = arg_namespace
+ else
+ id = SecureRandom.uuid
+ namespace = id_or_namespace
+ end
+
+ if id
+ Appsignal.internal_logger.warn(
+ "Appsignal::Transaction.create: " \
+ "A new Transaction is created using the transaction ID argument. " \
+ "This argument is deprecated without replacement."
+ )
+ end
+ if arg_namespace
+ Appsignal.internal_logger.warn(
+ "Appsignal::Transaction.create: " \
+ "A Transaction is created using the namespace argument. " \
+ "Specify the namespace as the first argument to the 'create' " \
+ "method without the ID argument."
+ )
+ end
+ if request
+ Appsignal.internal_logger.warn(
+ "Appsignal::Transaction.create: " \
+ "A Transaction is created using the request argument. " \
+ "This argument is deprecated. Please use the `Appsignal.set_*` helpers instead."
+ )
+ end
  # Allow middleware to force a new transaction
- Thread.current[:appsignal_transaction] = nil if options.include?(:force) && options[:force]
+ if options[:force]
+ Appsignal.internal_logger.warn(
+ "Appsignal::Transaction.create: " \
+ "A Transaction is created using the `:force => true` option argument. " \
+ "The options argument is deprecated without replacement."
+ )
+ Thread.current[:appsignal_transaction] = nil
+ end
 
  # Check if we already have a running transaction
  if Thread.current[:appsignal_transaction].nil?
  # If not, start a new transaction
  Thread.current[:appsignal_transaction] =
- Appsignal::Transaction.new(id, namespace, request, options)
+ Appsignal::Transaction.new(
+ id,
+ namespace,
+ request,
+ options
+ )
  else
  # Otherwise, log the issue about trying to start another transaction
  Appsignal.internal_logger.warn(
@@ -87,11 +129,15 @@ def clear_current_transaction!
  attr_reader :ext, :transaction_id, :action, :namespace, :request, :paused, :tags, :options,
  :breadcrumbs, :custom_data
 
- def initialize(transaction_id, namespace, request, options = {})
+ # Use {.create} to create new transactions.
+ #
+ # @see create
+ # @api private
+ def initialize(transaction_id, namespace, request = nil, options = {})
  @transaction_id = transaction_id
  @action = nil
  @namespace = namespace
- @request = request
+ @request = request || InternalGenericRequest.new({})
  @paused = false
  @discarded = false
  @tags = {}

diff --git a/spec/lib/appsignal/hooks/action_cable_spec.rb b/spec/lib/appsignal/hooks/action_cable_spec.rb
@@ -302,11 +302,6 @@ def self.to_s
  if DependencyHelper.rails6_present?
  context "with ConnectionStub" do
  let(:connection) { ActionCable::Channel::ConnectionStub.new }
- let(:transaction_id) { "Stubbed transaction id" }
- before do
- # Stub future (private AppSignal) transaction id generated by the hook.
- expect(SecureRandom).to receive(:uuid).and_return(transaction_id)
- end
 
  it "does not fail on missing `#env` method on `ConnectionStub`" do
  instance.unsubscribe_from_channel