Skip to content

feat(http2): trace core-API HTTP/2 servers#9138

Merged
tlhunter merged 7 commits into
masterfrom
BridgeAR/2026-06-29-http2-core-api-server
Jul 14, 2026
Merged

feat(http2): trace core-API HTTP/2 servers#9138
tlhunter merged 7 commits into
masterfrom
BridgeAR/2026-06-29-http2-core-api-server

Conversation

@BridgeAR

Copy link
Copy Markdown
Member

Summary

A server using the raw stream API (createServer().on('stream', ...)) produced no server span, because the instrumentation only hooked the compatibility request event. The stream event is now adapted into the minimal req/res shape the shared web lifecycle consumes, so the core path gets the same span, context propagation, and configured header tagging as the compatibility path.

A compatibility server emits both request and stream for every request, so the stream branch only creates a span when the server has no request listener; otherwise the request would be double-instrumented.

Test plan

  • should instrument the core stream API — a web.request span with the expected url/method/status for a core-API server.
  • produces exactly one server span permutations — createServer(handler), createServer().on('request'), and a server with both listeners each yield exactly one span (the gate that prevents double-instrumentation).
  • makes the core server span a child of the client span — distributed context extracted on the core path.
  • tags configured request and response headers from the stream — header config reads through the stream adapter.

Fixes: #312

A server using the raw stream API (`createServer().on('stream', ...)`) produced
no server span, because the instrumentation only hooked the compatibility
`request` event. This adapts the `Http2Stream` and its pseudo-header map into the
minimal req/res shape the shared web lifecycle consumes, so the core path now
gets the same span, context propagation, and header tagging as the compatibility
path.

A compatibility server emits both `request` and `stream` for every request, so
the `stream` branch only creates a span when the server has no `request`
listener; otherwise the request would be double-instrumented.

Fixes: #312
@dd-octo-sts

dd-octo-sts Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Overall package size

Self size: 6.72 MB
Deduped: 7.37 MB
No deduping: 7.37 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

@pr-commenter

pr-commenter Bot commented Jun 29, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-10 17:50:37

Comparing candidate commit 4f6c8d3 in PR branch BridgeAR/2026-06-29-http2-core-api-server with baseline commit d1aa54a in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 2248 metrics, 38 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-20

  • unstable execution_time [-191.943ms; +184.107ms] or [-5.138%; +4.929%]

scenario:appsec-appsec-enabled-24

  • unstable execution_time [-186.242ms; +183.298ms] or [-7.105%; +6.993%]

scenario:appsec-appsec-enabled-26

  • unstable execution_time [-208579.898µs; +209279.398µs] or [-8.324%; +8.352%]

scenario:appsec-appsec-enabled-with-attacks-26

  • unstable execution_time [-156.688ms; +151.645ms] or [-5.461%; +5.285%]

scenario:appsec-control-20

  • unstable execution_time [-160.662ms; +156.709ms] or [-9.174%; +8.949%]

scenario:appsec-control-24

  • unstable execution_time [-104.899ms; +101.591ms] or [-8.623%; +8.351%]

scenario:appsec-control-26

  • unstable execution_time [-112025.011µs; +113769.611µs] or [-9.291%; +9.435%]

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

  • unstable cpu_user_time [-1686.060ms; +561.745ms] or [-21.246%; +7.079%]
  • unstable execution_time [-1725.164ms; +577.152ms] or [-19.989%; +6.687%]
  • unstable instructions [-14.6G instructions; +4.8G instructions] or [-22.669%; +7.457%]
  • unstable throughput [-170.648op/s; +495.584op/s] or [-4.455%; +12.937%]

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

  • unstable cpu_user_time [-3243.642ms; +2912.158ms] or [-31.927%; +28.664%]
  • unstable execution_time [-3283.151ms; +2961.776ms] or [-30.252%; +27.291%]
  • unstable instructions [-28.1G instructions; +25.8G instructions] or [-33.119%; +30.424%]
  • unstable max_rss_usage [-10472.510KB; +8555.710KB] or [-6.488%; +5.301%]
  • unstable throughput [-642.410op/s; +683.308op/s] or [-20.650%; +21.964%]

scenario:debugger-line-probe-with-snapshot-minimal-24

  • unstable cpu_user_time [-2594.611ms; +3694.474ms] or [-28.826%; +41.045%]
  • unstable execution_time [-2636.742ms; +3746.073ms] or [-27.190%; +38.629%]
  • unstable instructions [-21.9G instructions; +31.5G instructions] or [-29.734%; +42.762%]
  • unstable max_rss_usage [-9.563MB; +13.603MB] or [-5.895%; +8.386%]
  • unstable throughput [-1107.970op/s; +785.918op/s] or [-31.508%; +22.350%]

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

  • unstable cpu_user_time [-1820.616ms; +639.846ms] or [-19.656%; +6.908%]
  • unstable execution_time [-1844.179ms; +654.225ms] or [-18.539%; +6.577%]
  • unstable instructions [-16.5G instructions; +5.7G instructions] or [-21.394%; +7.350%]
  • unstable throughput [-157.520op/s; +409.852op/s] or [-4.760%; +12.386%]

scenario:debugger-line-probe-without-snapshot-24

  • unstable cpu_user_time [-2563.091ms; +2582.745ms] or [-30.245%; +30.477%]
  • unstable execution_time [-2581.222ms; +2601.801ms] or [-28.189%; +28.413%]
  • unstable instructions [-22014.1M instructions; +22058.7M instructions] or [-31.913%; +31.977%]
  • unstable max_rss_usage [-9323.738KB; +9293.338KB] or [-5.823%; +5.804%]
  • unstable throughput [-771.991op/s; +763.573op/s] or [-21.050%; +20.820%]

scenario:debugger-line-probe-without-snapshot-26

  • unstable cpu_user_time [-1842.735ms; +599.751ms] or [-19.814%; +6.449%]
  • unstable execution_time [-1839.747ms; +604.539ms] or [-18.393%; +6.044%]
  • unstable instructions [-16.3G instructions; +5.4G instructions] or [-21.044%; +6.967%]
  • unstable throughput [-143.173op/s; +417.102op/s] or [-4.359%; +12.698%]

scenario:dogstatsd-with-tags-20

  • unstable cpu_user_time [-455.642ms; +222.981ms] or [-9.319%; +4.561%]
  • unstable execution_time [-456.868ms; +222.313ms] or [-9.207%; +4.480%]
  • unstable throughput [-76307.788op/s; +159610.957op/s] or [-4.512%; +9.438%]

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

  • unstable max_rss_usage [-34.153MB; +7.652MB] or [-8.846%; +1.982%]

@datadog-prod-us1-5

datadog-prod-us1-5 Bot commented Jun 29, 2026

Copy link
Copy Markdown

Tests

Fix all issues with BitsAI

⚠️ Warnings

🧪 1 Test failed in 1 job

Test Optimization | integration-playwright   GitHub Actions

[email protected] early flake detection keeps duration retry counts scoped by Playwright project from integration-tests/playwright/playwright-efd.spec.js   View in Datadog
Expected values to be strictly equal:

0 !== 3

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 96.54% (+8.53%)

Useful? React with 👍 / 👎

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

@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.58%. Comparing base (d1aa54a) to head (4f6c8d3).
⚠️ Report is 90 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9138      +/-   ##
==========================================
+ Coverage   93.69%   96.58%   +2.88%     
==========================================
  Files         889      918      +29     
  Lines       50864   121759   +70895     
  Branches    11834    20709    +8875     
==========================================
+ Hits        47657   117599   +69942     
- Misses       3207     4160     +953     
Flag Coverage Δ
aiguard 53.57% <89.47%> (+18.61%) ⬆️
aiguard-integration 57.01% <89.47%> (+15.22%) ⬆️
apm-bucket-0 53.80% <89.47%> (+18.87%) ⬆️
apm-bucket-1 58.37% <100.00%> (+17.95%) ⬆️
apm-bucket-2 56.12% <89.47%> (+18.70%) ⬆️
apm-capabilities-tracing 60.31% <54.38%> (+12.01%) ⬆️
apm-integrations-aerospike 52.95% <89.47%> (+19.74%) ⬆️
apm-integrations-confluentinc-kafka-javascript 57.07% <89.47%> (+16.96%) ⬆️
apm-integrations-couchbase 53.28% <89.47%> (+19.79%) ⬆️
apm-integrations-http 57.97% <89.47%> (+15.86%) ⬆️
apm-integrations-kafkajs 57.72% <89.47%> (+17.37%) ⬆️
apm-integrations-next 54.18% <41.00%> (+24.60%) ⬆️
apm-integrations-prisma 54.18% <89.47%> (+19.07%) ⬆️
apm-integrations-tedious 53.06% <89.47%> (+19.08%) ⬆️
appsec 68.86% <41.00%> (+11.49%) ⬆️
appsec-express_fastify_graphql 65.52% <41.00%> (+11.67%) ⬆️
appsec-integration 45.57% <38.53%> (+9.31%) ⬆️
appsec-kafka_ldapjs_lodash 59.10% <89.47%> (+15.43%) ⬆️
appsec-mongodb-core_mongoose_mysql 62.25% <89.47%> (+13.37%) ⬆️
appsec-next 52.81% <41.00%> (+24.73%) ⬆️
appsec-node-serialize_passport_postgres 61.88% <89.47%> (+13.82%) ⬆️
appsec-sourcing_stripe_template 60.24% <89.47%> (+14.62%) ⬆️
debugger 65.61% <41.00%> (+21.24%) ⬆️
instrumentations-bucket-0 48.76% <89.47%> (+20.53%) ⬆️
instrumentations-bucket-1 54.97% <89.47%> (+17.48%) ⬆️
instrumentations-bucket-10 56.63% <89.47%> (+16.13%) ⬆️
instrumentations-bucket-11 48.76% <89.47%> (+20.72%) ⬆️
instrumentations-bucket-12 48.86% <89.47%> (+20.11%) ⬆️
instrumentations-bucket-13 48.69% <89.47%> (+20.83%) ⬆️
instrumentations-bucket-2 49.75% <89.47%> (+19.44%) ⬆️
instrumentations-bucket-3 53.84% <89.47%> (+17.85%) ⬆️
instrumentations-bucket-4 49.17% <89.47%> (+21.02%) ⬆️
instrumentations-bucket-5 52.96% <41.00%> (+16.61%) ⬆️
instrumentations-bucket-6 55.61% <89.47%> (+17.28%) ⬆️
instrumentations-bucket-7 53.55% <41.00%> (+17.47%) ⬆️
instrumentations-bucket-8 54.64% <89.47%> (+17.61%) ⬆️
instrumentations-bucket-9 56.11% <89.47%> (+16.50%) ⬆️
instrumentations-instrumentation-couchbase 48.19% <89.47%> (+1.65%) ⬆️
instrumentations-integration-esbuild 33.98% <38.33%> (+9.11%) ⬆️
llmobs-ai_anthropic_bedrock 57.67% <41.00%> (+18.06%) ⬆️
llmobs-bucket-1 56.99% <89.47%> (?)
llmobs-google-genai_langchain_vertex-ai ?
llmobs-openai 57.57% <89.47%> (+17.91%) ⬆️
llmobs-sdk 59.96% <89.47%> (+16.27%) ⬆️
llmobs-vertex-ai 54.22% <89.47%> (?)
master-coverage 96.58% <100.00%> (?)
openfeature 54.62% <89.47%> (+16.86%) ⬆️
openfeature-unit 49.85% <89.47%> (-0.55%) ⬇️
platform-core_esbuild_instrumentations-misc 38.45% <38.33%> (+15.09%) ⬆️
platform-integration 62.15% <41.00%> (+14.72%) ⬆️
platform-shimmer_unit-guardrails_webpack 37.29% <38.33%> (+18.39%) ⬆️
plugins-bucket-0 53.13% <41.00%> (+16.73%) ⬆️
plugins-bucket-1 55.09% <89.47%> (+15.48%) ⬆️
plugins-bucket-11 55.54% <41.00%> (+17.00%) ⬆️
plugins-bucket-17 ?
plugins-bucket-18 57.08% <89.47%> (+15.00%) ⬆️
plugins-bucket-19 55.37% <89.47%> (+15.72%) ⬆️
plugins-bucket-20 57.45% <89.47%> (+14.11%) ⬆️
plugins-bucket-4 53.79% <41.00%> (+16.00%) ⬆️
plugins-bullmq_cassandra_cookie 57.41% <89.47%> (+17.56%) ⬆️
plugins-cookie-parser_crypto_dd-trace-api 52.33% <89.47%> (+19.03%) ⬆️
plugins-fetch_fs_generic-pool 54.47% <89.47%> (+18.34%) ⬆️
plugins-google-cloud-pubsub_grpc_handlebars 59.84% <50.00%> (+16.66%) ⬆️
plugins-hapi_hono_ioredis 55.80% <41.00%> (+17.93%) ⬆️
plugins-jest_knex_langgraph 51.68% <89.47%> (+18.99%) ⬆️
plugins-ldapjs_light-my-request_limitd-client 53.45% <89.47%> (+25.45%) ⬆️
plugins-lodash_mariadb_memcached 54.22% <89.47%> (+18.96%) ⬆️
plugins-moleculer_mongodb_mongodb-core 57.30% <89.47%> (?)
plugins-mongodb_mongodb-core_mongoose ?
plugins-mongoose_multer_mysql 54.72% <89.47%> (?)
plugins-multer_mysql_mysql2 ?
plugins-mysql2_nats_node-serialize 56.22% <89.47%> (?)
plugins-nats_node-serialize_opensearch ?
plugins-opensearch_passport-http_pino 55.07% <89.47%> (?)
plugins-passport-http_pino_postgres ?
plugins-postgres_process_pug 54.30% <89.47%> (?)
plugins-process_pug_redis ?
plugins-redis_router_sequelize 57.02% <89.47%> (?)
plugins-test-and-upstream-rhea_undici_url 56.94% <41.00%> (?)
plugins-undici_url_valkey ?
plugins-valkey_vm_winston 53.86% <89.47%> (?)
plugins-vm_winston_ws ?
plugins-ws 54.81% <89.47%> (?)
profiling 58.21% <89.47%> (+14.52%) ⬆️
serverless-aws-sdk-aws-sdk 50.63% <41.00%> (+17.26%) ⬆️
serverless-aws-sdk-bedrockruntime 50.65% <41.00%> (+18.41%) ⬆️
serverless-aws-sdk-client 51.98% <89.47%> (+14.77%) ⬆️
serverless-aws-sdk-dynamodb 51.71% <41.00%> (+17.54%) ⬆️
serverless-aws-sdk-eventbridge 46.26% <89.47%> (+18.94%) ⬆️
serverless-aws-sdk-kinesis 54.66% <41.00%> (+17.18%) ⬆️
serverless-aws-sdk-lambda 52.74% <41.00%> (+18.07%) ⬆️
serverless-aws-sdk-s3 51.49% <41.00%> (+18.82%) ⬆️
serverless-aws-sdk-serverless-peer-service 54.68% <41.00%> (+15.12%) ⬆️
serverless-aws-sdk-sns 55.43% <41.00%> (+17.10%) ⬆️
serverless-aws-sdk-sqs 55.92% <41.00%> (+17.84%) ⬆️
serverless-aws-sdk-stepfunctions 51.21% <41.00%> (+17.96%) ⬆️
serverless-aws-sdk-util 48.49% <89.47%> (+0.54%) ⬆️
serverless-bucket-0 55.16% <41.00%> (+15.69%) ⬆️
serverless-bucket-1 55.94% <41.00%> (?)
serverless-lambda ?
test-optimization-cucumber 72.98% <89.47%> (+20.51%) ⬆️
test-optimization-cypress 66.40% <89.47%> (+16.82%) ⬆️
test-optimization-jest 73.89% <89.47%> (+18.32%) ⬆️
test-optimization-mocha 74.30% <89.47%> (+20.71%) ⬆️
test-optimization-playwright-playwright-atr 61.50% <41.00%> (+17.99%) ⬆️
test-optimization-playwright-playwright-efd 61.90% <41.00%> (+18.11%) ⬆️
test-optimization-playwright-playwright-final-status 61.64% <41.00%> (+17.79%) ⬆️
test-optimization-playwright-playwright-impacted-tests 61.36% <41.00%> (+17.94%) ⬆️
test-optimization-playwright-playwright-reporting 61.27% <41.00%> (+17.85%) ⬆️
test-optimization-playwright-playwright-test-management 62.17% <41.00%> (+17.32%) ⬆️
test-optimization-playwright-playwright-test-span 61.41% <41.00%> (+16.70%) ⬆️
test-optimization-selenium 60.67% <89.47%> (+15.21%) ⬆️
test-optimization-testopt 59.10% <41.00%> (+10.90%) ⬆️
test-optimization-vitest 71.47% <41.00%> (+20.63%) ⬆️

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.

A core-API stream aborted before `stream.respond()` runs (client RST, server
`stream.close(code)`, a throwing handler) has an empty `stream.sentHeaders`, so
the adapter reported `res.statusCode` as `undefined`. `web.js` then ran
`validateStatus(undefined)`, which is falsy, tagged the span as an error, and
dropped the `http.status_code` tag — diverging from the compatibility path,
whose `Http2ServerResponse.statusCode` defaults to 200 for the same abort. The
adapter now falls back to 200 so both paths agree.

Drive-by fix:

* Pin the stream-adapter req/res shape with a `@typedef` naming the `web.js` /
  `url.js` / `ip_extractor.js` fields it must satisfy, so a new read added there
  fails the type check instead of silently resolving to `undefined` on the core
  path only.
* Record at `bindEmit` that finish is single-sourced from the stream's one
  `close` event, which is why the `!req.stream` idempotency-guard bypass in
  `web.js` is harmless here.

Fixes: #312
@BridgeAR
BridgeAR marked this pull request as ready for review June 30, 2026 16:51
@BridgeAR
BridgeAR requested a review from a team as a code owner June 30, 2026 16:51
@BridgeAR
BridgeAR requested review from wconti27 and removed request for a team June 30, 2026 16:51

@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: ac52b7ff9a

ℹ️ 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/http2/server.js Outdated
Comment thread packages/datadog-plugin-http2/test/server.spec.js Outdated
@BridgeAR
BridgeAR marked this pull request as draft June 30, 2026 18:42
A server that registers both a raw `stream` listener and a compatibility
`request` listener emits a single `stream` event whose internal listener
synthesizes the `request`. The previous gate created the span only on the
synthesized `request`, which fires nested in one stream listener, so the
application's own `stream` listener ran with no active span and any child
spans or handler duration it produced were lost.

1. Such mixed servers now create the span from the `stream` event, keeping
   it active across the application's stream listener; the synthesized
   `request` reuses it instead of creating a second span. A request-only
   server still traces on `request` so the compatibility response keeps its
   richer req/res, and the per-request cost stays a single `WeakSet.has`.
2. The `with configured headers` test reconfigured the plugin with a second
   `agent.load` nested under the suite that already loaded it, starting a
   second mock agent and leaking the first server's handle; it now reloads
   in place so a single agent runs.

Drive-by fix:

* Correct the stream-adapter `@typedef`: `res.statusCode` is the numeric
  `:status` pseudo-header, and `res.getHeader` can return a string array.
@BridgeAR
BridgeAR marked this pull request as ready for review July 1, 2026 15:21

@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: aebbb6938a

ℹ️ 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/http2/server.js Outdated
bengl
bengl previously approved these changes Jul 8, 2026
BridgeAR added 2 commits July 9, 2026 18:24
…channel

A server with both a raw `stream` listener and a compatibility `request`
listener creates its span from the `stream` event using a throwaway adapter.
The synthesized `request` off the same stream then found no context, so a
user's `request` handler calling `web.setRoute`/`web.setFramework` never
reached the span and the finish `hooks.request` received the adapter instead
of the real `Http2ServerRequest`/`Http2ServerResponse`.

The instrumentation now publishes `apm:http2:server:request:adopt` with the
real req/res, and the plugin points the stream-backed context at them. The
join key is the shared `Http2Stream`: `web.linkContextToStream` keys the
context on the stream so the second request resolves to the first's span. That
key is written only for mixed servers (`ctx.adoptable`), so the common
single-listener request pays no extra per-request map write.
@grpc/grpc-js builds its transport on a core Node `Http2Server` and registers
a raw `stream` listener with no `request` listener. The re-enabled http2
server instrumentation traces exactly that shape, so every gRPC call gained a
`web.request` span on top of its `grpc.server` span, and because the stream
event fires before gRPC's handler, `web.request` became the top frame.

The gRPC instrumentation now marks the server it owns and the http2
instrumentation skips any marked server, so a gRPC call keeps a single span
with gRPC as the top frame. Suppression is unconditional: disabling gRPC
tracing does not resurrect these as `web.request` spans, since a gRPC call over
HTTP/2 has no meaningful HTTP request/response semantics (`:path` is
`/pkg.Svc/Method`, status is always 200 + trailers) and surfacing it as one
would surprise a user who turned gRPC off.

The mark is a module-local `Symbol` on the server instance, read once per emit
behind the existing `hasSubscribers` gate (~1.6 ns; a WeakSet lookup measured
~3.5x slower and needs a shared mutable container across the two
instrumentations). It is set from `_setupHandlers`, the single server-creation
funnel present across the supported range — the newer `createHttp2Server` does
not exist on older versions — and it runs at bind time, before any request
reaches the wrapped `emit`.
BridgeAR added 2 commits July 10, 2026 17:44
Linux runners resolve localhost to ::1, but the gRPC fixture listens on IPv4, so the probe failed before reaching the server.
A synchronous throw from a core stream handler terminates the process, so exercise it in a forked fixture and let NYC collect the child coverage instead of excluding the error path.
@tlhunter
tlhunter merged commit 416cebe into master Jul 14, 2026
700 of 702 checks passed
@tlhunter
tlhunter deleted the BridgeAR/2026-06-29-http2-core-api-server branch July 14, 2026 19:44
This was referenced Jul 15, 2026
juan-fernandez pushed a commit that referenced this pull request Jul 16, 2026
* feat(http2): trace core-API HTTP/2 servers

A server using the raw stream API (`createServer().on('stream', ...)`) produced
no server span, because the instrumentation only hooked the compatibility
`request` event. This adapts the `Http2Stream` and its pseudo-header map into the
minimal req/res shape the shared web lifecycle consumes, so the core path now
gets the same span, context propagation, and header tagging as the compatibility
path.

A compatibility server emits both `request` and `stream` for every request, so
the `stream` branch only creates a span when the server has no `request`
listener; otherwise the request would be double-instrumented.

Fixes: #312

* fix(http2): default core-stream status to 200 when no response was sent

A core-API stream aborted before `stream.respond()` runs (client RST, server
`stream.close(code)`, a throwing handler) has an empty `stream.sentHeaders`, so
the adapter reported `res.statusCode` as `undefined`. `web.js` then ran
`validateStatus(undefined)`, which is falsy, tagged the span as an error, and
dropped the `http.status_code` tag — diverging from the compatibility path,
whose `Http2ServerResponse.statusCode` defaults to 200 for the same abort. The
adapter now falls back to 200 so both paths agree.

Drive-by fix:

* Pin the stream-adapter req/res shape with a `@typedef` naming the `web.js` /
  `url.js` / `ip_extractor.js` fields it must satisfy, so a new read added there
  fails the type check instead of silently resolving to `undefined` on the core
  path only.
* Record at `bindEmit` that finish is single-sourced from the stream's one
  `close` event, which is why the `!req.stream` idempotency-guard bypass in
  `web.js` is harmless here.

Fixes: #312

* fix(http2): keep the server span active for mixed stream/request servers

A server that registers both a raw `stream` listener and a compatibility
`request` listener emits a single `stream` event whose internal listener
synthesizes the `request`. The previous gate created the span only on the
synthesized `request`, which fires nested in one stream listener, so the
application's own `stream` listener ran with no active span and any child
spans or handler duration it produced were lost.

1. Such mixed servers now create the span from the `stream` event, keeping
   it active across the application's stream listener; the synthesized
   `request` reuses it instead of creating a second span. A request-only
   server still traces on `request` so the compatibility response keeps its
   richer req/res, and the per-request cost stays a single `WeakSet.has`.
2. The `with configured headers` test reconfigured the plugin with a second
   `agent.load` nested under the suite that already loaded it, starting a
   second mock agent and leaking the first server's handle; it now reloads
   in place so a single agent runs.

Drive-by fix:

* Correct the stream-adapter `@typedef`: `res.statusCode` is the numeric
  `:status` pseudo-header, and `res.getHeader` can return a string array.

* fix(http2): hand mixed-server requests the real req/res via an adopt channel

A server with both a raw `stream` listener and a compatibility `request`
listener creates its span from the `stream` event using a throwaway adapter.
The synthesized `request` off the same stream then found no context, so a
user's `request` handler calling `web.setRoute`/`web.setFramework` never
reached the span and the finish `hooks.request` received the adapter instead
of the real `Http2ServerRequest`/`Http2ServerResponse`.

The instrumentation now publishes `apm:http2:server:request:adopt` with the
real req/res, and the plugin points the stream-backed context at them. The
join key is the shared `Http2Stream`: `web.linkContextToStream` keys the
context on the stream so the second request resolves to the first's span. That
key is written only for mixed servers (`ctx.adoptable`), so the common
single-listener request pays no extra per-request map write.

* fix(http2): do not trace gRPC-owned HTTP/2 servers

@grpc/grpc-js builds its transport on a core Node `Http2Server` and registers
a raw `stream` listener with no `request` listener. The re-enabled http2
server instrumentation traces exactly that shape, so every gRPC call gained a
`web.request` span on top of its `grpc.server` span, and because the stream
event fires before gRPC's handler, `web.request` became the top frame.

The gRPC instrumentation now marks the server it owns and the http2
instrumentation skips any marked server, so a gRPC call keeps a single span
with gRPC as the top frame. Suppression is unconditional: disabling gRPC
tracing does not resurrect these as `web.request` spans, since a gRPC call over
HTTP/2 has no meaningful HTTP request/response semantics (`:path` is
`/pkg.Svc/Method`, status is always 200 + trailers) and surfacing it as one
would surprise a user who turned gRPC off.

The mark is a module-local `Symbol` on the server instance, read once per emit
behind the existing `hasSubscribers` gate (~1.6 ns; a WeakSet lookup measured
~3.5x slower and needs a shared mutable container across the two
instrumentations). It is set from `_setupHandlers`, the single server-creation
funnel present across the supported range — the newer `createHttp2Server` does
not exist on older versions — and it runs at bind time, before any request
reaches the wrapped `emit`.

* test(grpc): connect non-gRPC probe over IPv4

Linux runners resolve localhost to ::1, but the gRPC fixture listens on IPv4, so the probe failed before reaching the server.

* test(http2): cover stream handler crash path

A synchronous throw from a core stream handler terminates the process, so exercise it in a forked fixture and let NYC collect the child coverage instead of excluding the error path.
juan-fernandez pushed a commit that referenced this pull request Jul 16, 2026
* feat(http2): trace core-API HTTP/2 servers

A server using the raw stream API (`createServer().on('stream', ...)`) produced
no server span, because the instrumentation only hooked the compatibility
`request` event. This adapts the `Http2Stream` and its pseudo-header map into the
minimal req/res shape the shared web lifecycle consumes, so the core path now
gets the same span, context propagation, and header tagging as the compatibility
path.

A compatibility server emits both `request` and `stream` for every request, so
the `stream` branch only creates a span when the server has no `request`
listener; otherwise the request would be double-instrumented.

Fixes: #312

* fix(http2): default core-stream status to 200 when no response was sent

A core-API stream aborted before `stream.respond()` runs (client RST, server
`stream.close(code)`, a throwing handler) has an empty `stream.sentHeaders`, so
the adapter reported `res.statusCode` as `undefined`. `web.js` then ran
`validateStatus(undefined)`, which is falsy, tagged the span as an error, and
dropped the `http.status_code` tag — diverging from the compatibility path,
whose `Http2ServerResponse.statusCode` defaults to 200 for the same abort. The
adapter now falls back to 200 so both paths agree.

Drive-by fix:

* Pin the stream-adapter req/res shape with a `@typedef` naming the `web.js` /
  `url.js` / `ip_extractor.js` fields it must satisfy, so a new read added there
  fails the type check instead of silently resolving to `undefined` on the core
  path only.
* Record at `bindEmit` that finish is single-sourced from the stream's one
  `close` event, which is why the `!req.stream` idempotency-guard bypass in
  `web.js` is harmless here.

Fixes: #312

* fix(http2): keep the server span active for mixed stream/request servers

A server that registers both a raw `stream` listener and a compatibility
`request` listener emits a single `stream` event whose internal listener
synthesizes the `request`. The previous gate created the span only on the
synthesized `request`, which fires nested in one stream listener, so the
application's own `stream` listener ran with no active span and any child
spans or handler duration it produced were lost.

1. Such mixed servers now create the span from the `stream` event, keeping
   it active across the application's stream listener; the synthesized
   `request` reuses it instead of creating a second span. A request-only
   server still traces on `request` so the compatibility response keeps its
   richer req/res, and the per-request cost stays a single `WeakSet.has`.
2. The `with configured headers` test reconfigured the plugin with a second
   `agent.load` nested under the suite that already loaded it, starting a
   second mock agent and leaking the first server's handle; it now reloads
   in place so a single agent runs.

Drive-by fix:

* Correct the stream-adapter `@typedef`: `res.statusCode` is the numeric
  `:status` pseudo-header, and `res.getHeader` can return a string array.

* fix(http2): hand mixed-server requests the real req/res via an adopt channel

A server with both a raw `stream` listener and a compatibility `request`
listener creates its span from the `stream` event using a throwaway adapter.
The synthesized `request` off the same stream then found no context, so a
user's `request` handler calling `web.setRoute`/`web.setFramework` never
reached the span and the finish `hooks.request` received the adapter instead
of the real `Http2ServerRequest`/`Http2ServerResponse`.

The instrumentation now publishes `apm:http2:server:request:adopt` with the
real req/res, and the plugin points the stream-backed context at them. The
join key is the shared `Http2Stream`: `web.linkContextToStream` keys the
context on the stream so the second request resolves to the first's span. That
key is written only for mixed servers (`ctx.adoptable`), so the common
single-listener request pays no extra per-request map write.

* fix(http2): do not trace gRPC-owned HTTP/2 servers

@grpc/grpc-js builds its transport on a core Node `Http2Server` and registers
a raw `stream` listener with no `request` listener. The re-enabled http2
server instrumentation traces exactly that shape, so every gRPC call gained a
`web.request` span on top of its `grpc.server` span, and because the stream
event fires before gRPC's handler, `web.request` became the top frame.

The gRPC instrumentation now marks the server it owns and the http2
instrumentation skips any marked server, so a gRPC call keeps a single span
with gRPC as the top frame. Suppression is unconditional: disabling gRPC
tracing does not resurrect these as `web.request` spans, since a gRPC call over
HTTP/2 has no meaningful HTTP request/response semantics (`:path` is
`/pkg.Svc/Method`, status is always 200 + trailers) and surfacing it as one
would surprise a user who turned gRPC off.

The mark is a module-local `Symbol` on the server instance, read once per emit
behind the existing `hasSubscribers` gate (~1.6 ns; a WeakSet lookup measured
~3.5x slower and needs a shared mutable container across the two
instrumentations). It is set from `_setupHandlers`, the single server-creation
funnel present across the supported range — the newer `createHttp2Server` does
not exist on older versions — and it runs at bind time, before any request
reaches the wrapped `emit`.

* test(grpc): connect non-gRPC probe over IPv4

Linux runners resolve localhost to ::1, but the gRPC fixture listens on IPv4, so the probe failed before reaching the server.

* test(http2): cover stream handler crash path

A synchronous throw from a core stream handler terminates the process, so exercise it in a forked fixture and let NYC collect the child coverage instead of excluding the error path.
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.

Add support for HTTP2

3 participants