Skip to content

feat(ext): cache resolved functions to reduce allocations - #19

Merged
Litarnus merged 3 commits into
mainfrom
cache-resolved-functions
Jul 9, 2026
Merged

feat(ext): cache resolved functions to reduce allocations#19
Litarnus merged 3 commits into
mainfrom
cache-resolved-functions

Conversation

@Litarnus

@Litarnus Litarnus commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Introduces a HashTable that stores resolved function informations by using the function pointer as key.

Before that, we would calculate a lot of constant stuff per invocation, such as display name, which produced unnecessary allocations and more overhead for instrumentation

Comment thread sentry.c
return handlers;
}

if (sentry_resolve_function(execute_data) != NULL) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Functions without a #[Sentry\Trace] attribute that are registered for tracing via \Sentry\instrument() after their first call will not be traced.
Severity: MEDIUM

Suggested Fix

Add a test case that covers the scenario of instrumenting a function (without an attribute) after its first invocation to reproduce the bug. The fix may require finding a way to have the PHP engine re-evaluate the observer decision for a function, as simply clearing the extension's internal cache is insufficient. If the PHP observer API does not support this, the behavior should be documented as a known limitation.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry.c#L1187

Potential issue: Due to how the PHP observer API caches observation decisions, a
function without a `#[Sentry\Trace]` attribute will not be traced if it is registered
via `\Sentry\instrument()` after it has already been called at least once in the current
process. When the function is first called, the `sentry_observer` callback returns empty
handlers because no attribute or registration exists. The PHP engine caches this
negative result. Subsequent calls to `\Sentry\instrument()` to register the function
will not cause the engine to re-evaluate, so the function is never traced, silently
failing the instrumentation.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is a current limitation and will be solved a future PR

Comment thread sentry.c
@Litarnus
Litarnus merged commit d69133c into main Jul 9, 2026
36 checks passed
@Litarnus
Litarnus deleted the cache-resolved-functions branch July 9, 2026 22:03

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bed408b. Configure here.

Comment thread sentry.c
&SENTRY_G(resolved_functions),
(zend_ulong) (uintptr_t) func,
&resolved_zv
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale cache after re-entrant instrument

Medium Severity

sentry_resolve_function snapshots registration state before sentry_get_attribute_metadata, then caches that result afterward. If attribute evaluation runs user code (autoload or new in an attribute) that calls instrument, the new registration is applied and the cache is cleared, but resolve then inserts attribute-only data on top. Later sentry_observer_begin cache hits keep using that stale entry for the rest of the request, so registration metadata and pre/post callbacks never take effect.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bed408b. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant