Skip to content

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Sep 18, 2025

Supercedes #16874

This PR makes two changes

  1. It adds logger as an export to @sentry/core, and then refactors the browser, cloudflare and vercel-edge packages to just re-export @sentry/core's logger.

This change makes it easy to use logging in an isomorphic way, and reduces duplication between our various packages. We couldn't change the export from node-core because it has a different type signature than the standard logger.

  1. It expands the logger exports to accept an optional scope argument. This allows for users to provide their own custom clients to the methods, which helps with standalone client cases.
import * as Sentry from "@sentry/browser";

const client = createMySentryClient();
const scope = new Sentry.Scope();
scope.setClient(client);

Sentry.logger.info("Hello World!", {}, { scope });

Copy link
Contributor

github-actions bot commented Sep 18, 2025

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 9,290 - 8,870 +5%
GET With Sentry 1,422 15% 1,378 +3%
GET With Sentry (error only) 6,119 66% 5,946 +3%
POST Baseline 1,211 - 1,176 +3%
POST With Sentry 530 44% 492 +8%
POST With Sentry (error only) 1,075 89% 988 +9%
MYSQL Baseline 3,342 - 3,255 +3%
MYSQL With Sentry 516 15% 450 +15%
MYSQL With Sentry (error only) 2,714 81% 2,620 +4%

View base workflow run

@AbhiPrasad AbhiPrasad marked this pull request as ready for review September 18, 2025 15:46
@AbhiPrasad AbhiPrasad requested review from a team, stephanie-anderson, Lms24 and mydea and removed request for a team September 18, 2025 15:46
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

LGTM, just a general question regarding the scope parameter: AFAICT, we only use it to get the scope's client. No complaints with that but I'm wondering if you chose to go with scope to potentially extract other scope data as well in the future? If not, wouldn't it be easier to let users pass in the client directly?
Just a thought, as I said, the scope approach is fine.

@@ -1,11 +1,8 @@
import { feedbackAsyncIntegration } from './feedbackAsync';
import { feedbackSyncIntegration } from './feedbackSync';
import * as logger from './log';
Copy link
Member

Choose a reason for hiding this comment

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

does this mean we can remove log.ts from browser?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was removed, git just tracked it as a renamed
image

@AbhiPrasad
Copy link
Member Author

LGTM, just a general question regarding the scope parameter: AFAICT, we only use it to get the scope's client. No complaints with that but I'm wondering if you chose to go with scope to potentially extract other scope data as well in the future? If not, wouldn't it be easier to let users pass in the client directly? Just a thought, as I said, the scope approach is fine.

We use the scope to grab tracing info + user attributes:

const [, traceContext] = _getTraceInfoFromScope(client, currentScope);
const processedLogAttributes = {
...beforeLog.attributes,
};
const {
user: { id, email, username },
} = getMergedScopeData(currentScope);

The variable is just called currentScope when it's used, so it's a bit confusing to trace the path.

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.

2 participants