Skip to content

fix(sdk): defensive resource creation for otel v1, v2 resolution #647

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.

Sign up for GitHub

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

Merged
nirga merged 6 commits into from
Aug 15, 2025
Merged

fix(sdk): defensive resource creation for otel v1, v2 resolution #647

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
11ad43a
fix(sdk): defensive resource creation for otel v1, v2 resolution
galkleinman Aug 15, 2025
cf44602
lint
galkleinman Aug 15, 2025
8f95bd5
more convenient impl
galkleinman Aug 15, 2025
cbd1577
Apply suggestion from @ellipsis-dev[bot]
nirga Aug 15, 2025
42a627b
fix: remove unused envDetector and processDetector imports
nirga Aug 15, 2025
db16cd1
fix: apply prettier formatting to tracing index.ts
nirga Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/traceloop-sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,26 @@
"engines": {
"node": ">=14"
},
"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"
},
Comment on lines +25 to +30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 15, 2025

Choose a reason for hiding this comment

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

💡 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/resources and @opentelemetry/sdk-trace-base are part[1].

For context:

  • This version bump was introduced with the release of OpenTelemetry JS SDK 2.0 and covers releases >=2.0.0 for 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:

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.

"peerDependenciesMeta": {
"@opentelemetry/api": {
"optional": false
},
"@opentelemetry/resources": {
"optional": false
},
"@opentelemetry/sdk-trace-base": {
"optional": false
},
"@opentelemetry/exporter-trace-otlp-proto": {
"optional": false
}
},
"files": [
"dist/**/*.js",
"dist/**/*.mjs",
Expand Down
41 changes: 37 additions & 4 deletions packages/traceloop-sdk/src/lib/tracing/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { NodeSDK } from "@opentelemetry/sdk-node";
import { SpanProcessor } from "@opentelemetry/sdk-trace-node";
import { context, diag } from "@opentelemetry/api";
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-proto";
import { resourceFromAttributes } from "@opentelemetry/resources";
import {
resourceFromAttributes,
Resource,
envDetector,
processDetector,
} from "@opentelemetry/resources";
import { ATTR_SERVICE_NAME } from "@opentelemetry/semantic-conventions";
import { Instrumentation } from "@opentelemetry/instrumentation";
import { InitializeOptions } from "../interfaces";
Expand Down Expand Up @@ -307,10 +312,38 @@ export const startTracing = (options: InitializeOptions) => {
spanProcessors.push(options.processor);
}

// 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({
Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev bot Aug 15, 2025

Choose a reason for hiding this comment

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

Consider leveraging the newly imported detectResources (with envDetector and processDetector) to merge additional default resource attributes into your custom resource for enhanced resilience.

[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,
});
}

_sdk = new NodeSDK({
resource: resourceFromAttributes({
[ATTR_SERVICE_NAME]: options.appName || process.env.npm_package_name,
}),
resource,
spanProcessors,
contextManager: options.contextManager,
textMapPropagator: options.propagator,
Expand Down
Loading
⚡