Skip to content

⚗️ Add instrumentConstructor utility#4714

Merged
bdibon merged 23 commits into
mainfrom
boris.dibon/ws-1-instrument-method-constructor
Jun 10, 2026
Merged

⚗️ Add instrumentConstructor utility#4714
bdibon merged 23 commits into
mainfrom
boris.dibon/ws-1-instrument-method-constructor

Conversation

@bdibon

@bdibon bdibon commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Motivation

In order to instrument WebSocket connections, we need to wrap the WebSocket constructor, so far the instrumentMethod only supports instrumenting regular methods. This PR adds another utility instrumentConstructor supporting the instrumentation of constructor functions.

Changes

  • Add an instrumentConstructor utility that supports instrumenting constructor functions
  • Factorize logic between instrumentMethod and new instrumentConstructorutils
  • Add exhaustive unit tests for instrumentConstructor

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 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@bdibon bdibon marked this pull request as ready for review June 3, 2026 11:36
@bdibon bdibon requested a review from a team as a code owner June 3, 2026 11:36
@cit-pr-commenter-54b7da

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

Copy link
Copy Markdown

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 172.08 KiB 172.19 KiB +106 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.60 KiB 54.69 KiB +94 B +0.17%
Rum Slim 129.90 KiB 129.99 KiB +94 B +0.07%
Worker 22.96 KiB 22.96 KiB 0 B 0.00%

@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: 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".

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

datadog-official Bot commented Jun 3, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 86.11%
Overall Coverage: 76.81% (+0.03%)

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

@bdibon 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 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: 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".

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

@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: 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".

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

@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: 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".

Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.spec.ts
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
@bdibon bdibon force-pushed the boris.dibon/ws-1-instrument-method-constructor branch from 0f0b7a0 to 751858d Compare June 8, 2026 11:17

@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: 751858d195

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

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

@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: 201cc19f39

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

Comment thread packages/browser-core/src/tools/instrumentMethod.ts
@bdibon bdibon force-pushed the boris.dibon/ws-1-instrument-method-constructor branch from 201cc19 to 8d89c48 Compare June 8, 2026 15:12
Comment thread packages/browser-core/src/tools/instrumentMethod.ts
@bdibon bdibon changed the title ✨ Add constructor support to instrumentMethod ✨ Add instrumentConstructor utility Jun 8, 2026
@bdibon bdibon changed the title ✨ Add instrumentConstructor utility ⚗️ Add instrumentConstructor utility Jun 8, 2026
Comment on lines +169 to +170
export function instrumentConstructor<CONTAINER extends { [key: string]: any }, CONSTRUCTOR extends keyof CONTAINER>(
container: CONTAINER,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💬 suggestion: ‏is there a difference between CONTAINER and TARGET? if not, let's use the same term

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's semantics.

With instrumentMethod the target will also be the this value of instrumentation / original, so this is TARGET, I can wrap my head around this one.

With instrumentConstructor, in the normal usage (when it's called with new), the this value refers to the instance being constructed, "target" feels weird in this case, since we're really interested in the (global) object holding the reference to the function we're instrumenting.

Wdyt?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could it make sense to use CONTAINER for both instrumentMethod and instrumentConstructor?
Since the type parameter describes the object holding the reference and the this value is already surfaced through target in InstrumentedMethodCall for callers who need it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would leave it as is, it makes sense to have targetPrototype / TARGET for instrumentMethod and container / CONTAINER for instrumentConstructor.

I'll wait for a third pair of eyes to come by.

@BenoitZugmeyer BenoitZugmeyer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm impressed!

Comment thread packages/browser-core/src/tools/instrumentMethod.spec.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
@bdibon bdibon force-pushed the boris.dibon/ws-1-instrument-method-constructor branch from 8d89c48 to a191d86 Compare June 10, 2026 09:09
@bdibon bdibon requested a review from BenoitZugmeyer June 10, 2026 09:10
Comment thread packages/browser-core/src/tools/instrumentMethod.ts Outdated
@bdibon bdibon merged commit 25ec8c5 into main Jun 10, 2026
31 checks passed
@bdibon bdibon deleted the boris.dibon/ws-1-instrument-method-constructor branch June 10, 2026 12:12
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants