Skip to content

refactor(express): trace middleware via layer prototype dispatch#9067

Merged
tlhunter merged 5 commits into
masterfrom
BridgeAR/2026-06-25-express-router-prototype-dispatch
Jul 14, 2026
Merged

refactor(express): trace middleware via layer prototype dispatch#9067
tlhunter merged 5 commits into
masterfrom
BridgeAR/2026-06-25-express-router-prototype-dispatch

Conversation

@BridgeAR

@BridgeAR BridgeAR commented Jun 25, 2026

Copy link
Copy Markdown
Member

Summary

loopback's phase-based middleware sorting relies on _findLayerByHandler mapping each layer back to the user handler. The router instrumentation replaced layer.handle with a wrapper, then (in #9062) re-exposed the handler through an enumerable _datadog_orig back-reference so that lookup could still find it — keeping the tracer coupled to loopback's private reflection heuristic, where the property name and its enumerability are both load-bearing.

This replaces that shim: trace middleware by wrapping the host Layer prototype dispatch and reading per-layer metadata from a WeakMap, leaving layer.handle the user's function.

  1. router.jswrapStack annotates each layer in a WeakMap (annotateLayer) instead of replacing its handle; a new createLayerDispatchWrappers(name) wraps handle_request/handle_error (and handleRequest/handleError on router >=2). Drops wrapLayerHandle, the arity-split wrappers, the _datadog_orig back-reference, the __handle branch, and the _name write-back onto the user handler.
  2. express.js — wraps express 4's bundled Layer prototype; express 5 / standalone router flow through the router-package layer hook.

Why

loopback already works on master via #9062's _datadog_orig property, so this is a no-behavior-change refactor. Keeping layer.handle pristine makes loopback's first lookup (layer.handle === handler) match with no tracer-specific contract, removing the coupling to its for-in reflection heuristic entirely. As a side effect the design is lighter: one shared monomorphic dispatch wrapper replaces a per-layer closure, and the hot path drops its try/catch (a synchronous throw or rejected promise becomes next(error) in the host dispatch, which the wrapped next already captures).

Test plan

  • packages/datadog-instrumentations/test/router.spec.js rewritten to the dispatch model — 28 passing (pristine handle, arity gates, no-subscriber fast paths, host-converted errors, re-entrancy guard).
  • Hot-path microbench: prototype dispatch + WeakMap is within noise of (slightly faster than) the per-layer closure — no regression.
  • CI express matrix (4 and 5), router matrix (>=1 <2, >=2), and the loopback 2.x/3.x suite — version matrices are not installable locally; the loopback re-sorting canary (spans[4] resource handleDD) is the gate.
  • express-async-errors compatibility — handle is its getter/setter; the dispatch reads this.handle and the .length arity gate holds, so the __handle special case is gone.
  • codecov/patch — the router-package Layer addHook bodies and the nested-router mount line are covered by the express/router plugin integration jobs, not the unit spec.

Refs: #9062

@pr-commenter

pr-commenter Bot commented Jun 25, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-07 20:57:52

Comparing candidate commit 6ea01fd in PR branch BridgeAR/2026-06-25-express-router-prototype-dispatch with baseline commit 64bc811 in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 2263 metrics, 20 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:appsec-appsec-enabled-24

  • unstable execution_time [-155.112ms; +152.288ms] or [-5.901%; +5.793%]

scenario:appsec-appsec-enabled-26

  • unstable execution_time [-170865.631µs; +170835.265µs] or [-6.871%; +6.870%]

scenario:appsec-control-20

  • unstable execution_time [-125.103ms; +133.952ms] or [-7.310%; +7.827%]

scenario:appsec-control-24

  • unstable execution_time [-97816.023µs; +96343.023µs] or [-8.103%; +7.981%]

scenario:appsec-control-26

  • unstable execution_time [-96.023ms; +98.615ms] or [-8.089%; +8.308%]

scenario:debugger-line-probe-with-snapshot-default-26

  • unstable max_rss_usage [-17.089MB; +13.428MB] or [-6.594%; +5.181%]

scenario:dogstatsd-with-tags-20

  • unstable cpu_user_time [-349.658ms; +274.369ms] or [-7.438%; +5.837%]
  • unstable execution_time [-348.916ms; +272.565ms] or [-7.310%; +5.710%]
  • unstable throughput [-92942.785op/s; +118495.456op/s] or [-5.292%; +6.747%]

scenario:plugin-graphql-long-with-depth-and-collapse-off-24

  • unstable cpu_user_time [-530.655ms; +241.061ms] or [-11.737%; +5.332%]
  • unstable execution_time [-569.053ms; +251.710ms] or [-11.843%; +5.238%]
  • unstable max_rss_usage [-69.454MB; +17.706MB] or [-12.839%; +3.273%]
  • unstable throughput [-2.416op/s; +5.393op/s] or [-5.409%; +12.073%]

scenario:plugin-graphql-long-with-depth-off-20

  • unstable max_rss_usage [-7.529MB; +14.380MB] or [-5.151%; +9.838%]

scenario:plugin-graphql-long-with-depth-off-26

  • unstable max_rss_usage [-14.427MB; +4.475MB] or [-8.880%; +2.754%]

scenario:plugin-redis-get-26

  • unstable cpu_user_time [-310.269ms; +195.768ms] or [-9.591%; +6.051%]
  • unstable execution_time [-311.917ms; +197.665ms] or [-9.457%; +5.993%]
  • unstable instructions [-3215.3M instructions; +2064.0M instructions] or [-8.410%; +5.399%]
  • unstable throughput [-492732.106op/s; +781055.945op/s] or [-11.797%; +18.699%]

scenario:test-optimization-large-suite-20

  • unstable max_rss_usage [-6.490MB; +2.836MB] or [-7.997%; +3.495%]

Middleware tracing replaced `layer.handle` with a wrapper and re-exposed the
user handler through an enumerable `_datadog_orig` back-reference, the only
property loopback's `_findLayerByHandler` could follow to tag a layer with its
phase. That ties the tracer to loopback's private reflection heuristic, where
the property name and its enumerability are both load-bearing.

Wrap the host `Layer` prototype dispatch (`handle_request`/`handle_error`, and
`handleRequest`/`handleError` on router >=2) and read per-layer metadata from a
WeakMap, leaving `layer.handle` the user's function. loopback's first lookup
(`layer.handle === handler`) then matches with no tracer-specific contract, so
the back-reference, the `express-async-errors` `__handle` branch, and the
`_name` write-back onto the user handler are no longer needed. A synchronous
throw or a rejected promise becomes `next(error)` in the host dispatch, so the
wrapped `next` captures both without a tracer-side try/catch.

Refs: #9062
@BridgeAR
BridgeAR force-pushed the BridgeAR/2026-06-25-express-router-prototype-dispatch branch from bab9d0d to 5d5b152 Compare June 25, 2026 21:22
@dd-octo-sts

dd-octo-sts Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Overall package size

Self size: 6.64 MB
Deduped: 7.3 MB
No deduping: 7.3 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.1 | 122.62 kB | 437.94 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@datadog-prod-us1-6

datadog-prod-us1-6 Bot commented Jun 25, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 96.51%

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 6ea01fd | Docs | Datadog PR Page | Give us feedback!

@BridgeAR BridgeAR changed the title fix(express): restore loopback middleware phase ordering refactor(express): trace middleware via layer prototype dispatch Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.54%. Comparing base (64bc811) to head (6ea01fd).
⚠️ Report is 98 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9067      +/-   ##
==========================================
+ Coverage   91.80%   96.54%   +4.73%     
==========================================
  Files         885      913      +28     
  Lines       50753   120545   +69792     
  Branches    10012    20347   +10335     
==========================================
+ Hits        46596   116381   +69785     
- Misses       4157     4164       +7     
Flag Coverage Δ
aiguard 53.63% <ø> (?)
aiguard-integration 57.42% <67.30%> (?)
aiguard-integration-active ?
aiguard-integration-latest ?
aiguard-integration-maintenance ?
aiguard-macos ?
aiguard-windows ?
apm-bucket-0 53.79% <ø> (?)
apm-bucket-1 58.10% <ø> (?)
apm-bucket-2 56.12% <ø> (?)
apm-capabilities-tracing 60.50% <29.32%> (?)
apm-capabilities-tracing-macos ?
apm-capabilities-tracing-ubuntu-active ?
apm-capabilities-tracing-ubuntu-latest ?
apm-capabilities-tracing-ubuntu-maintenance ?
apm-capabilities-tracing-ubuntu-oldest ?
apm-capabilities-tracing-windows ?
apm-integrations-aerospike 52.94% <ø> (?)
apm-integrations-aerospike-18-gte.5.2.0 ?
apm-integrations-aerospike-20-gte.5.5.0 ?
apm-integrations-aerospike-22-gte.5.12.1 ?
apm-integrations-aerospike-22-gte.6.0.0 ?
apm-integrations-aerospike-eol- ?
apm-integrations-confluentinc-kafka-javascript 57.07% <ø> (?)
apm-integrations-confluentinc-kafka-javascript-18 ?
apm-integrations-confluentinc-kafka-javascript-20 ?
apm-integrations-confluentinc-kafka-javascript-22 ?
apm-integrations-confluentinc-kafka-javascript-24 ?
apm-integrations-couchbase 53.26% <ø> (?)
apm-integrations-couchbase-18 ?
apm-integrations-couchbase-eol ?
apm-integrations-dns ?
apm-integrations-elasticsearch ?
apm-integrations-http 57.98% <29.32%> (?)
apm-integrations-http-latest ?
apm-integrations-http-oldest ?
apm-integrations-http2 ?
apm-integrations-kafkajs 57.72% <ø> (?)
apm-integrations-kafkajs-latest ?
apm-integrations-kafkajs-oldest ?
apm-integrations-next 54.24% <ø> (?)
apm-integrations-next-11.1.4 ?
apm-integrations-next-12.3.7 ?
apm-integrations-next-13.0.0 ?
apm-integrations-next-13.2.0 ?
apm-integrations-next-13.5.11 ?
apm-integrations-next-14.0.0 ?
apm-integrations-next-14.2.35 ?
apm-integrations-next-14.2.6 ?
apm-integrations-next-14.2.7 ?
apm-integrations-next-15.0.0 ?
apm-integrations-next-15.4.0 ?
apm-integrations-next-latest ?
apm-integrations-oracledb ?
apm-integrations-prisma 54.18% <ø> (?)
apm-integrations-prisma-18-gte.6.16.0.and.lt.7.0.0 ?
apm-integrations-prisma-latest-all ?
apm-integrations-restify ?
apm-integrations-sharedb ?
apm-integrations-tedious 53.04% <ø> (+19.06%) ⬆️
appsec 69.04% <ø> (?)
appsec-express ?
appsec-express_fastify_graphql 65.78% <85.09%> (?)
appsec-integration 45.51% <62.50%> (?)
appsec-integration-active ?
appsec-integration-latest ?
appsec-integration-maintenance ?
appsec-integration-oldest ?
appsec-kafka ?
appsec-kafka_ldapjs_lodash 59.13% <ø> (?)
appsec-ldapjs ?
appsec-lodash ?
appsec-macos ?
appsec-mongodb-core ?
appsec-mongodb-core_mongoose_mysql 62.39% <84.61%> (?)
appsec-mongoose ?
appsec-next 52.86% <ø> (?)
appsec-next-latest-11.1.4 ?
appsec-next-latest-12.3.7 ?
appsec-next-latest-13.0.0 ?
appsec-next-latest-13.2.0 ?
appsec-next-latest-13.5.11 ?
appsec-next-latest-14.0.0 ?
appsec-next-latest-14.2.35 ?
appsec-next-latest-14.2.6 ?
appsec-next-latest-14.2.7 ?
appsec-next-latest-15.0.0 ?
appsec-next-latest-latest ?
appsec-next-oldest-11.1.4 ?
appsec-next-oldest-12.3.7 ?
appsec-next-oldest-13.0.0 ?
appsec-next-oldest-13.2.0 ?
appsec-next-oldest-13.5.11 ?
appsec-next-oldest-14.0.0 ?
appsec-next-oldest-14.2.35 ?
appsec-next-oldest-14.2.6 ?
appsec-next-oldest-14.2.7 ?
appsec-next-oldest-15.0.0 ?
appsec-node-serialize ?
appsec-node-serialize_passport_postgres 62.03% <84.61%> (?)
appsec-passport ?
appsec-postgres ?
appsec-sourcing ?
appsec-sourcing_stripe_template 60.30% <69.23%> (?)
appsec-stripe ?
appsec-ubuntu ?
debugger 65.67% <ø> (?)
debugger-ubuntu-active ?
debugger-ubuntu-latest ?
debugger-ubuntu-maintenance ?
debugger-ubuntu-oldest ?
instrumentations-bucket-0 48.77% <ø> (?)
instrumentations-bucket-1 55.02% <69.23%> (?)
instrumentations-bucket-10 56.67% <66.82%> (?)
instrumentations-bucket-11 48.73% <ø> (?)
instrumentations-bucket-12 48.91% <92.02%> (?)
instrumentations-bucket-13 48.67% <ø> (?)
instrumentations-bucket-2 49.89% <85.09%> (?)
instrumentations-bucket-3 53.91% <73.07%> (?)
instrumentations-bucket-4 49.16% <ø> (?)
instrumentations-bucket-5 53.08% <43.75%> (?)
instrumentations-bucket-6 55.62% <ø> (?)
instrumentations-bucket-7 53.61% <ø> (?)
instrumentations-bucket-8 54.69% <69.23%> (?)
instrumentations-bucket-9 56.13% <66.82%> (?)
instrumentations-instrumentation-ai ?
instrumentations-instrumentation-aws-sdk ?
instrumentations-instrumentation-bluebird ?
instrumentations-instrumentation-body-parser ?
instrumentations-instrumentation-child_process ?
instrumentations-instrumentation-connect ?
instrumentations-instrumentation-cookie-parser ?
instrumentations-instrumentation-couchbase 48.16% <ø> (?)
instrumentations-instrumentation-couchbase-18 ?
instrumentations-instrumentation-couchbase-eol ?
instrumentations-instrumentation-crypto ?
instrumentations-instrumentation-express ?
instrumentations-instrumentation-express-mongo-sanitize ?
instrumentations-instrumentation-express-multi-version ?
instrumentations-instrumentation-express-session ?
instrumentations-instrumentation-fastify ?
instrumentations-instrumentation-fetch ?
instrumentations-instrumentation-fs ?
instrumentations-instrumentation-generic-pool ?
instrumentations-instrumentation-hono ?
instrumentations-instrumentation-http ?
instrumentations-instrumentation-http-client-options ?
instrumentations-instrumentation-kafkajs ?
instrumentations-instrumentation-knex ?
instrumentations-instrumentation-koa ?
instrumentations-instrumentation-light-my-request ?
instrumentations-instrumentation-mongoose ?
instrumentations-instrumentation-multer ?
instrumentations-instrumentation-mysql2 ?
instrumentations-instrumentation-openai-lifecycle ?
instrumentations-instrumentation-otel-sdk-trace ?
instrumentations-instrumentation-passport ?
instrumentations-instrumentation-passport-http ?
instrumentations-instrumentation-passport-local ?
instrumentations-instrumentation-pg ?
instrumentations-instrumentation-promise ?
instrumentations-instrumentation-promise-js ?
instrumentations-instrumentation-q ?
instrumentations-instrumentation-restify ?
instrumentations-instrumentation-router ?
instrumentations-instrumentation-stripe ?
instrumentations-instrumentation-url ?
instrumentations-instrumentation-when ?
instrumentations-instrumentation-zlib ?
instrumentations-integration-esbuild 33.96% <29.32%> (?)
instrumentations-integration-esbuild-0.16.12-active ?
instrumentations-integration-esbuild-0.16.12-latest ?
instrumentations-integration-esbuild-0.16.12-maintenance ?
instrumentations-integration-esbuild-0.16.12-oldest ?
instrumentations-integration-esbuild-latest-active ?
instrumentations-integration-esbuild-latest-latest ?
instrumentations-integration-esbuild-latest-maintenance ?
instrumentations-integration-esbuild-latest-oldest ?
llmobs-ai ?
llmobs-ai_anthropic_bedrock 57.28% <ø> (?)
llmobs-anthropic ?
llmobs-bedrock ?
llmobs-bucket-1 56.81% <ø> (?)
llmobs-google-genai ?
llmobs-langchain ?
llmobs-openai 57.58% <ø> (?)
llmobs-openai-latest ?
llmobs-openai-oldest ?
llmobs-sdk 59.99% <ø> (?)
llmobs-sdk-active ?
llmobs-sdk-latest ?
llmobs-sdk-maintenance ?
llmobs-sdk-oldest ?
llmobs-vertex-ai 54.22% <ø> (+18.51%) ⬆️
master-coverage 96.54% <100.00%> (?)
openfeature 54.63% <60.57%> (?)
openfeature-macos ?
openfeature-unit 49.82% <ø> (?)
openfeature-unit-active ?
openfeature-unit-latest ?
openfeature-unit-maintenance ?
openfeature-unit-oldest ?
openfeature-windows ?
platform-core ?
platform-core_esbuild_instrumentations-misc 38.45% <46.15%> (?)
platform-esbuild ?
platform-instrumentations-misc ?
platform-integration 62.19% <60.57%> (?)
platform-integration-active ?
platform-integration-latest ?
platform-integration-maintenance ?
platform-integration-oldest ?
platform-shimmer ?
platform-shimmer_unit-guardrails_webpack 37.21% <29.32%> (?)
platform-unit-guardrails ?
platform-webpack ?
plugins-aws-durable-execution-sdk-js ?
plugins-axios ?
plugins-azure-cosmos ?
plugins-azure-event-hubs ?
plugins-azure-service-bus ?
plugins-body-parser ?
plugins-bucket-0 53.18% <ø> (?)
plugins-bucket-1 55.11% <63.94%> (?)
plugins-bucket-11 56.74% <ø> (?)
plugins-bucket-17 56.81% <72.34%> (?)
plugins-bucket-18 57.61% <67.30%> (?)
plugins-bucket-19 57.09% <ø> (?)
plugins-bucket-20 59.39% <ø> (?)
plugins-bucket-4 53.86% <ø> (?)
plugins-bullmq_cassandra_cookie 57.41% <ø> (?)
plugins-cookie ?
plugins-cookie-parser_crypto_dd-trace-api 52.32% <ø> (?)
plugins-crypto ?
plugins-dd-trace-api ?
plugins-express-mongo-sanitize ?
plugins-express-session ?
plugins-fastify ?
plugins-fetch ?
plugins-fetch_fs_generic-pool 54.46% <29.32%> (?)
plugins-fs ?
plugins-generic-pool ?
plugins-google-cloud-pubsub ?
plugins-google-cloud-pubsub_grpc_handlebars 59.89% <ø> (?)
plugins-handlebars ?
plugins-hapi ?
plugins-hapi_hono_ioredis 55.87% <ø> (?)
plugins-hono ?
plugins-ioredis ?
plugins-jest ?
plugins-jest_knex_langgraph 51.66% <ø> (?)
plugins-knex ?
plugins-langgraph ?
plugins-ldapjs ?
plugins-ldapjs_light-my-request_limitd-client 53.42% <ø> (?)
plugins-light-my-request ?
plugins-limitd-client ?
plugins-lodash ?
plugins-lodash_mariadb_memcached 54.21% <ø> (?)
plugins-memcached ?
plugins-microgateway-core ?
plugins-modelcontextprotocol-sdk ?
plugins-moleculer ?
plugins-mongodb ?
plugins-mongodb-core ?
plugins-mongodb_mongodb-core_mongoose 55.47% <ø> (?)
plugins-mongoose ?
plugins-multer ?
plugins-multer_mysql_mysql2 54.38% <ø> (?)
plugins-mysql ?
plugins-mysql2 ?
plugins-nats ?
plugins-nats_node-serialize_opensearch 56.07% <ø> (?)
plugins-node-serialize ?
plugins-opensearch ?
plugins-passport-http ?
plugins-passport-http_pino_postgres 54.75% <ø> (?)
plugins-pino ?
plugins-postgres ?
plugins-process ?
plugins-process_pug_redis 53.88% <ø> (?)
plugins-pug ?
plugins-redis ?
plugins-router ?
plugins-test-and-upstream-amqp10 ?
plugins-test-and-upstream-amqplib ?
plugins-test-and-upstream-apollo ?
plugins-test-and-upstream-avsc ?
plugins-test-and-upstream-connect ?
plugins-test-and-upstream-graphql ?
plugins-test-and-upstream-protobufjs ?
plugins-test-and-upstream-rhea ?
plugins-undici ?
plugins-undici_url_valkey 54.38% <29.32%> (?)
plugins-url ?
plugins-valkey ?
plugins-vm ?
plugins-vm_winston_ws 55.28% <ø> (?)
plugins-winston ?
plugins-ws ?
profiling 58.20% <29.32%> (?)
profiling-macos ?
profiling-ubuntu ?
profiling-windows ?
serverless-aws-sdk-aws-sdk 50.66% <ø> (?)
serverless-aws-sdk-bedrockruntime 50.68% <ø> (?)
serverless-aws-sdk-client 51.96% <ø> (?)
serverless-aws-sdk-dynamodb 51.78% <ø> (?)
serverless-aws-sdk-eventbridge 46.30% <43.75%> (?)
serverless-aws-sdk-kinesis 54.71% <ø> (?)
serverless-aws-sdk-lambda 52.78% <ø> (?)
serverless-aws-sdk-latest-aws-sdk ?
serverless-aws-sdk-latest-client ?
serverless-aws-sdk-latest-dynamodb ?
serverless-aws-sdk-latest-eventbridge ?
serverless-aws-sdk-latest-kinesis ?
serverless-aws-sdk-latest-lambda ?
serverless-aws-sdk-latest-s3 ?
serverless-aws-sdk-latest-serverless-peer-service ?
serverless-aws-sdk-latest-sns ?
serverless-aws-sdk-latest-sqs ?
serverless-aws-sdk-latest-stepfunctions ?
serverless-aws-sdk-latest-util ?
serverless-aws-sdk-oldest-aws-sdk ?
serverless-aws-sdk-oldest-bedrockruntime ?
serverless-aws-sdk-oldest-client ?
serverless-aws-sdk-oldest-dynamodb ?
serverless-aws-sdk-oldest-eventbridge ?
serverless-aws-sdk-oldest-kinesis ?
serverless-aws-sdk-oldest-lambda ?
serverless-aws-sdk-oldest-s3 ?
serverless-aws-sdk-oldest-serverless-peer-service ?
serverless-aws-sdk-oldest-sns ?
serverless-aws-sdk-oldest-sqs ?
serverless-aws-sdk-oldest-util ?
serverless-aws-sdk-s3 51.52% <ø> (?)
serverless-aws-sdk-serverless-peer-service 54.73% <ø> (?)
serverless-aws-sdk-sns 55.48% <ø> (?)
serverless-aws-sdk-sqs 55.91% <ø> (?)
serverless-aws-sdk-stepfunctions 51.25% <ø> (?)
serverless-aws-sdk-util 48.45% <ø> (?)
serverless-azure-durable-functions ?
serverless-azure-functions-eventhubs ?
serverless-azure-functions-servicebus ?
serverless-bucket-0 55.24% <ø> (?)
serverless-bucket-1 56.02% <ø> (?)
serverless-lambda ?
test-optimization-cucumber 72.98% <68.75%> (?)
test-optimization-cucumber-latest-7.0.0 ?
test-optimization-cucumber-latest-latest ?
test-optimization-cucumber-oldest-7.0.0 ?
test-optimization-cypress 66.47% <68.75%> (?)
test-optimization-cypress-latest-12.0.0-commonJS ?
test-optimization-cypress-latest-12.0.0-esm ?
test-optimization-cypress-latest-14.5.4-commonJS ?
test-optimization-cypress-latest-14.5.4-esm ?
test-optimization-cypress-latest-latest-commonJS ?
test-optimization-cypress-latest-latest-esm ?
test-optimization-cypress-oldest-12.0.0-commonJS ?
test-optimization-cypress-oldest-12.0.0-esm ?
test-optimization-cypress-oldest-14.5.4-commonJS ?
test-optimization-cypress-oldest-14.5.4-esm ?
test-optimization-jest 73.88% <68.75%> (?)
test-optimization-jest-latest-latest ?
test-optimization-jest-latest-oldest ?
test-optimization-jest-oldest-latest ?
test-optimization-jest-oldest-oldest ?
test-optimization-mocha 74.29% <68.75%> (?)
test-optimization-mocha-latest-latest ?
test-optimization-mocha-latest-oldest ?
test-optimization-mocha-oldest-latest ?
test-optimization-mocha-oldest-oldest ?
test-optimization-playwright-latest-latest-playwright-active-test-span ?
test-optimization-playwright-latest-latest-playwright-atr ?
test-optimization-playwright-latest-latest-playwright-efd ?
test-optimization-playwright-latest-latest-playwright-final-status ?
test-optimization-playwright-latest-latest-playwright-impacted-tests ?
test-optimization-playwright-latest-latest-playwright-reporting ?
test-optimization-playwright-latest-latest-playwright-test-management ?
test-optimization-playwright-latest-oldest-playwright-active-test-span ?
test-optimization-playwright-latest-oldest-playwright-atr ?
test-optimization-playwright-latest-oldest-playwright-efd ?
test-optimization-playwright-latest-oldest-playwright-final-status ?
test-optimization-playwright-latest-oldest-playwright-impacted-tests ?
test-optimization-playwright-latest-oldest-playwright-reporting ?
test-optimization-playwright-oldest-latest-playwright-active-test-span ?
test-optimization-playwright-oldest-latest-playwright-atr ?
test-optimization-playwright-oldest-latest-playwright-efd ?
test-optimization-playwright-oldest-latest-playwright-impacted-tests ?
test-optimization-playwright-oldest-latest-playwright-reporting ?
test-optimization-playwright-oldest-latest-playwright-test-management ?
test-optimization-playwright-oldest-oldest-playwright-active-test-span ?
test-optimization-playwright-oldest-oldest-playwright-atr ?
test-optimization-playwright-oldest-oldest-playwright-efd ?
test-optimization-playwright-oldest-oldest-playwright-final-status ?
test-optimization-playwright-oldest-oldest-playwright-impacted-tests ?
test-optimization-playwright-oldest-oldest-playwright-reporting ?
test-optimization-playwright-oldest-oldest-playwright-test-management ?
test-optimization-playwright-playwright-atr 61.59% <68.75%> (?)
test-optimization-playwright-playwright-efd 61.78% <68.75%> (?)
test-optimization-playwright-playwright-final-status 61.74% <68.75%> (?)
test-optimization-playwright-playwright-impacted-tests 61.52% <68.75%> (?)
test-optimization-playwright-playwright-reporting 61.61% <68.75%> (?)
test-optimization-playwright-playwright-test-management 62.27% <68.75%> (?)
test-optimization-playwright-playwright-test-span 61.51% <68.75%> (?)
test-optimization-selenium 60.70% <68.75%> (?)
test-optimization-selenium-latest ?
test-optimization-selenium-oldest ?
test-optimization-testopt 59.15% <ø> (?)
test-optimization-testopt-active ?
test-optimization-testopt-latest ?
test-optimization-testopt-maintenance ?
test-optimization-testopt-oldest ?
test-optimization-vitest 72.73% <68.75%> (?)
test-optimization-vitest-latest ?
test-optimization-vitest-oldest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

BridgeAR added 2 commits June 26, 2026 00:01
express <4.3.0 has no `Layer.prototype.handle_request`/`handle_error`; the
router invokes `layer.handle` directly, so wrapping the prototype traced
nothing and dropped middleware, route-handler, and code-origin spans there.
Replace `layer.handle` in place (arity preserved so the host still routes error
handlers) when the layer exposes no dispatch method. express 4.3.0+, express 5,
the router package, and loopback keep `handle` pristine via the prototype wraps.

Also point the router-helper spec at the renamed `setLayerMeta` helper.
express-async-errors redefines `handle` as a getter/setter that stores the
wrapped handler in `__handle` and patches `handle` only, never `handle_request`.
On express 4.3.0+ the tracer's prototype-dispatch wrap therefore has to survive
that patch, the arity gate has to read the real 3-arg handler, and a rejected
async handler has to reach the wrapped `next` as `next(error)`. Pin that path so
a regression surfaces here instead of in a downstream tracer test.
@BridgeAR
BridgeAR marked this pull request as ready for review June 25, 2026 23:37
@BridgeAR
BridgeAR requested a review from a team as a code owner June 25, 2026 23:37
@BridgeAR
BridgeAR requested review from tlhunter and removed request for a team June 25, 2026 23:37

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c90dd1dd6c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/datadog-instrumentations/src/router.js
@BridgeAR
BridgeAR marked this pull request as draft June 25, 2026 23:54
…ress <4.6.0

On express <4.6.0 the router has no `Layer` prototype dispatch: it runs
`layer.handle` directly and, on a synchronous throw, catches outside the layer
and calls its own `next(error)` — never the wrapped `next` the tracer installed.
The prototype-dispatch rewrite dropped the legacy handle wrap's catch, so a
throwing middleware or route handler no longer published `middleware:error` /
`middleware:next` / `middleware:finish`: the span lost its error tag and lingered
on the stack until request finish.

Restore the catch in `wrapLegacyHandle` to publish error/next/finish and rethrow,
mirroring `wrapNext`. The prototype hosts (express >=4.6.0, express 5, router)
already route the throw through the wrapped next, so only the legacy path needs it.

Also correct the version boundary in the surrounding comments: `handle_request`
landed in express 4.6.0, not 4.3.0, so the legacy path covers `>=4.0.0 <4.6.0`.

Refs: #9067 (comment)
@BridgeAR
BridgeAR marked this pull request as ready for review June 26, 2026 12:51
szegedi
szegedi previously approved these changes Jul 2, 2026

@szegedi szegedi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Few notes surfaced by automated review:

  • async rejections now fire exit before error/next/finish, which is a bit of a behavior change, but I think the net effect is better as async middleware errors are now tagged on the middleware span.
  • a handler that calls next() and then rejects double-calls wrappedNext e.g. async (req, res, next) => { next(); await bg() } where bg() rejects. The explicit next() finishes and pops the middleware span; the host's rejection .then then calls wrappedNext(error) a second time, which tags error on the parent (context.span) since the middleware stack is now empty. The finish re-fire is a guarded no-op (no crash). Old code routed the rejection through the host's raw next, so this second tracer pass didn't happen. Pathological input, low impact (request-level error tagging is unchanged), but a real behavioral delta.

None of these I think need to be addressed, but it's not a bad idea to at least have a record of them in form of this review comment on the PR.

return { wrapLayerRequest, wrapLayerError, wrapLegacyHandle }
}

function hasLayerDispatch (layer) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agent probably missed the “All new methods should receive a full JSDoc comment.” instruction from AGENTS.md.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes... I want to add a lint rule at some point. I believe that is the only way to get this right

A handler that calls `next()` and then rejects (`async (req, res, next) => {
next(); await bg() }` where `bg()` rejects) makes the host invoke the same
wrapped `next` twice: the clean `next()` finishes and pops the middleware span,
then the host's rejection pass calls `next(error)` again. Since the middleware
stack is already empty, that second pass tagged the error on the parent span,
inventing a request-level error the middleware never surfaced.

Guard the wrapped continuation so its error/next/finish publishes fire at most
once per dispatch; the second call still forwards to the host chain untouched.

Add the missing JSDoc on `hasLayerDispatch`.
@BridgeAR

BridgeAR commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

@szegedi I added a guard against calling next twice. This is actually undefined behavior for express and similar and the former behavior used the last call, if I am not mistaken, which is actually wrong. Now we use the first one, that is correct.

I am going to open a follow-up PR that adds a tag to surface the information that next was called more than once.

@tlhunter
tlhunter merged commit 4935968 into master Jul 14, 2026
766 of 767 checks passed
@tlhunter
tlhunter deleted the BridgeAR/2026-06-25-express-router-prototype-dispatch branch July 14, 2026 18:57
This was referenced Jul 15, 2026
juan-fernandez pushed a commit that referenced this pull request Jul 16, 2026
* refactor(express): trace middleware via layer prototype dispatch

Middleware tracing replaced `layer.handle` with a wrapper and re-exposed the
user handler through an enumerable `_datadog_orig` back-reference, the only
property loopback's `_findLayerByHandler` could follow to tag a layer with its
phase. That ties the tracer to loopback's private reflection heuristic, where
the property name and its enumerability are both load-bearing.

Wrap the host `Layer` prototype dispatch (`handle_request`/`handle_error`, and
`handleRequest`/`handleError` on router >=2) and read per-layer metadata from a
WeakMap, leaving `layer.handle` the user's function. loopback's first lookup
(`layer.handle === handler`) then matches with no tracer-specific contract, so
the back-reference, the `express-async-errors` `__handle` branch, and the
`_name` write-back onto the user handler are no longer needed. A synchronous
throw or a rejected promise becomes `next(error)` in the host dispatch, so the
wrapped `next` captures both without a tracer-side try/catch.

Refs: #9062

* fix(express): trace middleware on express <4.3.0 (no prototype dispatch)

express <4.3.0 has no `Layer.prototype.handle_request`/`handle_error`; the
router invokes `layer.handle` directly, so wrapping the prototype traced
nothing and dropped middleware, route-handler, and code-origin spans there.
Replace `layer.handle` in place (arity preserved so the host still routes error
handlers) when the layer exposes no dispatch method. express 4.3.0+, express 5,
the router package, and loopback keep `handle` pristine via the prototype wraps.

Also point the router-helper spec at the renamed `setLayerMeta` helper.

* test(router): cover express-async-errors on prototype-dispatch hosts

express-async-errors redefines `handle` as a getter/setter that stores the
wrapped handler in `__handle` and patches `handle` only, never `handle_request`.
On express 4.3.0+ the tracer's prototype-dispatch wrap therefore has to survive
that patch, the arity gate has to read the real 3-arg handler, and a rejected
async handler has to reach the wrapped `next` as `next(error)`. Pin that path so
a regression surfaces here instead of in a downstream tracer test.

* fix(express): restore error publishing for throwing middleware on express <4.6.0

On express <4.6.0 the router has no `Layer` prototype dispatch: it runs
`layer.handle` directly and, on a synchronous throw, catches outside the layer
and calls its own `next(error)` — never the wrapped `next` the tracer installed.
The prototype-dispatch rewrite dropped the legacy handle wrap's catch, so a
throwing middleware or route handler no longer published `middleware:error` /
`middleware:next` / `middleware:finish`: the span lost its error tag and lingered
on the stack until request finish.

Restore the catch in `wrapLegacyHandle` to publish error/next/finish and rethrow,
mirroring `wrapNext`. The prototype hosts (express >=4.6.0, express 5, router)
already route the throw through the wrapped next, so only the legacy path needs it.

Also correct the version boundary in the surrounding comments: `handle_request`
landed in express 4.6.0, not 4.3.0, so the legacy path covers `>=4.0.0 <4.6.0`.

Refs: #9067 (comment)

* fix(express): publish middleware next/finish once per continuation

A handler that calls `next()` and then rejects (`async (req, res, next) => {
next(); await bg() }` where `bg()` rejects) makes the host invoke the same
wrapped `next` twice: the clean `next()` finishes and pops the middleware span,
then the host's rejection pass calls `next(error)` again. Since the middleware
stack is already empty, that second pass tagged the error on the parent span,
inventing a request-level error the middleware never surfaced.

Guard the wrapped continuation so its error/next/finish publishes fire at most
once per dispatch; the second call still forwards to the host chain untouched.

Add the missing JSDoc on `hasLayerDispatch`.
juan-fernandez pushed a commit that referenced this pull request Jul 16, 2026
* refactor(express): trace middleware via layer prototype dispatch

Middleware tracing replaced `layer.handle` with a wrapper and re-exposed the
user handler through an enumerable `_datadog_orig` back-reference, the only
property loopback's `_findLayerByHandler` could follow to tag a layer with its
phase. That ties the tracer to loopback's private reflection heuristic, where
the property name and its enumerability are both load-bearing.

Wrap the host `Layer` prototype dispatch (`handle_request`/`handle_error`, and
`handleRequest`/`handleError` on router >=2) and read per-layer metadata from a
WeakMap, leaving `layer.handle` the user's function. loopback's first lookup
(`layer.handle === handler`) then matches with no tracer-specific contract, so
the back-reference, the `express-async-errors` `__handle` branch, and the
`_name` write-back onto the user handler are no longer needed. A synchronous
throw or a rejected promise becomes `next(error)` in the host dispatch, so the
wrapped `next` captures both without a tracer-side try/catch.

Refs: #9062

* fix(express): trace middleware on express <4.3.0 (no prototype dispatch)

express <4.3.0 has no `Layer.prototype.handle_request`/`handle_error`; the
router invokes `layer.handle` directly, so wrapping the prototype traced
nothing and dropped middleware, route-handler, and code-origin spans there.
Replace `layer.handle` in place (arity preserved so the host still routes error
handlers) when the layer exposes no dispatch method. express 4.3.0+, express 5,
the router package, and loopback keep `handle` pristine via the prototype wraps.

Also point the router-helper spec at the renamed `setLayerMeta` helper.

* test(router): cover express-async-errors on prototype-dispatch hosts

express-async-errors redefines `handle` as a getter/setter that stores the
wrapped handler in `__handle` and patches `handle` only, never `handle_request`.
On express 4.3.0+ the tracer's prototype-dispatch wrap therefore has to survive
that patch, the arity gate has to read the real 3-arg handler, and a rejected
async handler has to reach the wrapped `next` as `next(error)`. Pin that path so
a regression surfaces here instead of in a downstream tracer test.

* fix(express): restore error publishing for throwing middleware on express <4.6.0

On express <4.6.0 the router has no `Layer` prototype dispatch: it runs
`layer.handle` directly and, on a synchronous throw, catches outside the layer
and calls its own `next(error)` — never the wrapped `next` the tracer installed.
The prototype-dispatch rewrite dropped the legacy handle wrap's catch, so a
throwing middleware or route handler no longer published `middleware:error` /
`middleware:next` / `middleware:finish`: the span lost its error tag and lingered
on the stack until request finish.

Restore the catch in `wrapLegacyHandle` to publish error/next/finish and rethrow,
mirroring `wrapNext`. The prototype hosts (express >=4.6.0, express 5, router)
already route the throw through the wrapped next, so only the legacy path needs it.

Also correct the version boundary in the surrounding comments: `handle_request`
landed in express 4.6.0, not 4.3.0, so the legacy path covers `>=4.0.0 <4.6.0`.

Refs: #9067 (comment)

* fix(express): publish middleware next/finish once per continuation

A handler that calls `next()` and then rejects (`async (req, res, next) => {
next(); await bg() }` where `bg()` rejects) makes the host invoke the same
wrapped `next` twice: the clean `next()` finishes and pops the middleware span,
then the host's rejection pass calls `next(error)` again. Since the middleware
stack is already empty, that second pass tagged the error on the parent span,
inventing a request-level error the middleware never surfaced.

Guard the wrapped continuation so its error/next/finish publishes fire at most
once per dispatch; the second call still forwards to the host chain untouched.

Add the missing JSDoc on `hasLayerDispatch`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants