✨ Add constructor support to instrumentMethod by bdibon · Pull Request #4714 · DataDog/browser-sdk

bdibon · 2026-06-03T11:35:00Z

Motivation

The WebSocket instrumentation added higher in this stack needs to wrap the WebSocket constructor, but instrumentMethod only supported instrumenting regular methods.

Changes

Add support for instrumenting constructors to instrumentMethod: handle new.target, call the original via Reflect.construct, and preserve the original prototype so instanceof checks against the instrumented global keep working.
Introduce MethodParameters / MethodReturnType type helpers that resolve the parameters/return type from either a call or a construct signature.

Test instructions

yarn test:unit --spec packages/core/src/tools/instrumentMethod.spec.ts

Checklist

Tested locally
Tested on staging
Added unit tests for this change.
Added e2e/integration tests for this change.
Updated documentation and/or relevant AGENTS.md file

bdibon · 2026-06-03T11:35:22Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

cit-pr-commenter-54b7da · 2026-06-03T11:37:21Z

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	171.78 KiB	171.88 KiB	+101 B	+0.06%	✅
Rum Profiler	7.88 KiB	7.88 KiB	0 B	0.00%	✅
Rum Recorder	21.21 KiB	21.22 KiB	+9 B	+0.04%	✅
Logs	54.30 KiB	54.39 KiB	+89 B	+0.16%	✅
Rum Slim	129.68 KiB	129.77 KiB	+89 B	+0.07%	✅
Worker	22.96 KiB	22.96 KiB	0 B	0.00%	✅

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c445e3c783

ℹ️ 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".

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

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

datadog-official · 2026-06-03T11:38:32Z

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 76.74%
• Overall Coverage: 76.69% (-0.01%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 0f0b7a0 | Docs | Datadog PR Page | Give us feedback!}

Co-authored-by: Cursor <cursoragent@cursor.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2bdea2f62

ℹ️ 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".

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

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

bcaudan · 2026-06-04T12:12:23Z

 import { createHandlingStack } from './stackTrace/handlingStack'

+/* Resolves parameters from either a call or construct signature, so instrumentMethod can handle
+ * both regular methods and constructors (e.g. `new WebSocket()`).


🥜 nitpick: ‏What about replacing WebSocket references by something like MyClass?
No need to reference the usages in utility functions.

bcaudan · 2026-06-04T12:20:13Z

+  const instrumentation = function (this: TARGET): MethodReturnType<TARGET[METHOD]> {
    if (stopped) {
+      if (new.target) {
+        return Reflect.construct(original, arguments as unknown as MethodParameters<TARGET[METHOD]>)


❓ question: ‏the other construct call uses a third parameter new.target, should it be used in this call as well?

No, this is called when the instrumentation has been stopped, but the consumer might still have a reference to it, then we want to behave as if we're not here.

bcaudan · 2026-06-04T12:22:35Z

   * Note: if needed, parameters can be mutated by the instrumentation
   */
-  parameters: Parameters<TARGET[METHOD]>
+  parameters: MethodParameters<TARGET[METHOD]>


❓ question: ‏the TARGET[METHOD] type or even the target property do not really make sense in a constructor context, no?
Could it be worth considering an instrumentConstructor function instead with some shared helpers?

They don't in a regular constructor context, but all the constructor we're likely to instrument will be instrumented on the globalObject (window.WebSocket for example).

I will try to make this split and see if it reads better.

bcaudan · 2026-06-04T12:27:27Z

+  if (typeof original === 'function' && original.prototype) {
+    instrumentation.prototype = original.prototype
+  }
+
+  // Preserve static members (class fields, static methods, etc.) on the instrumented function.
+  if (typeof original === 'function') {


🥜 nitpick: ‏what about moving the prototype logic inside the other function guard below?

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b52da5151

ℹ️ 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".

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

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-04T15:06:14Z

+
+  // Preserve static members (class fields, static methods, etc.) on the instrumented function.
+  const excludedKeys = new Set(['prototype', 'length', 'name', 'arguments', 'caller'])
+  for (const key of Object.getOwnPropertyNames(original)) {


Preserve inherited constructor statics

issue: This only copies own static descriptors, but ES class constructors inherit static APIs through their constructor prototype chain (class Child extends Parent). When such a constructor is instrumented, static methods or constants that previously resolved from Parent become undefined because the wrapper still inherits from Function.prototype; preserve the original constructor's prototype chain (or otherwise delegate inherited statics) in addition to copying own descriptors.

Useful? React with 👍 / 👎.

Good point, but the usage of instrumentConstructor is the instrumentation of WebSocket and it doesn't have any inherited static members.

Plus the fix would be to use Object.setPrototypeOf which might degrade performance.

I've chosen to document the limitation.

…d method after stop

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f0b7a0fda

ℹ️ 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".

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

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-04T15:41:02Z

+
+  // Preserve the original prototype so that instanceof checks against the instrumented global work.
+  // e.g. `new MyClass() instanceof MyClass` must remain true.
+  instrumentation.prototype = original.prototype


Preserve the prototype constructor link

issue: When this wraps a global constructor such as WebSocket, the wrapper reuses the original prototype object but leaves that prototype's constructor property pointing at the pre-instrumentation function. While the SDK is active, code that checks new WebSocket(url).constructor === WebSocket now gets false because WebSocket is the wrapper and the instance inherits constructor from the original prototype, which is a backwards-compatibility break for constructor identity checks.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-04T15:41:02Z

+      return notifyInstrumentation<InstrumentedConstructorCall<TARGET[NAME]>, ConstructorInstanceOf<TARGET[NAME]>>(
+        onPreCall,
+        {
+          parameters,
+          handlingStack: computeHandlingStack ? createHandlingStack('instrumented method') : undefined,


Avoid instrumenting failed bare constructor calls

issue: If the instrumented constructor is invoked without new (for example WebSocket(url) or a class constructor called as a function), the original would throw before any constructor work happens, but this wrapper calls onPreCall first and only fails later because new.target is undefined. That means the SDK can record or mutate a constructor attempt that never actually reaches the original constructor; preserve the original call semantics for bare calls before notifying instrumentation.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-04T15:41:02Z

+
+  // Preserve the original prototype so that instanceof checks against the instrumented global work.
+  // e.g. `new MyClass() instanceof MyClass` must remain true.
+  instrumentation.prototype = original.prototype


Preserve the prototype property's descriptor

issue: Assigning instrumentation.prototype copies only the value, leaving the wrapper's own prototype descriptor writable even when the original constructor's descriptor is not (as with ES class constructors and many DOM-style constructors). While the SDK is active, code can replace WebSocket.prototype/MyClass.prototype successfully where the original constructor would reject or ignore the assignment, which changes observable constructor semantics; define the original prototype descriptor instead of assigning just the value.

Useful? React with 👍 / 👎.

bcaudan · 2026-06-05T08:07:55Z

+      // body. If a subclass extends the wrapper, or a third party wraps us and their class is
+      // `new`ed, `new.target` is that outer constructor and must be preserved.


🥜 nitpick: it is what you mean, right?
‏

Suggested change

// body. If a subclass extends the wrapper, or a third party wraps us and their class is

// `new`ed, `new.target` is that outer constructor and must be preserved.

// body. If a subclass extends the wrapper, or a third party wraps us and their class is

// instantiated, `new.target` is that outer constructor and must be preserved.

bcaudan · 2026-06-05T08:24:22Z

+        onPreCall,
+        {
+          parameters,
+          handlingStack: computeHandlingStack ? createHandlingStack('instrumented method') : undefined,


🥜 nitpick: it could be handy for debugging‏

Suggested change

handlingStack: computeHandlingStack ? createHandlingStack('instrumented method') : undefined,

handlingStack: computeHandlingStack ? createHandlingStack('instrumented constructor') : undefined,

bcaudan · 2026-06-05T08:27:20Z

  }
 })

+describe('instrumentConstructor', () => {


💬 suggestion: ‏comparing with instrumentMethod tests, it could be nice to have some equivalents on:

computeHandlingStack

third party compatibility

bcaudan · 2026-06-05T08:32:42Z

+  }
+
+  return replaceWithInstrumentation(target, name, original, (isStopped) => {
+    const instrumentation = function (this: TARGET): ConstructorInstanceOf<TARGET[NAME]> {


💬 suggestion: ‏this is actually not TARGET here but since we don't use it, we could just remove it

Suggested change

const instrumentation = function (this: TARGET): ConstructorInstanceOf<TARGET[NAME]> {

const instrumentation = function (): ConstructorInstanceOf<TARGET[NAME]> {

bcaudan · 2026-06-05T08:38:32Z

+  // Preserve own static members (class fields, static methods, etc.) on the instrumented function.
+  const excludedKeys = new Set(['prototype', 'length', 'name', 'arguments', 'caller'])


💬 suggestion: ‏Not sure to get why each of them are excluded, especially name/length, it could be worth adding a comment.

Sure! It's because we don't want to overwrite the ones of the subclass.

This was referenced Jun 3, 2026

✨ Collect WebSocket resource events #4718

Draft

✨ Add trackWebSockets configuration option (default: false) #4717

Draft

🔧 Sync rum-events-format with WebSocket resource schema #4716

Draft

bdibon mentioned this pull request Jun 3, 2026

✨ Add webSocketObservable for WebSocket lifecycle events #4715

Draft

5 tasks

bdibon marked this pull request as ready for review June 3, 2026 11:36

bdibon requested a review from a team as a code owner June 3, 2026 11:36

chatgpt-codex-connector Bot reviewed Jun 3, 2026

View reviewed changes

Comment thread packages/core/src/tools/instrumentMethod.ts Outdated

Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated

bdibon and others added 4 commits June 4, 2026 10:49

✨ Add constructor support to instrumentMethod

99462cd

Co-authored-by: Cursor <cursoragent@cursor.com>

👌 Handle subclassing instrumentation

b7a70bf

✅ Test after stop() behavior

698e884

👌 Preserve static members

e2bdea2

bdibon force-pushed the boris.dibon/ws-1-instrument-method-constructor branch from c445e3c to e2bdea2 Compare June 4, 2026 09:16

chatgpt-codex-connector Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated

bcaudan reviewed Jun 4, 2026

View reviewed changes

bdibon added 4 commits June 4, 2026 15:02

👌 Preserve original new.target when instrumentation is called directly

681eb7e

👌 Replace WebSocket occurences with MyClass

25e0810

👌 Merge duplicated code branches

f12b23f

👌 Separate constructor from method instrumentation logic

0b52da5

chatgpt-codex-connector Bot reviewed Jun 4, 2026

View reviewed changes

bdibon added 2 commits June 4, 2026 17:24

👌 Preserve new.target when constructing a subclass of the instrumente…

93ff3de

…d method after stop

👌 Outline limitation of static members preservation

0f0b7a0

chatgpt-codex-connector Bot reviewed Jun 4, 2026

View reviewed changes

bcaudan reviewed Jun 5, 2026

View reviewed changes

		// body. If a subclass extends the wrapper, or a third party wraps us and their class is
		// `new`ed, `new.target` is that outer constructor and must be preserved.

	handlingStack: computeHandlingStack ? createHandlingStack('instrumented method') : undefined,
	handlingStack: computeHandlingStack ? createHandlingStack('instrumented constructor') : undefined,

	const instrumentation = function (this: TARGET): ConstructorInstanceOf<TARGET[NAME]> {
	const instrumentation = function (): ConstructorInstanceOf<TARGET[NAME]> {

		// Preserve own static members (class fields, static methods, etc.) on the instrumented function.
		const excludedKeys = new Set(['prototype', 'length', 'name', 'arguments', 'caller'])

Conversation

bdibon commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Test instructions

Checklist

Uh oh!

bdibon commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cit-pr-commenter-54b7da Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundles Sizes Evolution

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

datadog-official Bot commented Jun 3, 2026 • edited by datadog-datadog-prod-us1-2 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

bdibon commented Jun 3, 2026 •

edited

Loading

bdibon commented Jun 3, 2026 •

edited

Loading

cit-pr-commenter-54b7da Bot commented Jun 3, 2026 •

edited

Loading

datadog-official Bot commented Jun 3, 2026 •

edited by datadog-datadog-prod-us1-2 Bot

Loading