fix(sdk): defensive resource creation for otel v1, v2 resolution#647
fix(sdk): defensive resource creation for otel v1, v2 resolution#647
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughAdds OpenTelemetry peer dependency metadata to traceloop-sdk’s package.json and updates tracing initialization to precompute, validate, and fallback for an OpenTelemetry Resource (service.name) before using it in NodeSDK configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant TracingInit
participant ResourceBuilder
participant NodeSDK
participant OTLPExporter
App->>TracingInit: initTracing(appName?)
TracingInit->>ResourceBuilder: build Resource (service.name)
alt Resource valid
ResourceBuilder-->>TracingInit: Resource
else Fallback
ResourceBuilder-->>TracingInit: Minimal Resource + warning
end
TracingInit->>NodeSDK: configure(resource)
NodeSDK->>OTLPExporter: setup exporter
App-->>NodeSDK: start
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 11ad43a in 1 minute and 27 seconds. Click for details.
- Reviewed
87lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1draft 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/traceloop-sdk/package.json:25
- Draft comment:
Duplicating opentelemetry packages as both dependencies and peerDependencies can lead to version conflicts. Verify if both are needed. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_JAw8xpJU13maVBDE
You can customize 
| diag.warn('Failed to create resource with resourceFromAttributes, using fallback', error); | ||
| resource = resourceFromAttributes({}); | ||
| // Manually set the service name on the fallback resource | ||
| (resource as any).attributes = { |
There was a problem hiding this comment.
Avoid mutating resource attributes via a type cast. Consider using Resource.merge (or an official API) rather than (resource as any).attributes assignment.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed cf44602 in 44 seconds. Click for details.
- Reviewed
46lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft 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/traceloop-sdk/src/lib/tracing/index.ts:311
- Draft comment:
Stylistic update: The multi-line assignment for serviceName improves readability. Ensure it’s consistent with your project’s style. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/src/lib/tracing/index.ts:322
- Draft comment:
Good defensive checks on the resource object. Note that using '!resource' safely catches null values (since typeof null is 'object') – consider abstracting these checks into a helper if reused elsewhere. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/traceloop-sdk/src/lib/tracing/index.ts:332
- Draft comment:
The diag.warn formatting update (using double quotes and trailing commas) improves clarity. Verify that the trailing comma complies with your linting rules. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_MZ3pqP0hhfKpHynY
You can customize
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
311-311: Consider honoring OTEL_SERVICE_NAMESince many users set
service.nameviaOTEL_SERVICE_NAME, include it in the precedence chain before falling back tonpm_package_nameor a default.If you apply the previous diff, this is already addressed.
packages/traceloop-sdk/package.json (1)
31-44:peerDependenciesMetawith"optional": falseis redundant and ignored
peerDependenciesMeta.optionalonly has meaning whentrue. Setting it tofalseprovides no benefit and adds noise. Consider removing this block entirely or marking only truly optional peers as{ "optional": true }.- "peerDependenciesMeta": { - "@opentelemetry/api": { - "optional": false - }, - "@opentelemetry/resources": { - "optional": false - }, - "@opentelemetry/sdk-trace-base": { - "optional": false - }, - "@opentelemetry/exporter-trace-otlp-proto": { - "optional": false - } - }, + // peerDependenciesMeta not needed unless marking peers as optional
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/traceloop-sdk/package.json(1 hunks)packages/traceloop-sdk/src/lib/tracing/index.ts(2 hunks)
🔇 Additional comments (2)
packages/traceloop-sdk/src/lib/tracing/index.ts (2)
342-342: LGTM: Using the precomputedresourcein NodeSDK configPassing the constructed
resourceinto NodeSDK keeps initialization consistent and avoids repeated creation.
5-5: resourceFromAttributes is a stable public API in both OTel v1.x and v2.x
The functionresourceFromAttributesis officially exported by@opentelemetry/resourcesin both major versions. There’s no breaking change or removal documented, and current guides (including the OTel JS docs and Azure Monitor examples) continue to use it. You can safely keep the existing import.
- Official OTel JS documentation shows
resourceFromAttributesin v1.x and v2.x- Recent v2.0 blog posts and migration notes make no mention of its removal
- Azure Monitor (June 2025) sample code imports and uses it directly
No changes required here—please ignore the original suggestion.
Likely an incorrect or invalid review comment.
| "peerDependencies": { | ||
| "@opentelemetry/api": "^1.9.0", | ||
| "@opentelemetry/resources": "^2.0.0", | ||
| "@opentelemetry/sdk-trace-base": "^2.0.0", | ||
| "@opentelemetry/exporter-trace-otlp-proto": "^0.203.0" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Peer ranges should reflect v1 and v2 compatibility to match the PR objective
Declaring only v2 for several OpenTelemetry peers contradicts the “v1, v2 resolution” goal and can force consumers onto v2. Use union ranges to support both v1 and v2 where applicable.
Apply this diff (adjust minimal v1 versions to those you actually support):
"peerDependencies": {
"@opentelemetry/api": "^1.9.0",
- "@opentelemetry/resources": "^2.0.0",
- "@opentelemetry/sdk-trace-base": "^2.0.0",
- "@opentelemetry/exporter-trace-otlp-proto": "^0.203.0"
+ "@opentelemetry/resources": "^1.10.0 || ^2.0.0",
+ "@opentelemetry/sdk-trace-base": "^1.10.0 || ^2.0.0",
+ "@opentelemetry/exporter-trace-otlp-proto": "^0.40.0 || ^0.203.0"
},Also, consider removing these packages from "dependencies" to avoid duplicate installs and version skew. If needed for local builds/tests, move them to "devDependencies".
Please verify Node.js engine compatibility: OTel v2 typically requires Node >= 18. If you retain v2 ranges, the "engines.node": ">=14" may be misleading for consumers on Node 14/16.
🌐 Web query:
What Node.js versions are required by @opentelemetry/resources v2.x and @opentelemetry/sdk-trace-base v2.x?
💡 Result:
@opentelemetry/resources v2.x and @opentelemetry/sdk-trace-base v2.x require a minimum Node.js version of 18.19.0 or 20.6.0 and above[1]. This requirement applies to all stable OpenTelemetry JS SDK v2.x packages, including these two[1].
- The requirement is formally described as:
Node.js:^18.19.0 || >=20.6.0 - Node.js 14 and 16 are no longer supported[1].
- This minimum is in line with OpenTelemetry JS SDK v2.x, under which both
@opentelemetry/resourcesand@opentelemetry/sdk-trace-baseare part[1].
For context:
- This version bump was introduced with the release of OpenTelemetry JS SDK 2.0 and covers releases
>=2.0.0for stable packages (including these)[1]. - You must use TypeScript 5.0.4 or newer if you use TypeScript[1].
If you intend to use these packages, ensure your application is running Node 18.19.0+, or 20.6.0+ to guarantee support and compatibility[1][7].
Citations:
- 1: https://opentelemetry.io/blog/2025/otel-js-sdk-2-0/
- 2: https://learn.microsoft.com/en-us/azure/azure-monitor/app/opentelemetry-nodejs-migrate
- 3: https://opentelemetry.io/docs/languages/js/resources/
- 4: https://github.com/open-telemetry/opentelemetry-js/blob/main/README.md?plain=1
- 5: https://learn.microsoft.com/en-us/azure/azure-monitor/app/opentelemetry-enable
- 6: https://www.influxdata.com/blog/how-use-opentelemetry-javascript-tutorial/
- 7: https://opentelemetry.io/docs/languages/js/
- 8: https://www.npmjs.com/package/@opentelemetry/sdk-trace-base
- 9: https://www.npmjs.com/package/@opentelemetry/sdk-node
- 10: https://axiom.co/docs/guides/opentelemetry-nodejs
Bump OpenTelemetry peer ranges for v1/v2 support and enforce Node ≥18.19.0
- Adjust peerDependencies to allow both v1 (e.g. ^1.10.0) and v2 (^2.0.0) for the OTel packages.
- Move any OpenTelemetry packages from
"dependencies"to"devDependencies"to avoid duplicate installs and version skew. - Update
"engines.node"in package.json from" >=14"to" >=18.19.0"(v2.x requires Node 18.19.0 or newer).
"peerDependencies": {
"@opentelemetry/api": "^1.9.0",
- "@opentelemetry/resources": "^2.0.0",
- "@opentelemetry/sdk-trace-base": "^2.0.0",
- "@opentelemetry/exporter-trace-otlp-proto": "^0.203.0"
+ "@opentelemetry/resources": "^1.10.0 || ^2.0.0",
+ "@opentelemetry/sdk-trace-base": "^1.10.0 || ^2.0.0",
+ "@opentelemetry/exporter-trace-otlp-proto": "^0.40.0 || ^0.203.0"
},
- "dependencies": {
- /* …OTel packages… */
- },
- "engines": {
- "node": ">=14"
- }
+ "devDependencies": {
+ /* Move OTel packages here if used only for build/tests */
+ },
+ "engines": {
+ "node": ">=18.19.0"
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
packages/traceloop-sdk/package.json lines 25-30: update peerDependencies to
accept both OpenTelemetry v1 and v2 (e.g. set each OTel package to "^1.10.0 ||
^2.0.0"), move any OpenTelemetry packages currently listed under "dependencies"
into "devDependencies" to avoid duplicate installs/version skew, and change
"engines.node" from ">=14" to ">=18.19.0"; ensure package.json still validates
(run npm install / npm pack) after edits.
| // Create resource with defensive handling for OTLP serialization | ||
| const serviceName = options.appName || process.env.npm_package_name || "unknown-service"; | ||
| let resource: Resource; | ||
|
|
||
| try { | ||
| resource = resourceFromAttributes({ | ||
| [ATTR_SERVICE_NAME]: serviceName, | ||
| }); | ||
|
|
||
| // Defensive check to prevent OTLP serialization errors | ||
| // Ensure the resource has the expected structure for createResourceMap | ||
| if (!resource || typeof resource !== 'object') { | ||
| throw new Error('Invalid resource object'); | ||
| } | ||
|
|
||
| // Ensure resource has required properties for OTLP export | ||
| if (!resource.attributes || typeof resource.attributes !== 'object') { | ||
| throw new Error('Resource missing attributes'); | ||
| } | ||
|
|
||
| } catch (error) { | ||
| // Fallback: create a basic resource manually | ||
| diag.warn('Failed to create resource with resourceFromAttributes, using fallback', error); | ||
| resource = resourceFromAttributes({}); | ||
| // Manually set the service name on the fallback resource | ||
| (resource as any).attributes = { | ||
| [ATTR_SERVICE_NAME]: serviceName, | ||
| ...((resource as any).attributes || {}), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify and harden resource creation; remove fallback that re-invokes the same failing path and avoid mutating resource.attributes
- The fallback path re-calls
resourceFromAttributes({}), which will fail for the same reason as the primary call. - Directly mutating
(resource as any).attributesis brittle and may break with internal changes or readonly properties. Construct aResourceonce with the attributes you need (works in OTel v1 and v2) and let NodeSDK merge with defaults.
Apply this diff:
- // Create resource with defensive handling for OTLP serialization
- const serviceName = options.appName || process.env.npm_package_name || "unknown-service";
- let resource: Resource;
-
- try {
- resource = resourceFromAttributes({
- [ATTR_SERVICE_NAME]: serviceName,
- });
-
- // Defensive check to prevent OTLP serialization errors
- // Ensure the resource has the expected structure for createResourceMap
- if (!resource || typeof resource !== 'object') {
- throw new Error('Invalid resource object');
- }
-
- // Ensure resource has required properties for OTLP export
- if (!resource.attributes || typeof resource.attributes !== 'object') {
- throw new Error('Resource missing attributes');
- }
-
- } catch (error) {
- // Fallback: create a basic resource manually
- diag.warn('Failed to create resource with resourceFromAttributes, using fallback', error);
- resource = resourceFromAttributes({});
- // Manually set the service name on the fallback resource
- (resource as any).attributes = {
- [ATTR_SERVICE_NAME]: serviceName,
- ...((resource as any).attributes || {}),
- };
- }
+ // Create resource with a construction compatible across OTel v1 and v2
+ const serviceName =
+ options.appName ||
+ process.env.OTEL_SERVICE_NAME ||
+ process.env.npm_package_name ||
+ "unknown-service";
+ const resource: Resource = new Resource({ [ATTR_SERVICE_NAME]: serviceName });Notes:
- NodeSDK will merge this with its default resource; no need to mutate attributes post-construction.
- Added support for the standard OTEL_SERVICE_NAME env variable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Create resource with defensive handling for OTLP serialization | |
| const serviceName = options.appName || process.env.npm_package_name || "unknown-service"; | |
| let resource: Resource; | |
| try { | |
| resource = resourceFromAttributes({ | |
| [ATTR_SERVICE_NAME]: serviceName, | |
| }); | |
| // Defensive check to prevent OTLP serialization errors | |
| // Ensure the resource has the expected structure for createResourceMap | |
| if (!resource || typeof resource !== 'object') { | |
| throw new Error('Invalid resource object'); | |
| } | |
| // Ensure resource has required properties for OTLP export | |
| if (!resource.attributes || typeof resource.attributes !== 'object') { | |
| throw new Error('Resource missing attributes'); | |
| } | |
| } catch (error) { | |
| // Fallback: create a basic resource manually | |
| diag.warn('Failed to create resource with resourceFromAttributes, using fallback', error); | |
| resource = resourceFromAttributes({}); | |
| // Manually set the service name on the fallback resource | |
| (resource as any).attributes = { | |
| [ATTR_SERVICE_NAME]: serviceName, | |
| ...((resource as any).attributes || {}), | |
| }; | |
| } | |
| // Create resource with a construction compatible across OTel v1 and v2 | |
| const serviceName = | |
| options.appName || | |
| process.env.OTEL_SERVICE_NAME || | |
| process.env.npm_package_name || | |
| "unknown-service"; | |
| const resource: Resource = new Resource({ [ATTR_SERVICE_NAME]: serviceName }); |
🤖 Prompt for AI Agents
In packages/traceloop-sdk/src/lib/tracing/index.ts around lines 310-339, the
current code calls resourceFromAttributes and on error re-invokes
resourceFromAttributes({}) and mutates resource.attributes; instead, build the
desired attributes up-front (serviceName = options.appName ||
process.env.OTEL_SERVICE_NAME || process.env.npm_package_name ||
"unknown-service"), call resourceFromAttributes exactly once with that
attributes object, remove the fallback that re-calls resourceFromAttributes, and
do not mutate the returned Resource; if resourceFromAttributes throws, log the
error and create a new Resource instance containing only the pre-computed
attributes (so you do not attempt to mutate internals or re-run the same failing
call) so NodeSDK can merge it with defaults.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 8f95bd5 in 1 minute and 44 seconds. Click for details.
- Reviewed
60lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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/traceloop-sdk/src/lib/tracing/index.ts:337
- Draft comment:
Simplified fallback resource construction now directly calls resourceFromAttributes with the service name. Ensure that no essential default attributes are lost compared to the previous manual merge. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_FFloN6ogqZCTbPs0
You can customize
|
|
||
| try { | ||
| // Create our custom resource with service name and let NodeSDK handle default detection | ||
| resource = resourceFromAttributes({ |
There was a problem hiding this comment.
Consider leveraging the newly imported detectResources (with envDetector and processDetector) to merge additional default resource attributes into your custom resource for enhanced resilience.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
316-344: Fallback retries the same failing path; construct Resource directly and respect OTEL_SERVICE_NAMEThe catch block calls
resourceFromAttributesagain with the same args, so if the initial call throws (version mismatch, API differences), the fallback likely throws too and bubbles up. The extra shape checks are unnecessary for a typedResource. Prefer constructingnew Resource(works in OTel v1 and v2) and includeprocess.env.OTEL_SERVICE_NAMEin the resolution order.Apply this diff:
- // Create resource with proper detection and defensive handling for OTLP serialization - const serviceName = - options.appName || process.env.npm_package_name || "unknown-service"; - let resource: Resource; - - try { - // Create our custom resource with service name and let NodeSDK handle default detection - resource = resourceFromAttributes({ - [ATTR_SERVICE_NAME]: serviceName, - }); - - // Defensive check to prevent OTLP serialization errors - if (!resource || typeof resource !== "object") { - throw new Error("Invalid resource object"); - } - - if (!resource.attributes || typeof resource.attributes !== "object") { - throw new Error("Resource missing attributes"); - } - } catch (error) { - // Fallback: create a basic resource manually - diag.warn( - "Failed to create resource with resourceFromAttributes, using fallback", - error, - ); - resource = resourceFromAttributes({ - [ATTR_SERVICE_NAME]: serviceName, - }); - } + // Create a minimal Resource compatible with OTel v1 and v2; NodeSDK will merge with detected defaults + const serviceName = + options.appName || + process.env.OTEL_SERVICE_NAME || + process.env.npm_package_name || + "unknown-service"; + const resource: Resource = new Resource({ [ATTR_SERVICE_NAME]: serviceName });
🧹 Nitpick comments (1)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
5-11: Simplify imports to the stable API usedWith the above change, prefer importing only
Resourcefrom@opentelemetry/resources. This keeps the code lean and avoids reliance on helpers that complicate v1/v2 compatibility.-import { - resourceFromAttributes, - Resource, - detectResources, - envDetector, - processDetector, -} from "@opentelemetry/resources"; +import { Resource } from "@opentelemetry/resources";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/traceloop-sdk/src/lib/tracing/index.ts(2 hunks)
🔇 Additional comments (1)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
347-347: LGTM: Passing the precomputed Resource into NodeSDK is correctNodeSDK will merge this with its detected defaults, satisfying the PR’s goal to ensure a valid
service.namewithout mutating internals.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/traceloop-sdk/src/lib/tracing/index.ts (2)
315-343: Avoid re-invoking the same failing path; use a stable fallback and honor OTEL_SERVICE_NAME
- The catch block calls resourceFromAttributes again with the same input; if it failed once, it’s likely to fail again, defeating the “warn instead of throw” goal.
- Add support for OTEL_SERVICE_NAME to align with OTel conventions.
- Simplify/remove redundant defensive checks; NodeSDK accepts a Resource instance and merges defaults.
Apply this diff:
- // Create resource with proper detection and defensive handling for OTLP serialization - const serviceName = - options.appName || process.env.npm_package_name || "unknown-service"; - let resource: Resource; - - try { - // Create our custom resource with service name and let NodeSDK handle default detection - resource = resourceFromAttributes({ - [ATTR_SERVICE_NAME]: serviceName, - }); - - // Defensive check to prevent OTLP serialization errors - if (!resource || typeof resource !== "object") { - throw new Error("Invalid resource object"); - } - - if (!resource.attributes || typeof resource.attributes !== "object") { - throw new Error("Resource missing attributes"); - } - } catch (error) { - // Fallback: create a basic resource manually - diag.warn( - "Failed to create resource with resourceFromAttributes, using fallback", - error, - ); - resource = resourceFromAttributes({ - [ATTR_SERVICE_NAME]: serviceName, - }); - } + // Create resource with defensive handling and a stable fallback + const serviceName = + options.appName || + process.env.OTEL_SERVICE_NAME || + process.env.npm_package_name || + "unknown-service"; + let resource: Resource; + + try { + resource = resourceFromAttributes({ [ATTR_SERVICE_NAME]: serviceName }); + } catch (error) { + // Fallback: construct a plain Resource to avoid re-invoking the same failing path + diag.warn( + "Failed to create resource via resourceFromAttributes; falling back to bare Resource", + error, + ); + resource = new Resource({ [ATTR_SERVICE_NAME]: serviceName }); + }
5-10: Fix CI: remove unused imports (TS6133) to unblock the buildenvDetector and processDetector are imported but never used, causing the pipeline failure.
Apply this diff:
import { - resourceFromAttributes, - Resource, - envDetector, - processDetector, + resourceFromAttributes, + Resource, } from "@opentelemetry/resources";
🧹 Nitpick comments (1)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
315-315: Clarify comment or actually perform detectionThe comment says “proper detection,” but detection isn’t performed here. Either adjust the comment or wire in detection elsewhere (NodeSDK can merge defaults).
Apply this diff (comment tweak):
- // Create resource with proper detection and defensive handling for OTLP serialization + // Create resource with defensive handling for OTLP serialization; NodeSDK will merge defaults
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/traceloop-sdk/src/lib/tracing/index.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/traceloop-sdk/src/lib/tracing/index.ts
[error] 8-8: @rollup/plugin-typescript TS6133: 'envDetector' is declared but its value is never read. Build failed during 'nx run @traceloop/node-server-sdk:build' (rollup -c).
🔇 Additional comments (1)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
345-347: LGTM: Passing the precomputed Resource into NodeSDK is correctThis cleanly centralizes resource construction and lets NodeSDK merge in defaults.
Removes unused imports that were causing TypeScript build errors: - envDetector from @opentelemetry/resources - processDetector from @opentelemetry/resources 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 42a627b in 42 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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/traceloop-sdk/src/lib/tracing/index.ts:6
- Draft comment:
Removal of unused imports 'envDetector' and 'processDetector' improves code clarity. Ensure this removal is consistent with the updated behavior of resource creation in the SDK. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_0H3cHk8o8J2HuTem
You can customize
Fixes prettier formatting issues after removing unused imports. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed db16cd1 in 28 seconds. Click for details.
- Reviewed
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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/traceloop-sdk/src/lib/tracing/index.ts:2
- Draft comment:
Simplify multiline import to a single line for better readability; the change is stylistic and poses no functional risk. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_lxIVsds7YkroWPIi
You can customize
2 participants
Important
Improves tracing initialization with defensive resource creation and updates OpenTelemetry dependencies for compatibility.
startTracing()inindex.ts, adds defensive resource creation to handle OTLP serialization errors.unknown-serviceif not provided.@opentelemetry/api,@opentelemetry/resources,@opentelemetry/sdk-trace-base, and@opentelemetry/exporter-trace-otlp-protoas non-optional peer dependencies inpackage.json.This description was created by
for db16cd1. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
Chores