Skip to content

Conversation

@andreiborza
Copy link
Member

@andreiborza andreiborza commented Jan 17, 2025

Previously, we only had one node lambda layer called SentryNodeServerlessSDK. With the v8 major of the sentry-javascript SDKs a new layer for v7 was published under SentryNodeServerlessSDKv7 while the v8 layer rolled over into SentryNodeServerlessSDK.

With v8 the @sentry/serverless package was removed (see: #70137) and a version check was added to the integration to update NODE_OPTIONS for the integration accordingly. The version check assumes the SentryNodeServerlessSDK name and only checks the ARN version which is not enough to identify a v7 layer since the introduction of SentryNodeServerlessSDKv7.

With v9 we will stop updating the SentryNodeServerlessSDK and instead publish SentryNodeServerlessSDKv9 exclusively, the integration has to ensure the correct NODE_OPTIONS are set depending on layer name and ARN version which this PR adds.

Closes: #82646

@andreiborza andreiborza requested review from a team as code owners January 17, 2025 12:25
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 17, 2025
@andreiborza andreiborza force-pushed the ab/improved-lambda-layer-node-options-checks branch from 062ba8a to 45c49db Compare January 17, 2025 12:37
@codecov
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #83633      +/-   ##
==========================================
+ Coverage   87.53%   87.59%   +0.05%     
==========================================
  Files        9409     9491      +82     
  Lines      537867   538935    +1068     
  Branches    21179    21179              
==========================================
+ Hits       470836   472094    +1258     
+ Misses      66683    66493     -190     
  Partials      348      348              

Previously, we only had one node lambda layer called `SentryNodeServerlessSDK`.
With the `v8` major of the sentry-javascript SDKs a new layer for `v7` was
published under `SentryNodeServerlessSDKv7` while the `v8` layer rolled over
into `SentryNodeServerlessSDK`.

With `v8` the `@sentry/serverless` package was removed (see: #70137) and a
version check was added to the integration to update `NODE_OPTIONS` for the
integration accordingly. The version check assumes the `SentryNodeServerlessSDK`
name and only checks the ARN version which is not enough to identify a `v7`
layer since the introduction of `SentryNodeServerlessSDKv7`.

With `v9` we will stop updating the `SentryNodeServerlessSDK` and instead
publish `SentryNodeServerlessSDKv9` exclusively, the integration has to ensure
the correct `NODE_OPTIONS` are set depending on layer name and ARN version which
this PR adds.
Copy link
Collaborator

@iamrajjoshi iamrajjoshi left a comment

Choose a reason for hiding this comment

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

changes lgtm, going to leave the final stamp to someone with a bit more context.

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.

Nice, good that we're now also taking the layer name into account. I guess previously, the SentryNodeServerlessSDKv7 layer would have implicitly worked because its ARN version number by chance was <=235? Def better to check this explicitly.

@andreiborza
Copy link
Member Author

Nice, good that we're now also taking the layer name into account. I guess previously, the SentryNodeServerlessSDKv7 layer would have implicitly worked because its ARN version number by chance was <=235? Def better to check this explicitly.

Yep, happened to work because of that. For the v8 layer that completely broke which is how we found that out 😅.

@andreiborza andreiborza merged commit 69f87da into master Jan 20, 2025
49 checks passed
@andreiborza andreiborza deleted the ab/improved-lambda-layer-node-options-checks branch January 20, 2025 15:10
andrewshie-sentry pushed a commit that referenced this pull request Jan 22, 2025
…rs (#83633)

Previously, we only had one node lambda layer called
`SentryNodeServerlessSDK`. With the `v8` major of the sentry-javascript
SDKs a new layer for `v7` was published under
`SentryNodeServerlessSDKv7` while the `v8` layer rolled over into
`SentryNodeServerlessSDK`.

With `v8` the `@sentry/serverless` package was removed (see: #70137) and
a version check was added to the integration to update `NODE_OPTIONS`
for the integration accordingly. The version check assumes the
`SentryNodeServerlessSDK` name and only checks the ARN version which is
not enough to identify a `v7` layer since the introduction of
`SentryNodeServerlessSDKv7`.

With `v9` we will stop updating the `SentryNodeServerlessSDK` and
instead publish `SentryNodeServerlessSDKv9` exclusively, the integration
has to ensure the correct `NODE_OPTIONS` are set depending on layer name
and ARN version which this PR adds.

Closes: #82646
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v9] Improve version check for node lambda layer

3 participants