DONOTMERGE feat(agent): Fiber Support#1198
Draft
zsistla wants to merge 15 commits into
Draft
Conversation
* show_fiber functionality; indentations align with show_execs; the special keyword is "show_fibers" * basic user function fiber usage - does not include anything with special instrumentation. Instrumentation of internal functions have not been tested; although the show_fibers properly detects them. * a few basic tests * some get_current_segment, start_segment have been updated * added values to track global asynchronous context (the actual context we are in) and txn context (the context the txn is currently working with which can be modified with set_current_segment) * modified nr_txn_get_current_span_id for context to be passed * added nr_txn_get_current_context * added a few axiom unit tests This is not the complete implementation, but will provide a base for other PRs to iterate on.
zsistla
added
the
on hold
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1198 +/- ##
==========================================
+ Coverage 80.25% 80.46% +0.20%
==========================================
Files 189 191 +2
Lines 28002 28308 +306
==========================================
+ Hits 22474 22779 +305
- Misses 5528 5529 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… PHPs < 8.0 (#1211) Most of references to force_current_segment or nr_txn_force_current segment were placed in if/endif blocks with the OAPI effort. The change was never explicitly mentioned and there's a chance it could be used in a breaking manner. All remaining references in axiom specify explicitly it is not to be used with oapi. Axiom doesn't understand ZEND_API numbers so it can't be placed in if/endif blocks.
1) Updates `nr_txn_get_current_segment` usages to also incorporate new get_current_segment calls. 2) New functions added are: `nr_txn_get_current_segment_txn_context` (in axiom) `nr_php_txn_get_current_segment_php_context` (in agent since axiom is php agnostic) 3) Added additional integration tests
Analyze all segment starts to work with context information. Will behave as usual for non-fibers. Will use context information for starting fibers when in a fiber environment. 1) Updated segment start logic across the agent to be context aware 2) Added fiber integration tests to exercise the code paths 3) Incorporated bug fixes that surfaced during increased test coverage runs. Of the tests: * The _basic tests all follow the same pattern. * For the pdo/*/base-class tests, once you've seen one of the variants, the others are basically the same but for different databases. * the span_events/fibers/*.php tests going through various scenarios (starting/stopping/resume/noresume/errors/etc/nesting) etc. * guzzle7 and curl_multi_exec are actually async so there's quite a few more tests in that area.
## Summary
Each PHP Fiber now gets its own snapshot of the request-scoped globals
the
agent relies on (`ctx`), so instrumentation running inside
a fiber no longer trips over state owned by the main context (or another
fiber). The fiber observer hooks already in place from the parent branch
are wired up to populate, swap, and tear down these snapshots; the
`NRPRG_CTX` accessor transparently reads from the active
fiber's snapshot when one is set.
All new behavior is gated on PHP 8.1+.
## What changed
**Per-fiber globals (`agent/php_fibers.{c,h}`, new)**
- New `fiber_globals_t` = `{ ctx_globals_t* }`.
- `nr_fiber_copy_ctx_globals` deep-copy the parts a fiber should own.
- A per-request `fiber_globals_map` (keyed by `"%p"` of the
`zend_fiber_context*`) holds these snapshots; it has a destructor so
removed
entries are fully cleaned up. Init is lazy on first fiber; teardown
happens
in `nr_php_post_deactivate`. `php_rshutdown.c` clears
`NRPRG(fiber_globals)`
before normal global teardown so the existing free paths don't reach
into a
snapshot.
**OAPI fiber observer wiring (`agent/php_observer.c`)**
- `nr_fiber_init_observe`: lazy-init `fiber_globals_map`, add a snapshot
for
the new fiber under its `"%p"` key.
- `nr_fiber_destroy_observe`: remove the snapshot for that fiber.
- `nr_fiber_switch_observe`: at the end of the existing switch logic,
swap
`NRPRG(fiber_globals)` to the destination context's snapshot via
`nr_fiber_switch_global_context`.
**Globals accessors (`agent/php_newrelic.h`)**
- New `fiber_globals_t` struct; `NRPRG(fiber_globals_map)` and
`NRPRG(fiber_globals)` added to module globals.
- On PHP 8.1+, `NRPRG_CTX(Y)` resolves through
`NRPRG(fiber_globals)` when it's non-NULL, otherwise read the
main-context
globals exactly as before. Callers don't have to know which context
they're
in. PHP < 8.1 macros are unchanged.
---------
Co-authored-by: Amber Sistla <asistla@newrelic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.