getsentry / sentry-javascript

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

Automatically instrument profiling for Node and Python when Sentry is set up via the Lambda Layer #11800

Open cstavitsky opened 2 months ago

cstavitsky commented 2 months ago

Problem Statement

Currently the Lambda Layer setup instructions for serverless functions suggest that errors and performance is automatically enabled:

You can configure Sentry by setting these environment variables for your Lambda function: SENTRY_INITIAL_HANDLER: The handler function of your AWS Lambda funtion. SENTRY_DSN: This is set to the DSN of your project SENTRY_TRACES_SAMPLE_RATE: This sets the sampling rate for transactions.

(I assume this applies to both Node and Python. The Node lambda layer docs are not explicit about what you get out of the box)

Beyond the above, though, it seems profiling has to be manually set up for Node lambdas and I believe also for Python lambdas. This manual step can be difficult when a customer has, say 1000 lambdas and doesn't want to instrument them all with profiling individually.

Solution Brainstorm

Add a configuration option for Node and Python profiling in the lambda layer.

Product Area

Profiling

getsantry[bot] commented 2 months ago

Assigning to @getsentry/support for routing ⏲️

getsantry[bot] commented 2 months ago

Routing to @getsentry/product-owners-profiling for triage ⏲️

JonasBa commented 2 months ago

@AbhiPrasad I guess this would mean that we inject the profiling integration based on an env variable. Is this something that we do in any other SDK and would there be a reason not to do this?

AbhiPrasad commented 2 months ago

I guess this would mean that we inject the profiling integration based on an env variable

This is a little more complicated because this means we would have to bundle in @sentry/profiling-node with the lambda layer we upload to AWS (also all of this lambda layer code is scary to touch).

I think the environmental variable is a great idea, but the bundling will cause us complexities, especially because there are 0 tests. I'm going to transfer this to the Sentry JavaScript SDK repo, but backlog because we have higher priority items atm. Maybe after v8 releases we can revisit.

JonasBa commented 2 months ago

I guess this would mean that we inject the profiling integration based on an env variable

This is a little more complicated because this means we would have to bundle in @sentry/profiling-node with the lambda layer we upload to AWS (also all of this lambda layer code is scary to touch).

I think the environmental variable is a great idea, but the bundling will cause us complexities, especially because there are 0 tests. I'm going to transfer this to the Sentry JavaScript SDK repo, but backlog because we have higher priority items atm. Maybe after v8 releases we can revisit.

Oh yes, I was assuming the user would have to ensure the binaries are there, so some manual work would be required anyways. Bundling the binaries with the package isnt a good idea imo, I'd rather defer to manual instrumentation

joelbarnard commented 1 week ago

Is this feature still on the radar?

AbhiPrasad commented 1 week ago

@joelbarnard this feature is still backlogged at the moment. PRs are welcome if you'd like to implement this though!