Skip to content

add node24 as supported runtime #692

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 1, 2025
Merged

add node24 as supported runtime #692

merged 3 commits into from
Dec 1, 2025

Conversation

@ojproductions
Copy link
Contributor

@ojproductions ojproductions commented Nov 25, 2025 •
edited
Loading

What does this PR do?

adds node 24.11 as a supported runtime

Motivation

APMSVLS-162

Testing Guidelines

Additional Notes

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

@ojproductions ojproductions marked this pull request as ready for review November 25, 2025 15:32
@ojproductions ojproductions requested review from a team as code owners November 25, 2025 15:32
astuyve
astuyve reviewed Nov 25, 2025
View reviewed changes
Copy link
Contributor

@astuyve astuyve left a comment

Choose a reason for hiding this comment

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

We should bump the version of dd-trace-js in our package.json/package-lock.json, likely it's a major version bump to add support for 24.x but you should ask that team

scripts/publish_layers.sh
NODE_VERSIONS_FOR_AWS_CLI=("nodejs18.x" "nodejs20.x" "nodejs22.x")
LAYER_PATHS=(".layers/datadog_lambda_node18.12.zip" ".layers/datadog_lambda_node20.19.zip" ".layers/datadog_lambda_node22.11.zip")
AVAILABLE_LAYERS=("Datadog-Node18-x" "Datadog-Node20-x" "Datadog-Node22-x")
NODE_VERSIONS_FOR_AWS_CLI=("nodejs18.x" "nodejs20.x" "nodejs22.x" "nodejs24.x")
Copy link
Contributor

@astuyve astuyve Nov 25, 2025

Choose a reason for hiding this comment

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

Hi! This looks great. Just a heads up that at one point we had an incident where I did not append the new layer version to all of these variables which caused an off-by-one error and an incompatible layer to be published. This is because we iterate through these lists pair/touple-wise and expect them to be exactly the same length.

Some assertions would help save us, but let's also just carefully check this is correct everywhere

Copy link
Contributor Author

@ojproductions ojproductions Nov 25, 2025

Choose a reason for hiding this comment

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

I added some validation in all the relevant scripts. lmk if that fits what you were thinking.

src/metrics/enhanced-metrics.ts
* If a version cannot be identified, returns null
* See https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html
*/
export function getRuntimeTag(): string | null {
Copy link
Contributor

@astuyve astuyve Nov 25, 2025

Choose a reason for hiding this comment

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

we could probably refactor this a bit

README.md
[![License](https://img.shields.io/badge/license-Apache--2.0-blue)](https://github.com/DataDog/datadog-lambda-js/blob/main/LICENSE)

Datadog Lambda Library for Node.js (18.x, 20.x, and 22.x) enables enhanced Lambda metrics, distributed tracing, and custom metric submission from AWS Lambda functions.
Datadog Lambda Library for Node.js (18.x, 20.x, 22.x, and 24.x) enables enhanced Lambda metrics, distributed tracing, and custom metric submission from AWS Lambda functions.
Copy link
Contributor

@astuyve astuyve Nov 25, 2025

Choose a reason for hiding this comment

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

todo: deprecate v18.x because it's been deprecated by lambda since september

Copy link
Contributor Author

@ojproductions ojproductions Nov 25, 2025

Choose a reason for hiding this comment

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

should I do that here or maybe split into a different PR?

Copy link
Contributor

@astuyve astuyve Nov 26, 2025

Choose a reason for hiding this comment

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

different PR is fine

@ryanb93 ryanb93 mentioned this pull request Nov 25, 2025
Closed
@ojproductions
Copy link
Contributor Author

ojproductions commented Nov 25, 2025

likely it's a major version bump to add support for 24.x but you should ask that team

@astuyve I bumped to 5.80.0 for now but do yk which team I should be asking? I did run all the unit/integration tests and they pass

@ojproductions
Copy link
Contributor Author

ojproductions commented Dec 1, 2025

/merge

@dd-devflow-routing-codex
Copy link

dd-devflow-routing-codex bot commented Dec 1, 2025 •
edited
Loading

View all feedbacks in Devflow UI.

2025-12-01 17:26:23 UTC ℹ️ Start processing command /merge

2025-12-01 17:26:28 UTC ℹ️ MergeQueue: waiting for PR to be ready

This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-12-01 17:35:21 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in main is approximately 0s (p90).

2025-12-01 17:45:04 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue bot merged commit 995a42b into main Dec 1, 2025
34 checks passed
@dd-mergequeue dd-mergequeue bot deleted the onzia/add-node-24 branch December 1, 2025 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@astuyve astuyve astuyve approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ojproductions @astuyve