Skip to content

startTransaction method no longer returning transaction #4731

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

Closed
3 tasks done
crice88 opened this issue Mar 17, 2022 · 24 comments
Closed
3 tasks done

startTransaction method no longer returning transaction #4731

crice88 opened this issue Mar 17, 2022 · 24 comments
Assignees

Comments

@crice88
Copy link

crice88 commented Mar 17, 2022

Is there an existing issue for this?

How do you use Sentry?

Saas (sentry.io)

Which package are you using?

@sentry/node

SDK Version

6.18.2

Framework Version

No response

Link to Sentry event

No response

Steps to Reproduce

We are using Nestjs and create spanned transactions for logging. Recently upgraded from 6.11.0 to 6.18.2 and began seeing the issue. Here's just a bit of sample usage:

const { method, headers, url } = this.request;
    const transaction = Sentry.startTransaction({
      name: `Route: ${method} ${url}`,
      op: 'transaction'
    });

    Sentry.getCurrentHub().configureScope((scope) => {
      scope.setSpan(transaction);
      scope.setContext('http', {
        method,
        url,
        headers
      });
    });

Transaction is undefined in this instance so a scope is never configured. I have tested the startTransaction method in two separate projects and both are experiencing the same issue. Downgrading library back to 6.11.0 fixes the issue.

Expected Result

startTransaction method should return an instance of transaction so it can be used for proper scoping.

Actual Result

When pulling the scoped span via Sentry.getCurrentHub().getScope()?.getSpan(); I am receiving undefined as the span was never created properly due to the transaction being undefined.

@lobsterkatie
Copy link
Member

Hi, @crice88.

This is very odd. startTransaction, when set up correctly, should always return a transaction, so what this says to me is that the guys of the function aren't getting injected the way they should. Here is the code that does that, which runs when @sentry/tracing is imported.

I can't think right now of any change we've made at all recently which would affect that. Would you be able to provide a minimal repro for us to poke at?

@crice88
Copy link
Author

crice88 commented Mar 18, 2022

This repo is not mine, but is affected with the same issue

If you were to update the sentry SDK to current in the project, you'll see the error being thrown.

@ericjeker
Copy link

Thanks @crice88, saw your message on StackOverflow. I am the owner of the Nest.js example. I can reproduce the bug on my side as well. I am not sure what's happening so far. It seems I have another error in my log:

Sentry Logger [Warn]: Extension method startTransaction couldn't be found, doing nothing.

As you said this started after upgrading from 6.11. My repo hasn't changed since Aug 18, 2021.

@geraldosalazar16
Copy link

geraldosalazar16 commented Mar 21, 2022

I am also getting the undefined transaction issue for a Nest JS API. In my case it only happens on windows, I tested on a Macbook Air with an M1 chip and transactions where working properly but tested on two PCs (Windows 10 and 11) and I am not able to create transactions. I also got the idea of trying with version 6.11.0 of @sentry/node and transactions started working

@j-elmer123
Copy link

I have the same problem here when I tried to implement https://blog.sentry.io/2021/08/31/guest-post-performance-monitoring-in-graphql#solution it seems I have to use version 6.11.0 for @sentry/node @sentry/tracing and @sentry/types for now.

@sionzee
Copy link

sionzee commented Mar 22, 2022

Same problem, startTransaction returns undefined on the sentry object

@danizord
Copy link

Importing the @sentry/tracing package before calling Sentry.startTransaction() fixed the issue for me.

@onurtemizkan
Copy link
Collaborator

You can use:
import "@sentry/tracing"
when importing @sentry/node as a temporary workaround while we're working on this.

Name-space imports such as:
import * as Tracing from "@sentry/tracing"
currently don't inject relevant functionality unless you specifically use Tracing somewhere in your project.

@niieani
Copy link

niieani commented Mar 25, 2022

Encountered the same problem. Confirming that import "@sentry/tracing" as a workaround fixes the issue.

@j-elmer123
Copy link

just a question for this, is this means that this issue is fixed by this? getsentry/sentry-docs#4876 and doesn't need to import @sentry/tracing anymore? @onurtemizkan

@crice88
Copy link
Author

crice88 commented Mar 30, 2022

@j-elmer123 no, that's just documentation. When a fix has been merged, they'll close this issue.

@SimonSchick
Copy link
Contributor

SimonSchick commented Apr 8, 2022

Encountering the same issue after updating to sentry sdk latest version 6.19.6, this unfortunately caused us some downtime, can you please alter the return type of startTransaction to include | undefined? This would be a temporary indicator so at least some or your users will not be affected.

Adding import '@sentry/tracing'; as an explicit import doesn't seem to affect this either, (we already import parts of it elsewhere in the code base).

andipaetzold added a commit to andipaetzold/zwiftmap that referenced this issue Apr 12, 2022
@mcrawshaw
Copy link

mcrawshaw commented Apr 13, 2022

@SimonSchick Sometimes your bundler might be a little too smart and will remove the import statement that has no use. Try import * as Tracing from "@sentry/tracing";, then use it somewhere below like Tracing && true;.

@kpdecker
Copy link
Contributor

kpdecker commented Apr 19, 2022

Manually reverting eb09c28 solved this for me. @onurtemizkan I can submit a PR to revert, but unsure of the consequences of that.

6.19.4 confirmed good

@simonwinter
Copy link

Concur with @kpdecker - 6.19.4 has the issue resolved.

@n1ru4l
Copy link

n1ru4l commented Apr 20, 2022

In 6.19.6 having a import "@sentry/tracing"; statement is still mandatory for a normal (unbundled) Node.js application. 😕

@JVMartin
Copy link

JVMartin commented May 11, 2022

I'm on 6.19.7 and still have this issue, the result is undefined unless I import @sentry/tracing.

@lforst
Copy link
Contributor

lforst commented May 12, 2022

@JVMartin We thought we fixed this in #4955. Can you double check if you have version 6.19.7 in your node_modules (or simply post the output of npm ls @sentry/core)? If yes, we should investigate further.

@JVMartin
Copy link

JVMartin commented May 12, 2022

@lforst Yes, 100% sure. Here's the breakdown of what happened...

We were working great in production, then we shipped this upgrade:

-        "@sentry/node": "^6.17.7",
-        "@sentry/tracing": "^6.17.7",
+        "@sentry/node": "^6.19.7",
+        "@sentry/tracing": "^6.19.7",

Production went down because startTransaction started to return undefined.

We updated the file that calls startTransaction with:

+// TODO: This is a workaround; see https://github.com/getsentry/sentry-javascript/issues/4731
+// Check for updates, maybe they have a fix
+import '@sentry/tracing';

And then production recovered.

Here is the full file with the patch included:

import * as Sentry from '@sentry/node';
import { Transaction } from '@sentry/types';
import { ApolloServerPlugin, GraphQLRequestListener } from 'apollo-server-plugin-base';

// TODO: This is a workaround; see https://github.com/getsentry/sentry-javascript/issues/4731
// Check for updates, maybe they have a fix
import '@sentry/tracing';

export interface Context {
  transaction: Transaction;
}

export function createContext(): Context {
  const transaction = Sentry.startTransaction({
    op: 'gql',
    name: 'GraphQLTransaction', // this will be the default name, unless the gql query has a name
  });
  return { transaction };
}

export const SentryPlugin: ApolloServerPlugin<Context> = {
  async requestDidStart(requestContext): Promise<GraphQLRequestListener<Context>> {
    const context = requestContext.context;
    const request = requestContext.request;

    if (request.operationName) {
      // set the transaction Name if we have named queries
      context.transaction.setName(request.operationName);
    }
    return {
      willSendResponse(): any {
        context.transaction.finish();
      },
      executionDidStart(): any {
        return {
          willResolveField({ info }): any {
            // hook for each new resolver
            const span = context.transaction.startChild({
              op: 'resolver',
              description: `${info.parentType.name}.${info.fieldName}`,
            });
            return () => {
              // this will execute once the resolver is finished
              span.finish();
            };
          },
        };
      },
    };
  },
};

^ this file was working fine on 6.17.7 without the import '@sentry/tracing';, but it only works on 6.19.7 with the import added.

> npm ls @sentry/core
[email protected] /home/user/code/api
└─┬ @sentry/[email protected]
  └── @sentry/[email protected]

@lforst
Copy link
Contributor

lforst commented May 13, 2022

@JVMartin just checked back with the rest of the team. In short: The way you've set up things right now is the intended and correct way. We have updated the docs to list import '@sentry/tracing'; as a required import statement.

For anyone stumbling onto this issue, please import as follows for tracing to work:

import * as Sentry from "@sentry/node";
import "@sentry/tracing";

@lforst lforst closed this as completed May 13, 2022
@shankie-aa
Copy link

FWIW, none of these solutions above helped me until I upgraded all my Sentry packages to 7.1.1

@raoulus
Copy link

raoulus commented Oct 12, 2022

Still facing this issue with version "@sentry/node": "7.14.2",. Any update on this?

@MichaelSp
Copy link

MichaelSp commented Nov 21, 2022

@lforst Even tho it's documented like that, given all the comments and confusion in this ticket, it's rather unexpected.

How about something like this:

import { NodeTracing } "@sentry/tracing";

Sentry.init({
    // ...
    integrations: [new NodeTracing()],
});

Even if its useless and does nothing, but it adds the syntactic sugar to avoid confusion.

@lforst
Copy link
Contributor

lforst commented Nov 23, 2022

@MichaelSp Good idea. Our long-term plan for this is probably to merge the tracing packages into the respective packages and to export some kind of tracing integration as you mentioned. It would be pretty easy to do that for @sentry/node but in @sentry/browser we need to keep CDN bundles and tree-shakability in mind.

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

No branches or pull requests