Add fiber instrumentation #1802
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
always uses correct state
fallwith
reviewed
Feb 8, 2023
Co-authored-by: James Bunch <fallwith@gmail.com>
Co-authored-by: James Bunch <fallwith@gmail.com>
Co-authored-by: James Bunch <fallwith@gmail.com>
Co-authored-by: James Bunch <fallwith@gmail.com>
Co-authored-by: James Bunch <fallwith@gmail.com>
fallwith
approved these changes
Feb 8, 2023
| def thread_block_with_current_transaction(*args, segment_name:, parent: nil, &block) | ||
| parent ||= current_segment | ||
| current_txn = ::Thread.current[:newrelic_tracer_state].current_transaction if ::Thread.current[:newrelic_tracer_state] && ::Thread.current[:newrelic_tracer_state].is_execution_traced? | ||
| current_txn = ::Thread.current[:newrelic_tracer_state]&.current_transaction if ::Thread.current[:newrelic_tracer_state]&.is_execution_traced? |
There was a problem hiding this comment.
nice use of >2.3 syntax 😎
SimpleCov Report
|
tannalynn
commented
Feb 8, 2023
| @@ -410,9 +410,17 @@ def clear_state | |||
|
|
|||
| alias_method :tl_clear, :clear_state | |||
|
|
|||
| def current_segment_key | |||
| ::Fiber.current.object_id | |||
There was a problem hiding this comment.
Using the fiber id instead of the thread id (which is what we did before), still totally works with threads, because every new thread is a new fiber as well, but it also allows us to accurately track multiple fibers that exist inside of a single thread.
tannalynn
commented
Feb 8, 2023
| @@ -260,20 +257,26 @@ def initialize(category, options) | |||
| end | |||
| end | |||
|
|
|||
| def parent_thread_id | |||
| ::Thread.current.nr_parent_thread_id if ::Thread.current.respond_to?(:nr_parent_thread_id) | |||
| def state | |||
There was a problem hiding this comment.
This just makes sense the transaction is always using the correct state object. we were storing the state on the transaction originally, which wasn't causing any issues, but this is just a preemptive safety change for just in case situations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds instrumentation to the Fiber class.
This will allow the agent to treat fibers the same way it is currently treating threads, making it possible to automatically record spans created inside of fibers and will create a fiber spans.
This will share the
instrumentation.thread.tracingconfig to control whether the current transaction is carried over to the new fiber. It made more sense to use the same config as threads for this behavior, since there isn't any reason only one would need to be disabled.closes #1756