getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.87k stars 1.55k forks source link

Child span/transaction name replaced by Lambda function name #13391

Open mstuercke opened 1 month ago

mstuercke commented 1 month ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/aws-serverless

SDK Version

8.26.0

Framework Version

No response

Link to Sentry event

https://mstuercke.sentry.io/performance/trace/7b533ed86b30290955b43e8b10513c5d/?node=txn-0e0841f1cf3b40a581c8cb50bc1736a9&node=txn-8b8c36aab7304b0e94d50cd767c1a2eb&pageEnd&pageStart&project=4507512062017616&source=traces&statsPeriod=5m&timestamp=1723726598.892

Reproduction Example/SDK Setup

Sentry.init({
  dsn: '<DSN>',
  tracesSampleRate: 1.0,
  environment: 'dev',
})

export const lambdaHandler = Sentry.wrapHandler(() => {
  return Sentry.startSpan({op: `foo-op`, name: 'foo-name', forceTransaction: true}, () => {
    // ...
  })
})

Steps to Reproduce

Just execute the code

Additional information: The renaming happens here: https://github.com/getsentry/sentry-javascript/blob/5c08d03dbe6c298ed94b847253c6674f2a3f3855/packages/aws-serverless/src/sdk.ts#L219-L226

Expected Result

The span should keep the name foo-name

Actual Result

The span name is replaced with the name of the lambda function. The op is untouched.

AbhiPrasad commented 1 month ago

Hey @mstuercke thanks for writing in!

wrapHandler should be automatically creating transactions - why do you need to create a span with forceTransaction: true?

The scope.addEventProcessor is meant to apply to the transaction created by wrapHandler

mstuercke commented 4 weeks ago

wrapHandler should be automatically creating transactions - why do you need to create a span with forceTransaction: true?

I wanted to write a custom tRPC request handler inside a lambda function and name it like {op: 'trpc.query', name: '/path'}. But the name will always be overwritten. I wanted to create a transaction for that to analyze it separately. But a transaction seems not to be the best choice for that, right? I also wanted to keep the lambda metrics.

The scope.addEventProcessor is meant to apply to the transaction created by wrapHandler

It does, but it also renames nested transactions

Anyway, it would be nice to somehow rename the transaction created by wrapHandler. Seeing a lot of function.aws.lambda — <function-name> in Traces makes it harder to differenciate the executions

andreiborza commented 4 weeks ago

Hello, thanks for writing in. We are on company-wide hackweek and thus on limited support this week. We'll take a look at this next week.

andreiborza commented 3 weeks ago

Hello, we are looking into this.

AlexMayleRdn commented 2 weeks ago

This appears to be the case even if you use the method of assigning transaction names described in the documentation and do not attempt to create a span. Relevant docs: https://docs.sentry.io/platforms/javascript/guides/aws-lambda/enriching-events/transaction-name/

// @ts-ignore it's in a lambda layer
const Sentry = require("@sentry/aws-serverless");

Sentry.init({
    dsn: process.env.SENTRY_DSN,
    environment: process.env.SENTRY_ENVIRONMENT,
    debug: true,
    release: randomBytes(8).toString('hex'),
    traceSampleRate: 1.0
});

export default Sentry.wrapHandler(async (
    event: APIGatewayProxyEventV2WithJWTAuthorizer,
): Promise<APIGatewayProxyResultV2<ISpecificSystem | IListSystemsResp | void>> => {
    Sentry.getCurrentScope().setTransactionName(`${event.requestContext.http.method} ${event.requestContext.http.path}`);

    try {
        throw new Number(10);
    } finally {
        await new Db().cleanUp();
    }
})

My transaction name associated with the error is just the lambda name instead of like I'd like.

andreiborza commented 1 week ago

Hi, sorry for the silence on this. I haven't had time to look into this yet, but hopefully this week.

andreiborza commented 2 days ago

@mstuercke sorry for the late response. Are your lambdas in non-transpiled ESM?

The code block you identified only triggers when the lambda handler isn't wrapped by OpenTelemetry. We definitely need to change that so only the root span is named after the function.

From my testing, I can name child spans started inside lambdas however I want tho - but I used CJS to test.

Going forward, we will probably not add the option to rename the rootspan. You'd have to do that at instantiation of the instrumentation which comes from OpenTelemetry and with some drawbacks.

That being said, you should be able to rename these yourself as a user, using our beforeSendTransaction hook, would that work for you as a workaround?

ancheetah commented 2 days ago

Hi @andreiborza , have you had a chance to look into @AlexMayleRdn comment? I am having the same issue. Sentry.getCurrentScope().setTransactionName() does not appear to do anything. We still see the lambda name as the transaction name in Sentry.

andreiborza commented 1 day ago

@ancheetah the transaction name is set by opentelemetry, we can't change that via setTransaction. Can you try changing it via a beforeSend hook in Sentry.init?

mstuercke commented 1 day ago

Thank you for the response!

Are your lambdas in non-transpiled ESM?

Yes, it's not transpiled. I'm using Bun and execute .ts files directly

The code block you identified only triggers when the lambda handler isn't wrapped by OpenTelemetry.

I'm not using OpenTelemetry

That being said, you should be able to rename these yourself as a user, using our beforeSendTransaction hook, would that work for you as a workaround?

I think that I've already tried that and even that didn't work. But I'm not 100% sure and didn't retry it yet.

andreiborza commented 1 day ago

@mstuercke right. Yea for the non-opentelemetry case every transaction gets renamed. I'll be working on a fix for that but it might take a while.

ancheetah commented 1 day ago

@ancheetah the transaction name is set by opentelemetry, we can't change that via setTransaction. Can you try changing it via a beforeSend hook in Sentry.init?

@andreiborza that did not work for me. Update: It works, I needed to refresh my issues page.

  beforeSend(event) {
    event.transaction = 'my-transaction-name'
    return event
  }

However, I was able to use beforeSend another way to reach my end goal and filter out events on my dev environment by returning null for a specific tag. Thanks for your help!