fix(pino): support proxied loggers like cls-proxify

[naaa760]

PR Description

Fix Pino Integration with cls-proxify

Problem: Pino logs weren't appearing in Sentry when using cls-proxify to wrap loggers.

Root Cause: Logger detection and tracking didn't handle proxy-wrapped Pino instances.

Solution:

• Enhanced logger detection for proxied loggers
• Replaced symbol tracking with WeakSet for proxy compatibility
• Added proxy target detection for manual tracking

Testing: Added test scenarios for proxied loggers.

Closes #18940

[sentry]

Bug: The proxy detection logic using logger.constructor === Proxy is incorrect. It will always fail, preventing manually tracked proxied loggers from being instrumented.
_{Severity: HIGH}

Suggested Fix

The proxy detection logic needs to be rewritten. Instead of checking logger.constructor, a more reliable method should be used. Since direct introspection of proxies is limited, consider if the tracking function can be designed to always receive the target object, or if both the proxy and target need to be tracked explicitly. The current implementation attempting to access __target__ or target properties is also incorrect and should be removed.

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: packages/node-core/src/integrations/pino.ts#L283-L289

Potential issue: The mechanism to detect and handle proxied Pino loggers is
fundamentally flawed. The condition `logger.constructor === Proxy` will never evaluate
to true because a proxy's `constructor` property resolves to its target's constructor.
Consequently, the logic to extract the underlying target logger and add it to the
`trackedLoggers` `WeakSet` is dead code. When a user manually tracks a proxied logger
with `autoInstrument: false`, the diagnostic hook receives the target object, but only
the proxy is in the `WeakSet`. This mismatch causes `shouldTrackLogger` to return false,
and no logs are captured.

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

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[cursor]

Proxy detection check will never execute

High Severity

The check logger.constructor === Proxy will never be true because accessing constructor on a proxy goes through the proxy's handler, which returns the target's constructor, not Proxy. JavaScript provides no standard way to detect proxies. When trackLogger(proxiedLogger) is called, only the proxy gets added to trackedLoggers, but the diagnostic channel receives the target logger (due to .apply(target, args) in the proxy handler binding this to the target). Since the target isn't tracked, proxied loggers fail to capture logs when autoInstrument: false.

Additional Locations (1)

• packages/node-core/src/integrations/pino.ts#L300-L307

 

[cursor]

Child loggers not tracked when parent tracked

High Severity

Child loggers created from tracked parent loggers aren't automatically tracked with autoInstrument: false. When logger.child() is called, it returns a new logger instance not present in trackedLoggers. The shouldTrackLogger function checks if the child is in the WeakSet, finds it missing, and returns options.autoInstrument (false), causing child loggers to not be captured. Existing tests like scenario-track.mjs expect child loggers to inherit tracking status from parents but this functionality is missing.

 

[chargome]

Hey @naaa760, thanks for your contribution! The issue in question hasn’t been confirmed or reproduced yet, so no fix is needed at this point.

Since we’ve run into this situation before, to avoid you spending time on a fix that might end up being closed, we’d recommend first sharing your intended approach with us (the maintainers) before opening a PR. This will save time for both you and us 🙏