Skip to content

Conversation

@IlyasShabi
Copy link
Contributor

@IlyasShabi IlyasShabi commented Nov 6, 2025

Motivation

Enable resource renaming tests for Node.js on express and fastify weblogs

Ref: DataDog/dd-trace-js#6861

Changes

  • Enable tests
  • Create /resource_naming handler in weblogs

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

"DD_LOGS_INJECTION": "true",
"DD_TRACE_RESOURCE_RENAMING_ENABLED": "true",
"DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT": "true",
"DD_TRACE_COMPUTE_STATS": "true",
Copy link
Contributor Author

@IlyasShabi IlyasShabi Nov 6, 2025

Choose a reason for hiding this comment

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

DD_TRACE_STATS_COMPUTATION_ENABLED is the right env var for all tracers
@florentinl

"DD_TRACE_COMPUTE_STATS": "true",
"DD_TRACE_STATS_COMPUTATION_ENABLED": "true",
},
appsec_enabled=False,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enabling appsec will also enable resource renaming without the need of adding new env var and with the help of http.createServer flexibility that Node.js offers for Express/Fastify

@IlyasShabi IlyasShabi marked this pull request as ready for review November 6, 2025 13:13
@IlyasShabi IlyasShabi requested review from a team as code owners November 6, 2025 13:13
@IlyasShabi IlyasShabi changed the title [Node.js@ishabi/endpoint-resource-renaming] Enable resource renaming tests for Node.js [Nodejs@ishabi/endpoint-resource-renaming] Enable resource renaming tests for Node.js Nov 7, 2025
@IlyasShabi IlyasShabi changed the title [Nodejs@ishabi/endpoint-resource-renaming] Enable resource renaming tests for Node.js [nodejs@ishabi/endpoint-resource-renaming] Enable resource renaming tests for Node.js Nov 7, 2025
@github-actions
Copy link
Contributor

CODEOWNERS have been resolved as:

manifests/nodejs.yml                                                    @DataDog/dd-trace-js
tests/test_resource_renaming.py                                         @DataDog/system-tests-core
utils/_context/_scenarios/__init__.py                                   @DataDog/system-tests-core
utils/build/docker/nodejs/express/app.js                                @DataDog/dd-trace-js @DataDog/system-tests-core
utils/build/docker/nodejs/express4-typescript/app.ts                    @DataDog/dd-trace-js @DataDog/system-tests-core
utils/build/docker/nodejs/fastify/app.js                                @DataDog/dd-trace-js @DataDog/system-tests-core

@cbeauchesne
Copy link
Collaborator

Setting this PR as draft to unstress my notifications 😉

@cbeauchesne cbeauchesne marked this pull request as draft November 12, 2025 10:53
@IlyasShabi IlyasShabi force-pushed the ishabi/endpoint-resource-renaming branch from 9fd6deb to 9fde0ef Compare November 12, 2025 13:39
@IlyasShabi IlyasShabi changed the title [nodejs@ishabi/endpoint-resource-renaming] Enable resource renaming tests for Node.js [nodejs] Enable resource renaming tests for Node.js Nov 14, 2025
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

Successfully merging this pull request may close these issues.

4 participants