Use thread-local variables instead of fiber-local variables to store global configuration
pdany1116 opened this issue · 1 comments
Currently, Appsignal is using fiber-local variables to store the current appsignal transaction and some information about logger.
When opening a new Fiber, you will not have access to the Appsignal current transaction inside the new fiber since the transaction it's stored in a fiber-local variable and they are not shared between fibers. (https://docs.ruby-lang.org/en/3.2/Thread.html#method-i-5B-5D) See below example:
require 'appsignal'
Appsignal::Transaction.create(
SecureRandom.uuid,
Appsignal::Transaction::HTTP_REQUEST,
{}
)
pp Appsignal::Transaction.current # => Appsignal::Transaction:0x00007f865a78f2c0
Fiber.new do
pp Appsignal::Transaction.current # => Appsignal::Transaction::NilTransaction:0x00007f865a5b96f8
end.resume
My suggestion is to make the Appsignal transaction available as a thread local variable, instead of using Thread.current[:appsignal_transaction]
to use Thread.current.thread_variable_set(:appsignal_transaction, ...)
and Thread.current.thread_variable_get(:appsignal_transaction)`.
I have encountered this issue using dry-effects library. Please see this issue.
I am not sure if there are any other concerns, but I can open up a PR with the changes if green light.
Thank you!
TODO
- Replace all
Thread.current[]
andThread.current[]=
withThread.current.thread_variable_set
andThread.current.thread_variable_get
Hi @pdany1116,
Thanks for the suggestion!
With the way our transaction API works, it doesn't support async events. Having fibers concurrently instrumented will result in a broken event timeline. Best way to go about it right now is to create a new transaction in a fiber.
We'll update the Ruby gem's API in the future, to support concurrent events better.