Skip to content

refactor(postgres): remove identity hooks - #10649

Merged
pabloerhard merged 1 commit into
masterfrom
pabloerhard/orchestrion-decouple
Oct 6, 2026
Merged

pabloerhard merged 1 commit into
masterfrom
pabloerhard/orchestrion-decouple

Conversation

@pabloerhard

Copy link
Copy Markdown
Contributor

What does this PR do?

Moves the Postgres.js (postgres) integration off the RITM/IITM identity hooks so it activates only through the Orchestrion rewrite protocol, matching azure-cosmos, bullmq, and supabase:

  • Deletes the identity-hook stub packages/datadog-instrumentations/src/postgres.js.
  • Removes the postgres entry from helpers/hooks.js.
  • Sets activate: true on the postgres entry in helpers/rewriter/instrumentation-registry.js, so evaluating a rewritten Postgres.js file publishes the load that enables the plugin.
  • Adds postgres to the hookless Orchestrion plugin lists in plugin-structure.spec.js and to the assertion that pure Orchestrion integrations are registered only as rewrite targets.

Motivation

Orchestrion can now activate integrations without a module hook. Postgres.js is a pure Orchestrion integration, so the identity hook only adds loader overhead and a second activation path.

Additional Notes

  • targets.json is unchanged when regenerated, and the generated supported-versions output still lists postgres 3.0.0 – 3.4.9, because rewriter-only ranges are already included.
  • Tests run locally: PLUGINS=postgres npm run test:plugins against Postgres 9.5 (85 passing, covering 3.0.0 and 3.4.9, ESM variants, CJS, and the preloaded query.js partial-rewrite case), plus the plugin-structure, supported-integrations generator, loader-hook, plugin versions, rewriter/instrumentation helper, turbopack, and installer specs.

Postgres.js is a pure Orchestrion integration, so activate it through the
rewrite protocol instead of RITM/IITM identity hooks, matching
azure-cosmos, bullmq, and supabase. Drop the hooks.js entry and the
identity-hook stub, and enable rewrite activation for the postgres
registry entry.

Rewriter targets and the generated supported-versions output are
unchanged.
@dd-octo-sts

dd-octo-sts Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Overall package size

Self size: 9.44 MB
Deduped: 10.2 MB
No deduping: 10.2 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.5.1 | 127.66 kB | 531.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-datadog-us1-prod

datadog-datadog-us1-prod Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Tests

✅ All CI checks and tests passed. Datadog automation helped this PR pass.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🔄 Datadog retried 2 tests - 2 passed on retry View in Datadog

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 98.28% (+0.02%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 22a8d30 | Docs | View more details | Give us feedback!

@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.87%. Comparing base (32ab2ff) to head (22a8d30).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10649      +/-   ##
==========================================
- Coverage   98.87%   98.87%   -0.01%     
==========================================
  Files        1059     1058       -1     
  Lines      171895   171887       -8     
  Branches       74       74              
==========================================
- Hits       169957   169949       -8     
  Misses       1938     1938              
Flag Coverage Δ
ai-guard 63.75% <100.00%> (-0.02%) ⬇️
apm-capabilities 54.72% <100.00%> (-0.01%) ⬇️
apm-integrations 80.98% <100.00%> (-0.02%) ⬇️
appsec 57.88% <100.00%> (-0.02%) ⬇️
debugger 69.87% <100.00%> (-0.03%) ⬇️
instrumentation 55.36% <100.00%> (-0.01%) ⬇️
llmobs 80.70% <100.00%> (-0.02%) ⬇️
master-coverage 98.87% <100.00%> (?)
openfeature 67.33% <100.00%> (-0.03%) ⬇️
platform 68.57% <100.00%> (-0.01%) ⬇️
profiling 65.98% <100.00%> (-0.03%) ⬇️
serverless 65.59% <100.00%> (-0.02%) ⬇️
test-optimization 81.85% <100.00%> (+0.03%) ⬆️

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.

@pabloerhard
pabloerhard marked this pull request as ready for review October 6, 2026 18:01
@pabloerhard
pabloerhard requested review from a team as code owners October 6, 2026 18:01
@pabloerhard
pabloerhard requested review from wconti27 and removed request for a team October 6, 2026 18:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T18:04:36.793928Z 22a8d30 Draft marked ready
🔒 Security Review ✅ Completed 2026-10-06T18:05:17.923136Z 22a8d30 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@wconti27 wconti27 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.

Looks good, would you want to do a bulk transition for the remainders considering this PR is so easy to review?

@pabloerhard

Copy link
Copy Markdown
Contributor Author

Looks good, would you want to do a bulk transition for the remainders considering this PR is so easy to review?

@wconti27 I think this is the last easy flip one, any other is either a hybrid instrumentation or use a different activation path (for example test opt)

@pr-commenter

pr-commenter Bot commented Oct 6, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-10-06 18:11:57

Comparing candidate commit 22a8d30 in PR branch pabloerhard/orchestrion-decouple with baseline commit 32ab2ff in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 2387 metrics, 16 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-iast-no-vulnerability-iast-enabled-default-config-20

  • unstable max_rss_usage [-14260.868KB; +15400.868KB] or [-6.294%; +6.798%]

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

  • unstable cpu_user_time [-402.440ms; +576.055ms] or [-4.580%; +6.556%]
  • unstable execution_time [-418.522ms; +633.922ms] or [-4.267%; +6.463%]
  • unstable throughput [-52.209op/s; +34.004op/s] or [-6.376%; +4.153%]

scenario:encoders-0.4-immediate-flush-20

  • unstable max_rss_usage [-12.488MB; +7.933MB] or [-14.724%; +9.354%]

scenario:llmobs-encode-unicode-ascii-24

  • unstable cpu_usage_percentage [-2.857%; +8.633%]
  • unstable execution_time [-135.635ms; +44.661ms] or [-12.173%; +4.008%]
  • unstable throughput [-677.745op/s; +2015.059op/s] or [-3.089%; +9.184%]

scenario:llmobs-span-processor-retrieval-20

  • unstable cpu_usage_percentage [-5.926%; +4.960%]
  • unstable execution_time [-231.108ms; +262.667ms] or [-7.682%; +8.731%]
  • unstable throughput [-41382.065op/s; +39549.395op/s] or [-6.167%; +5.894%]

scenario:openfeature-scale-full-24

  • unstable max_rss_usage [-38.395MB; +28.582MB] or [-13.784%; +10.261%]

scenario:openfeature-typical-full-20

  • unstable max_rss_usage [-5.745MB; +10.851MB] or [-3.936%; +7.435%]

scenario:plugin-aws-sdk-eventbridge-inject-detail-24

  • unstable execution_time [-199.747ms; +303.441ms] or [-4.136%; +6.282%]

scenario:plugin-memcached-hashring-24

  • unstable execution_time [-159.137ms; +178.446ms] or [-6.143%; +6.888%]
  • unstable throughput [-823231.386op/s; +781197.600op/s] or [-5.566%; +5.281%]

@wconti27

wconti27 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Looks good, would you want to do a bulk transition for the remainders considering this PR is so easy to review?

@wconti27 I think this is the last easy flip one, any other is either a hybrid instrumentation or use a different activation path (for example test opt)

@pabloerhard Ah okay, thanks for the info and the changes here!

@pabloerhard

Copy link
Copy Markdown
Contributor Author

Looks good, would you want to do a bulk transition for the remainders considering this PR is so easy to review?

@wconti27 I think this is the last easy flip one, any other is either a hybrid instrumentation or use a different activation path (for example test opt)

@pabloerhard Ah okay, thanks for the info and the changes here!

@wconti27 That said, I'll look into what the pending non hybrid instrumentations (the ones that subscribe through other means) need to be hookless, if simple enough I might be opening a batch of PR's for that.

@pabloerhard
pabloerhard merged commit a505931 into master Oct 6, 2026
723 of 725 checks passed
@pabloerhard
pabloerhard deleted the pabloerhard/orchestrion-decouple branch October 6, 2026 18:30
dd-octo-sts Bot pushed a commit that referenced this pull request Oct 6, 2026
Postgres.js is a pure Orchestrion integration, so activate it through the
rewrite protocol instead of RITM/IITM identity hooks, matching
azure-cosmos, bullmq, and supabase. Drop the hooks.js entry and the
identity-hook stub, and enable rewrite activation for the postgres
registry entry.

Rewriter targets and the generated supported-versions output are
unchanged.
dd-octo-sts Bot pushed a commit that referenced this pull request Oct 6, 2026
Postgres.js is a pure Orchestrion integration, so activate it through the
rewrite protocol instead of RITM/IITM identity hooks, matching
azure-cosmos, bullmq, and supabase. Drop the hooks.js entry and the
identity-hook stub, and enable rewrite activation for the postgres
registry entry.

Rewriter targets and the generated supported-versions output are
unchanged.
This was referenced Oct 6, 2026
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.

2 participants