-
Notifications
You must be signed in to change notification settings - Fork 41
feat: add support for OpenAI SDK v5 #644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 3e0b7b1 in 55 seconds. Click for details.
- Reviewed
52
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
5
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/instrumentation-openai/src/instrumentation.ts:28
- Draft comment:
Import updated to include OpenAIVersion for type safety. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, stating that an import was updated for type safety. It does not provide any actionable feedback or suggestions for improvement. According to the rules, purely informative comments should be removed.
2. packages/instrumentation-openai/src/instrumentation.ts:121
- Draft comment:
Updated version range to [">=3.1.0 <6"] to support OpenAI SDK v5. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only states what was done without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest any changes.
3. packages/instrumentation-openai/src/instrumentation.ts:213
- Draft comment:
Changed patchOpenAI parameter type to OpenAIVersion for clarity and support for v5. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining a change that was made. It does not provide a suggestion, ask for confirmation, or point out a potential issue. It violates the rule against making purely informative comments.
4. packages/instrumentation-openai/src/instrumentation.ts:602
- Draft comment:
Updated _wrapPromise version parameter to use OpenAIVersion, ensuring v5 is handled like v4. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing a change made in the code without providing any actionable feedback or suggestions. It does not ask for confirmation or suggest improvements, nor does it point out any potential issues with the change.
5. packages/instrumentation-openai/src/types.ts:3
- Draft comment:
Extended OpenAIVersion union to include "v5", ensuring proper type support. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, stating what was done without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest any improvements.
Workflow ID: wflow_bm8yv8lJH3cH4kRw
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
WalkthroughExpands OpenAI SDK compatibility from ">=3.1.0 <5" to ">=3.1.0 <6", adds an exported type Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Instrumentation as instrumentation.ts
participant Wrapper as _wrapPromise
Caller->>Instrumentation: call patchOpenAI(version: OpenAIVersion)
Instrumentation->>Wrapper: wrap promises with version
alt version === "v3"
Wrapper->>Wrapper: apply v3-specific wiring
else v4 or v5
Wrapper->>Wrapper: apply non-v3 wiring (shared)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hmnzr! Can you sign the CLA and update package.json with OpenAI v6 - this will trigger the tests with the new version so we can see that everything indeed works (I'm not sure it will though)
@nirga I did sign it but it still haven't refreshed the status. As for bumping the version, you mean the dev dependency or manually bump the package.json version? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/instrumentation-openai/src/instrumentation.ts (1)
216-216
: Clarify defaulting behavior: v5 currently shares the v4 code pathTo avoid confusion for future readers, annotate that v5 intentionally uses the non-v3 branch.
Apply this minimal inline comment:
- version: OpenAIVersion = "v4", + version: OpenAIVersion = "v4", // v5 currently shares v4 behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/instrumentation-openai/src/instrumentation.ts
(4 hunks)packages/instrumentation-openai/src/types.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/instrumentation-openai/src/instrumentation.ts (1)
packages/instrumentation-openai/src/types.ts (1)
OpenAIVersion
(3-3)
🔇 Additional comments (3)
packages/instrumentation-openai/src/types.ts (1)
3-3
: OpenAIVersion union extended to include v5 — looks goodThis aligns the public type surface with the widened SDK support.
packages/instrumentation-openai/src/instrumentation.ts (2)
28-28
: Type import update is consistent with the new OpenAIVersionImporting the union from a single source keeps version handling consistent.
605-605
: Confirmed: APIPromise._thenUnwrap remains stable in v5, returning the unwrapped entity
We’ve verified that in OpenAI Node SDK v5 (5.0.0-alpha) the private-style helper_thenUnwrap
still exists and continues to resolve to the parsed data payload (not anOpenAIResponse
wrapper), matching v4 behavior.• Note that
_thenUnwrap
is not part of the public API and may change without notice.
• For guaranteed stability, you can useawait
(which yields the parsed entity), or the new public methodsasResponse()
(to get the rawResponse
) andwithResponse()
(to get both{ data, response }
).
@@ -119,7 +119,7 @@ export class OpenAIInstrumentation extends InstrumentationBase { | |||
protected init(): InstrumentationModuleDefinition { | |||
const module = new InstrumentationNodeModuleDefinition( | |||
"openai", | |||
[">=3.1.0 <5"], | |||
[">=3.1.0 <6"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Widened module semver range to >=3.1.0 <6 — verify repo-wide alignment (docs, deps, tests)
Ensure we don’t have stale references to "<5" elsewhere and that CI/tests cover v5.
Run the following to find and update any lingering constraints and to surface any openai dependency ranges:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for legacy range '>=3.1.0 <5' across the repo..."
rg -n ">=3\.1\.0\s*<5" || true
echo
echo "Searching for 'openai' version constraints in package.json files..."
fd -a package.json | while read -r f; do
hits=$(rg -n '"openai"\s*:\s*"[^\"]+"' "$f" || true)
if [[ -n "$hits" ]]; then
echo "$f"
echo "$hits"
echo
fi
done
echo "Checking docs/workflows for hardcoded SDK support statements..."
rg -n "(OpenAI SDK|openai@|>=3\.1\.0\s*<5|<5)" --glob '*.md' --glob '.github/**/*.yml' || true
Length of output: 5204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting instrumentation-openai package.json for openai dependency entries..."
grep -R '"openai"' -n packages/instrumentation-openai/package.json || true
Length of output: 300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Displaying lines 40-80 from packages/instrumentation-openai/package.json to inspect dependency sections..."
sed -n '40,80p' packages/instrumentation-openai/package.json
Length of output: 1173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Listing all GitHub workflow files:"
fd -a -e yml .github/workflows
echo
echo "Scanning workflows for 'openai' references:"
rg -n "openai" .github/workflows
echo
echo "Scanning workflows for matrix definitions:"
rg -n "matrix" .github/workflows
Length of output: 505
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting peerDependencies in instrumentation-openai package.json..."
grep -R '"peerDependencies"' -n packages/instrumentation-openai/package.json || true
sed -n '1,39p' packages/instrumentation-openai/package.json
echo
echo "Checking for GitHub workflow files..."
if [ -d ".github/workflows" ]; then
find .github/workflows -type f -name '*.yml'
else
echo ".github/workflows directory not found"
fi
echo
echo "Scanning workflows for 'matrix' or 'openai' references..."
grep -R -n -E 'matrix|openai' .github/workflows || true
Length of output: 1578
Ensure OpenAI v5 is covered in tests, deps, and CI
- ✅ No legacy
>=3.1.0 <5
ranges found in code or docs. - packages/instrumentation-openai/package.json
- devDependencies still pin
"openai": "4.38.3"
(tests only exercise v4). - No
peerDependencies
entry foropenai
—add:"peerDependencies": { "openai": ">=3.1.0 <6" }
- devDependencies still pin
- CI workflows (
.github/workflows/ci.yml
) have no matrix for multipleopenai
versions.- Introduce a job matrix to test against at least
[email protected]
and[email protected]
.
- Introduce a job matrix to test against at least
- After adding peerDeps and CI matrix, update devDependency or use
npm install openai@${{ matrix.version }}
in the test job.
🤖 Prompt for AI Agents
In packages/instrumentation-openai/src/instrumentation.ts around line 122, tests
and package metadata don’t cover OpenAI v5: add a peerDependencies entry in
packages/instrumentation-openai/package.json with "openai": ">=3.1.0 <6"; stop
pinning openai in devDependencies (or update it to a range that includes v5) so
local dev isn’t the only source of truth; modify .github/workflows/ci.yml to add
a job matrix for openai versions (at minimum 4.x and 5.x) and in the test job
run npm install openai@${{ matrix.version }} (or use npm ci after adjusting
package.json) so CI actually runs tests against both versions; ensure tests pass
with v5 and adjust any version-specific mocks or imports accordingly.
@hmnzr the dev dependency in the package.json |
@nirga done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/instrumentation-openai/package.json
(1 hunks)
@@ -55,7 +55,7 @@ | |||
"@types/mocha": "^10.0.10", | |||
"@types/node": "^24.0.15", | |||
"mocha": "^11.7.1", | |||
"openai": "4.38.3", | |||
"openai": "5.12.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Use a caret range for devDependency and verify Node.js >= 18 in CI/tooling
Pinning openai
to an exact version is inconsistent with the rest of devDependencies and prevents receiving compatible patch/minor updates within v5. Also, the OpenAI SDK v5 requires Node.js >= 18. Make sure your CI/tooling uses Node >= 18, otherwise installs/tests may fail even if this package’s engines allow >=14.
Apply this diff to align the semver policy:
- "openai": "5.12.2",
+ "openai": "^5.12.2",
If you want to make the Node requirement explicit in this package to avoid confusion for contributors running tests locally, consider bumping the engines field (outside this hunk). Example:
{
"engines": {
"node": ">=18"
}
}
Run this script to verify Node versions configured across the repo:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching engines declarations across package.json files..."
rg -n '"engines"\s*:\s*{' -A 3 package.json packages/**/package.json || true
echo
echo "Inspecting GitHub Actions node setup (if any)..."
if [ -d .github/workflows ]; then
rg -n "setup-node" -A 8 .github/workflows || true
rg -n "node-version" -A 2 .github/workflows || true
else
echo "No .github/workflows directory found."
fi
echo
echo "Checking local Node version config files..."
for f in .nvmrc .tool-versions .node-version; do
if [ -f "$f" ]; then
echo "== $f =="
cat "$f"
echo
fi
done
Length of output: 3790
Align openai
to semver policy & confirm Node.js ≥18
- In packages/instrumentation-openai/package.json, change the pinned
openai
devDependency to a caret range so that patch/minor updates within v5 are picked up. - CI already sources Node.js version from
.nvmrc
(currently “20”), so the OpenAI SDK v5 requirement (Node ≥18) is satisfied. - (Optional) To make the Node.js requirement explicit for local development, you can add an
engines
field to this package’s package.json:
--- a/packages/instrumentation-openai/package.json
+++ b/packages/instrumentation-openai/package.json
@@ devDependencies
- "openai": "5.12.2",
+ "openai": "^5.12.2",
Optional engines snippet:
{
"engines": {
"node": ">=18"
}
}
🤖 Prompt for AI Agents
In packages/instrumentation-openai/package.json around line 58, the
devDependency "openai" is pinned to an exact version ("5.12.2"); change it to a
caret range (e.g., "^5.12.2") so minor/patch updates within v5 are picked up,
and optionally add an "engines" field with "node": ">=18" to make the Node.js
requirement explicit for local development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 @hmnzr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Like I suspected @hmnzr - it requires some updates to the instrumentation code |
Closes #642
After bumping the SDK to v5, we've stopped receiving traces.
However, the new SDK seems to be backward compatible for chat completions and completions. I have tested a local build with
[email protected]
and it's still working fine.Important
Add support for OpenAI SDK v5 in
instrumentation.ts
andtypes.ts
, ensuring backward compatibility.InstrumentationNodeModuleDefinition
ininstrumentation.ts
to support OpenAI SDK versions ">=3.1.0 <6".OpenAIVersion
type intypes.ts
to include "v5".patchOpenAI()
and_wrapPromise()
ininstrumentation.ts
to useOpenAIVersion
type for version handling.This description was created by
for 3e0b7b1. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores