-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replacement for Sentry.Integrations.Apollo() #12887
Comments
Hey @RodoVJ - you don't need the explicit https://docs.sentry.io/platforms/javascript/guides/node/configuration/integrations/graphql/ We need to update the migration docs! |
Opened #12889 to track migration docs changes! |
@AbhiPrasad Thank you for getting back to me. With this change I am no longer able to get Apollo/GraphQL trace information to Sentry with I'm initiating Sentry in a This is what how I'm initiating Sentry: export const sentry = Sentry.init({
dsn',
includeLocalVariables: true,
enableTracing: true,
integrations: [
Sentry.httpIntegration(),
Sentry.expressIntegration(),
Sentry.graphqlIntegration(),
Sentry.postgresIntegration(),
Sentry.captureConsoleIntegration({ levels: ['error'] }),
],
tracesSampleRate: environment.production ? 0.2 : 1.0, }); |
you should be able to simplify your SDK config like so: export const sentry = Sentry.init({
dsn,
includeLocalVariables: true,
integrations: [
Sentry.captureConsoleIntegration({ levels: ['error'] }),
],
tracesSampleRate: environment.production ? 0.2 : 1.0,
}); you don't need to specify you also don't need to include Alright to debug your SDK setup. First off, could you add Second, you mentioned that your TS config does not support You may need to reference these docs to adjust installation based on if your code is ESM or CJS: https://docs.sentry.io/platforms/javascript/guides/node/install/ |
@AbhiPrasad Thanks for the tips on simplifying the integrations! My transpiled JavaScript has When I include
Btw I tried removing |
@RodoVJ can you share some more information on what happened in your application when these logs were produced? Is it all during startup? Weird idea, but can you try remove the |
@lforst It was all during startup. Also removing |
you are required to use We can see this is okay because the sdk logs out
From the logs the internal queue the SDK uses to send events seems to be full, hence it makes sense that no performance data is being sent. if you add Sentry.init({
beforeSend: (event) => {
console.log(event);
return event;
},
}); What version of Node.js are you using? |
We are using Node v.18.8. However I am now using v.18.19 locally since I was following the migration docs. I added the
|
@RodoVJ any chance you could provide a minimal reproduction that we could use to further investigate this issue? Based on the logs you posted it seems like our SDK is able to patch |
Oh another thing I jut noticed: Since we're looking for missing spans, please log the event sent in |
@Lms24 I apologize for the late reply, I will get you this information by tomorrow :). I appreciate the support! |
@Lms24 I think I figured out what the issue is.
The event is being sent, the problem comes with how the transaction is being displayed on Sentry. On Sentry I can only see the transaction as I did a small test like this so to check this behavior:
This does change the transaction name (based on logging the current scope) but it's not reflected in Sentry. Something interested to note is that when I compare the event ID logged and the one on Sentry, they are the same. But when I compare the trace ID's they are not the same. I'm getting the trace id from printing the current scope. Any insight would be appreciated! |
Calling Believe me, I'm more than unhappy about the naming colission here but our move away from calling the root span "transaction" will hopefully alleviate this a bit in the future.
Did you use this before updating to v8 as well? Does it also call If you have such a plugin and it updates the name before the transaction event is sent, here's how you can manually update the root span's (formerly known as transaction) name: const span = Sentry.getActiveSpan();
if (span) {
const rootSpan = Sentry.getRootSpan(span);
rootSpan.updateName('customName');
}
// set this to have the same transaction name reported for error events
Sentry.getCurrentScope().setTransactionName('customName') I would recommend you call both -
This is strange but possibly expected. Generally, you should get one trace id for all events happening throughout one server request life cycle. This might not be the same trace id as the one on the scope's propagationContext since it's handled by openTelemetry. |
Before v8 we used the method below but Sentry.configureScope((scope) => {
scope.setTransactionName(`GraphQL ${ctx.request.operationName}`);
});
I tried this method in the plugin and when I log the In fact I tried changing the span's name in the beforeSendTransaction: (event) => {
const customName = 'JUST TESTING';
const span = Sentry.getActiveSpan();
if (span) {
const rootSpan = Sentry.getRootSpan(span);
rootSpan.updateName(customName);
console.log(Sentry.getRootSpan(span)) // name is still set to `POST /graphql`
}
Sentry.getCurrentScope().setTransactionName(customName);
return event;
},
You are right about this, I think that trace id is from openTelemtry. When logging the |
Ahh this might be related to our name inferral method. Not sure though yet. I'll check on Monday. |
Hi @Lms24 any updates on this? |
Still gotta check the name inferral I was mentioning earlier. But if you're able to overwrite the name to what you want it to be at the time when beforeSendTransaction: (event) => {
const customName = getYourName();
// explanation: Your root span's name becomes the `event.transaction` property when
// we convert the span to an event in the SDK
event.transaction = customName;
return event;
}, Side note: Calling |
What you can also try, in case const span = Sentry.getActiveSpan();
if (span) {
const rootSpan = Sentry.getRootSpan(span);
rootSpan.updateName('customName');
rootSpan.setAttribute('sentry.skip_span_data_inference', true);
}
// set this to have the same transaction name reported for error events
Sentry.getCurrentScope().setTransactionName('customName') Background: I think that your root span is created by our |
@Lms24 I tried your last suggestion above, using |
This works, but it seems pretty inefficient: beforeSendTransaction(event) {
if (event.spans) {
for (const span of event.spans) {
const operationName = span.data?.["graphql.operation.name"];
if (typeof operationName === "string") {
event.transaction = operationName;
break;
}
}
}
return event;
}, |
@trevorr thanks for providing a workaround. I've filed an enhancement issue where we could potentially tackle this at the integration level. |
Problem Statement
I'm trying to upgrade to v8 of
@sentry/node
. However the Apollo integration was deprecated and can't find a replacement for it. I can see a replacement for other integrations have been made here https://github.com/getsentry/sentry-javascript/blob/develop/MIGRATION.md but there is no mention of the Apollo integration.Solution Brainstorm
I'd expect the v8 migration to provide an alternative to the Apollo integration that was available in v7.
The text was updated successfully, but these errors were encountered: