Skip to content

Generalize setupOptionOtel as an independent runtime - #23

Merged
Ryan Zhu (underthestars-zhy) merged 9 commits into
mainfrom
felix/eng-2270-developer-logs-v4-clean-complete-traces-before-clickhouse
Aug 12, 2026
Merged

Generalize setupOptionOtel as an independent runtime#23
Ryan Zhu (underthestars-zhy) merged 9 commits into
mainfrom
felix/eng-2270-developer-logs-v4-clean-complete-traces-before-clickhouse

Conversation

@gtxy27

@gtxy27 gtxy27 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Supersedes #22 after renaming the head branch to match ENG-2270.

Summary

The second OTel pipeline shipped in 3.4.0 as setupOptionOtel() — a runtime hardwired to Photon Developer Logs and dependent on the main setupOtel() runtime. This PR generalizes it into a genuinely independent runtime, then renames it to createIsolatedOtel() to match what it actually is.

setupOtel() behavior is unchanged. Its Resource attributes are built in the same order and resolve to the same values.

What changed

1. Truly independent, no ordering requirement

On main, the runtime was not independent at all — it borrowed the main runtime's Resource and threw if setup hadn't happened yet:

// src/option-runtime.ts on main
const resource = activeOtelResource();
if (!resource) {
  throw new Error(
    "setupOptionOtel: setupOtel() must complete before creating an option runtime"
  );
}

It now builds its own Resource from its own options. activeOtelResource() and the module-level activeResource cache are deleted from src/setup.ts, so the two pipelines share no code path and no import edge. The isolated runtime no longer requires setupOtel() to exist at all, in any order.

2. Caller-owned propagation header, validated

The carrier header and instrumentation scope were hardcoded product names:

const DEVELOPER_TRACEPARENT_HEADER = "photon-developer-traceparent";
const DEVELOPER_INSTRUMENTATION_SCOPE = "@photon-ai/developer-logs";

traceparentHeader is now a required caller-supplied option, rejected at construction with a TypeError when it is empty, not a valid HTTP header name, or the standard traceparent (case-insensitively — TraceParent is rejected too). Refusing to hijack traceparent is what keeps the isolated trace context from colliding with the main W3C context; spans propagate independently through the private header and traceparent is never rewritten.

Two identifiers were realigned to the package rather than the product:

main this PR
Instrumentation scope @photon-ai/developer-logs @photon-ai/otel
Context key @photon-ai/otel.option-runtime.local-span @photon-ai/isolated-otel.local-span

3. setupOptionOtelcreateIsolatedOtel

"Option" was a stranded domain word once the runtime was generalized, and it collided with "options," the config object — SetupOptionOtelOptions parses as "Setup Option Otel Options." The docs had already moved to "isolated"; the code now matches.

create* rather than setup* is deliberate and carries information: setupOtel() is an idempotent process-wide singleton that returns the same handle on a second call, while this is a plain factory returning a new independent runtime per call (pinned by a new test). It also matches the existing createLogger / createInstrumentedFetch convention for "returns an instance you own."

register: false on setupOtel() remains a separate, weaker concept — scoped mode still shares the global context manager, the W3C propagator, and the standard traceparent. The naming keeps "scoped" and "isolated" distinct.

4. Shared service identity across both runtimes

New src/service-resource.ts holds a ServiceResourceOptions base and a serviceResourceAttributes() helper. SetupOtelOptions and IsolatedOtelOptions both extend it, so service identity cannot drift between the two again.

This fixes a silent footgun: the isolated runtime previously accepted only resourceAttributes, and omitting service.name produced an empty Resource that backends display as unknown_service — with no compile-time or runtime signal. serviceName is now required.

Two asymmetries are intentional and now have tests:

  • No deployment.environment. setupOtel() injects it from DEPLOYMENT_ENV; the isolated runtime must not read ambient env, because its Resource is explicit by contract. A test sets DEPLOYMENT_ENV and asserts the attribute is absent.
  • No logLevel / register / instrumentFetch. setLogLevel writes module-global state (src/logger.ts), so accepting logLevel here would let an "isolated" runtime silently change the main runtime's log level. register is meaningless when never registering globals is the entire point, and the standard fetch instrumentation propagates traceparent, not this runtime's private header.

setupOtel's resourceAttributes widened from Record<string, string | number | boolean> to OTel's Attributes to match the base. Widening an input type, so existing callers are unaffected.

5. PHOTON_OTEL_VERSION derived from package.json

src/version.ts was a hand-maintained literal that nobody remembered to update: main ships package.json 3.5.0 alongside PHOTON_OTEL_VERSION 3.4.0, so published spans have been reporting a stale instrumentation-scope version for two releases.

import { version } from "../package.json";
export const PHOTON_OTEL_VERSION: string = version;

module: "Preserve" already implies resolveJsonModule, so no tsconfig change was needed. Rolldown inlines the value — dist/index.js contains a plain const PHOTON_OTEL_VERSION = "3.5.0"; — and tree-shaking drops the rest of the file (verified: no devDependencies, scripts, or keywords in the bundle).

Generating the file in a prebuild step was the alternative, and it was rejected because the drift would persist: the release bot bumps package.json and commits without regenerating. Gitignoring a generated file instead breaks bun run test on a fresh clone, since src/ and tests/ import it. Deriving the value removes the failure mode rather than adding a step someone has to remember.

Both resolution paths were checked, since exports.bun serves TypeScript source while npm consumers get dist:

  • Bun source path (exports.bunsrc/index.ts): 3.5.0
  • Node dist path (published): 3.5.0

Two guards were added: a unit test that fails if a hand-maintained literal is reintroduced, and a CI step that asserts the built artifact reports package.json's version — that is the one that matters, since the publish job builds from the commit the release bot just bumped.

Breaking changes

Change Impact
setupOptionOtelcreateIsolatedOtel Import fails to compile. No deprecated alias — clean break.
SetupOptionOtelOptionsIsolatedOtelOptions, OptionOtelHandleIsolatedOtelHandle Type imports fail to compile.
serviceName now required Compile error until supplied, instead of silently exporting unknown_service.
traceparentHeader no longer defaults to photon-developer-traceparent Callers must pass their private header explicitly.
Instrumentation scope @photon-ai/developer-logs@photon-ai/otel Queries or dashboards filtering on the old scope name need updating.
activeOtelResource() removed from src/setup.ts Was only consumed by the old coupled runtime.
PHOTON_OTEL_VERSION type "3.6.0"string Type-level widening; the value genuinely varies per release.

Migration

-import { setupOptionOtel } from "@photon-ai/otel";
+import { createIsolatedOtel } from "@photon-ai/otel";

-const runtime = setupOptionOtel({
+const runtime = createIsolatedOtel({
   endpoint: "https://collector.example.com",
+  serviceName: "my-service",
+  serviceVersion: "1.0.0",
+  traceparentHeader: "photon-developer-traceparent",
-  resourceAttributes: { "service.version": "1.0.0" },
 });

setupOtel() callers need no changes.

Versioning

package.json is intentionally not bumped in this PR — it is unchanged from main at 3.5.0. The buildspace release pipeline owns the version: determine-publish-version reads the previous version from the latest GitHub Release, classifies the diff, and the bump-npm-version block writes package.json and commits as photon-release[bot]. A hand-bump here would just be overwritten, and could even be overwritten downward if the classifier picked a patch.

This diff should classify as major (4.0.0) given the removed export and the newly required option. Publishing requires the release label on this PR at merge time; without it the release job is skipped entirely.

Validation

  • bun x ultracite check — clean (the one warning is a pre-existing broken symlink under .cursor/rules/)
  • bun run test — 129 passed, 12 files
  • node ./node_modules/vitest/vitest.mjs run — 129 passed, 12 files (CI runs Node 20 and 22, so the package.json import was verified on that path too)
  • bun run build — clean; dist/index.d.ts exports createIsolatedOtel / IsolatedOtelOptions / IsolatedOtelHandle, with ServiceResourceOptions emitted as an internal declaration rather than public surface
  • bunx tsc --noEmit — no new errors (3 pre-existing new Request(input, init) overload errors remain in the fetch test files; CI does not run tsc)
  • dist version check — built artifact and package.json agree

New test coverage in tests/isolated-runtime.test.ts (15 → 20 cases): a new runtime per call, serviceName / serviceVersion landing in the exported Resource, resourceAttributes overriding the derived service identity, and DEPLOYMENT_ENV being ignored.

Follow-up, not in this PR

The Mintlify nav config lives outside this repo and likely still points at docs/guides/option-runtime, which this PR renames to isolated-runtime.mdx. That path needs updating or the page 404s.

Related


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features

    • Added createIsolatedOtel() for independently managed OpenTelemetry runtimes.
    • Added configurable service metadata, resource attributes, propagation headers, and headers.
    • Added validation for endpoints and propagation header configuration.
    • Added support for isolated tracing, propagation, diagnostics, and independent shutdown.
  • Breaking Changes

    • Replaced setupOptionOtel() with createIsolatedOtel().
    • Updated exported types and removed the previous option-runtime APIs.
  • Documentation

    • Added an isolated-runtime guide and updated API and architecture documentation.
  • Bug Fixes

    • Package version reporting now stays synchronized with the declared package version.

Copilot AI lite review requested due to automatic review settings August 12, 2026 14:34

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR replaces setupOptionOtel() with createIsolatedOtel(). The isolated runtime creates its own resource, providers, exporters, context, and configurable propagation header. Shared resource construction and package-version validation are also updated.

Changes

Isolated runtime API

Layer / File(s) Summary
Shared service resource contract
src/service-resource.ts, src/setup.ts
ServiceResourceOptions and serviceResourceAttributes centralize service metadata and resource attributes. Main-runtime resource state and its accessor are removed.
Isolated runtime implementation
src/isolated-runtime.ts
The new API validates endpoints and private propagation headers. Each runtime creates independent resources, providers, exporters, context, and instrumentation identifiers.
Public API and runtime validation
src/index.ts, tests/isolated-runtime.test.ts
The package exports createIsolatedOtel, IsolatedOtelHandle, and IsolatedOtelOptions. Tests cover isolation, lifecycle, resources, propagation, logging, validation, and processor behavior.
Documentation and version validation
README.md, docs/..., src/version.ts, .github/workflows/ci.yml, tests/version.test.ts
Documentation describes isolated runtime configuration and lifecycle. Build and test checks compare PHOTON_OTEL_VERSION with package.json.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant createIsolatedOtel
  participant serviceResourceAttributes
  participant IsolatedOtelRuntime
  Caller->>createIsolatedOtel: provide service options and traceparentHeader
  createIsolatedOtel->>serviceResourceAttributes: build resource attributes
  createIsolatedOtel->>IsolatedOtelRuntime: create independent runtime
  IsolatedOtelRuntime->>IsolatedOtelRuntime: validate endpoint and header
  IsolatedOtelRuntime->>IsolatedOtelRuntime: propagate through configured header
Loading

Possibly related PRs

  • photon-hq/otel#17: Introduced the option runtime that this PR replaces.
  • photon-hq/otel#19: Updated the option runtime APIs and behavior superseded by this isolated runtime.
  • photon-hq/otel#22: Added the independent resource and configurable propagation behavior formalized here.

Suggested labels: release

Suggested reviewers: underthestars-zhy

Poem

I hop through a runtime, self-contained and bright,
With private trace headers tucked out of sight.
My resources are mine, my providers agree,
Version checks keep the package drift-free.
Squeak, ship, and celebrate—
The rabbit approves this isolated state! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: transforming setupOptionOtel into an independent runtime.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch felix/eng-2270-developer-logs-v4-clean-complete-traces-before-clickhouse

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@package.json`:
- Line 3: Update the package version from 3.6.0 to 4.0.0 to reflect the breaking
public API change, and synchronize the same version in src/version.ts. Ensure
both version declarations match.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6d441bb2-a051-4bd1-9e16-11c7ea7953ae

📥 Commits

Reviewing files that changed from the base of the PR and between f584cc5 and 48ad4db.

📒 Files selected for processing (9)
  • README.md
  • docs/concepts/architecture.mdx
  • docs/guides/option-runtime.mdx
  • docs/reference/api.mdx
  • package.json
  • src/option-runtime.ts
  • src/setup.ts
  • src/version.ts
  • tests/option-runtime.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Prefer interface for defining object shapes in TypeScript rather than type aliases

**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Prefer unknown over any when the type is genuinely unknown
Use as const const assertions for immutable values and literal types
Leverage TypeScript type narrowing instead of type assertions
Use meaningful variable names instead of magic numbers; extract descriptive constants
Use arrow functions for callbacks and short functions
Prefer for...of loops over .forEach() and indexed for loops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Use const by default, let only when reassignment is needed, and never use var
Always await promises in async functions and use the return value
Use async/await syntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors

Files:

  • src/version.ts
  • src/option-runtime.ts
  • tests/option-runtime.test.ts
  • src/setup.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

**/*.{js,jsx,ts,tsx}: Use camelCase for variable and function names in JavaScript/TypeScript
Use PascalCase for class and component names in JavaScript/TypeScript
Always use async/await for promise handling instead of .then() chains
Include JSDoc comments for exported functions and classes
Use meaningful variable names that clearly describe their purpose
Avoid deeply nested conditionals; use early returns or guard clauses instead
Use const by default, let when reassignment is needed, avoid var

**/*.{js,jsx,ts,tsx}: Remove console.log, debugger, and alert statements from production code
Throw Error objects with descriptive messages, not strings or other values
Use try-catch blocks meaningfully; don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Keep functions focused and under reasonable cognitive complexity limits
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Group related code together and separate concerns
Add rel="noopener" when using target="_blank" on links
Avoid dangerouslySetInnerHTML unless absolutely necessary
Don't use eval() or assign directly to document.cookie
Validate and sanitize user input
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Avoid barrel files (index files that re-export everything)
Use proper image components (for example, Next.js <Image>) over <img> tags
Use next/head or the App Router metadata API for head elements
Use Server Components for async data fetching instead of async Client Components

Files:

  • src/version.ts
  • src/option-runtime.ts
  • tests/option-runtime.test.ts
  • src/setup.ts
**/*.test.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Write unit tests for all public functions and components

Files:

  • tests/option-runtime.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{test,spec}.{js,jsx,ts,tsx}: Write assertions inside it() or test() blocks
Avoid done callbacks in async tests; use async/await instead
Don't use .only or .skip in committed code
Keep test suites reasonably flat; avoid excessive describe nesting

Files:

  • tests/option-runtime.test.ts
🔇 Additional comments (13)
src/option-runtime.ts (1)

20-23: LGTM!

Also applies to: 36-52, 67-69, 123-138, 171-177, 191-212, 309-316

src/setup.ts (1)

22-22: LGTM!

Also applies to: 356-366

tests/option-runtime.test.ts (1)

20-20: LGTM!

Also applies to: 29-66, 75-75, 85-110, 120-189, 220-220, 244-244, 285-299, 332-332, 354-361, 382-382, 402-402, 434-434, 444-444

README.md (3)

8-8: LGTM!


98-98: LGTM!


119-140: LGTM!

docs/concepts/architecture.mdx (1)

60-63: LGTM!

docs/guides/option-runtime.mdx (3)

6-14: LGTM!


23-38: LGTM!


55-66: LGTM!

docs/reference/api.mdx (2)

93-111: LGTM!


145-146: LGTM!

src/version.ts (1)

1-1: LGTM!

Comment thread package.json Outdated
Copilot AI review requested due to automatic review settings August 12, 2026 15:51

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai coderabbitai Bot added the release Fight on! label Aug 12, 2026
Copilot AI review requested due to automatic review settings August 12, 2026 15:59

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai coderabbitai Bot removed the release Fight on! label Aug 12, 2026
Copilot AI review requested due to automatic review settings August 12, 2026 16:07

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai coderabbitai Bot added the release Fight on! label Aug 12, 2026
Renames the isolated runtime API for clarity: the new name
`createIsolatedOtel` better communicates that it is a plain factory
(not a singleton) and that the result is an independent runtime.

- Rename `src/option-runtime.ts` → `src/isolated-runtime.ts`
- Rename `SetupOptionOtelOptions` → `IsolatedOtelOptions`
- Rename `OptionOtelHandle` → `IsolatedOtelHandle`
- Extract shared service identity into `src/service-resource.ts` so
  `setupOtel()` and `createIsolatedOtel()` accept `serviceName`,
  `serviceVersion`, and `resourceAttributes` identically
- Update all docs, tests, and the package version accordingly
Replaces the hand-maintained version literal with a direct import from
`package.json`, so the release pipeline only needs to bump one place.
Adds a unit test and a CI step to assert the built artifact reports the
correct version.
Copilot AI review requested due to automatic review settings August 12, 2026 21:15

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/isolated-runtime.ts (2)

174-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pass the package version to getTracer.

src/with-span.ts builds its tracer with resolveTracer("@photon-ai/otel", PHOTON_OTEL_VERSION). The isolated runtime omits the version, so spans it exports carry an instrumentation scope without a version. Pass PHOTON_OTEL_VERSION for consistent scope identity across both runtimes.

♻️ Proposed change
-  const tracer = tracerProvider.getTracer(INSTRUMENTATION_SCOPE);
+  const tracer = tracerProvider.getTracer(
+    INSTRUMENTATION_SCOPE,
+    PHOTON_OTEL_VERSION
+  );

Add the import:

import { PHOTON_OTEL_VERSION } from "./version";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/isolated-runtime.ts` around lines 174 - 178, Update the tracer
initialization in the isolated runtime to pass PHOTON_OTEL_VERSION as the
version argument to tracerProvider.getTracer, adding the version import from
"./version" if needed. Keep INSTRUMENTATION_SCOPE as the instrumentation scope
name.

130-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the empty check out of the try block.

Headers.prototype.set() throws a TypeError for empty and malformed header names in Node.js and Bun. Use a guard clause for an empty traceparentHeader, and keep the try block around the Headers.set() probe.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/isolated-runtime.ts` around lines 130 - 140, In the traceparentHeader
validation flow, move the empty-value guard before the try block so it directly
throws the existing empty-header TypeError. Keep only the new Headers().set()
probe inside try/catch, preserving the malformed-header error conversion in the
catch block.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/index.ts`:
- Around line 9-13: Update the package version to a major release, 4.0.0,
because the public exports setupOptionOtel, SetupOptionOtelOptions, and
OptionOtelHandle were removed from the index export surface. Alternatively,
restore those exports with deprecation notices while retaining the new
isolated-runtime exports.

In `@src/isolated-runtime.ts`:
- Around line 112-128: Update the validation errors in createIsolatedOtelRuntime
to identify the exported function consistently as createIsolatedOtelRuntime
instead of createIsolatedOtel; apply the same prefix to all related endpoint
validation throws, including the later validation at the referenced location.

In `@src/version.ts`:
- Line 12: Update the package manifest version to 3.6.0, ensuring the value used
by PHOTON_OTEL_VERSION remains synchronized with package.json and the release
checks observe the new version.

---

Nitpick comments:
In `@src/isolated-runtime.ts`:
- Around line 174-178: Update the tracer initialization in the isolated runtime
to pass PHOTON_OTEL_VERSION as the version argument to tracerProvider.getTracer,
adding the version import from "./version" if needed. Keep INSTRUMENTATION_SCOPE
as the instrumentation scope name.
- Around line 130-140: In the traceparentHeader validation flow, move the
empty-value guard before the try block so it directly throws the existing
empty-header TypeError. Keep only the new Headers().set() probe inside
try/catch, preserving the malformed-header error conversion in the catch block.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eaa97689-8b8f-44a7-9b50-9f8a50bd02bb

📥 Commits

Reviewing files that changed from the base of the PR and between 98f655b and efff421.

📒 Files selected for processing (12)
  • .github/workflows/ci.yml
  • README.md
  • docs/concepts/architecture.mdx
  • docs/guides/isolated-runtime.mdx
  • docs/reference/api.mdx
  • src/index.ts
  • src/isolated-runtime.ts
  • src/service-resource.ts
  • src/setup.ts
  • src/version.ts
  • tests/isolated-runtime.test.ts
  • tests/version.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/concepts/architecture.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Prefer interface for defining object shapes in TypeScript rather than type aliases

**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Prefer unknown over any when the type is genuinely unknown
Use as const const assertions for immutable values and literal types
Leverage TypeScript type narrowing instead of type assertions
Use meaningful variable names instead of magic numbers; extract descriptive constants
Use arrow functions for callbacks and short functions
Prefer for...of loops over .forEach() and indexed for loops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Use const by default, let only when reassignment is needed, and never use var
Always await promises in async functions and use the return value
Use async/await syntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors

Files:

  • src/service-resource.ts
  • src/version.ts
  • src/index.ts
  • tests/version.test.ts
  • src/setup.ts
  • tests/isolated-runtime.test.ts
  • src/isolated-runtime.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

**/*.{js,jsx,ts,tsx}: Use camelCase for variable and function names in JavaScript/TypeScript
Use PascalCase for class and component names in JavaScript/TypeScript
Always use async/await for promise handling instead of .then() chains
Include JSDoc comments for exported functions and classes
Use meaningful variable names that clearly describe their purpose
Avoid deeply nested conditionals; use early returns or guard clauses instead
Use const by default, let when reassignment is needed, avoid var

**/*.{js,jsx,ts,tsx}: Remove console.log, debugger, and alert statements from production code
Throw Error objects with descriptive messages, not strings or other values
Use try-catch blocks meaningfully; don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Keep functions focused and under reasonable cognitive complexity limits
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Group related code together and separate concerns
Add rel="noopener" when using target="_blank" on links
Avoid dangerouslySetInnerHTML unless absolutely necessary
Don't use eval() or assign directly to document.cookie
Validate and sanitize user input
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Avoid barrel files (index files that re-export everything)
Use proper image components (for example, Next.js <Image>) over <img> tags
Use next/head or the App Router metadata API for head elements
Use Server Components for async data fetching instead of async Client Components

Files:

  • src/service-resource.ts
  • src/version.ts
  • src/index.ts
  • tests/version.test.ts
  • src/setup.ts
  • tests/isolated-runtime.test.ts
  • src/isolated-runtime.ts
**/*.test.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)

Write unit tests for all public functions and components

Files:

  • tests/version.test.ts
  • tests/isolated-runtime.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{test,spec}.{js,jsx,ts,tsx}: Write assertions inside it() or test() blocks
Avoid done callbacks in async tests; use async/await instead
Don't use .only or .skip in committed code
Keep test suites reasonably flat; avoid excessive describe nesting

Files:

  • tests/version.test.ts
  • tests/isolated-runtime.test.ts
🔇 Additional comments (25)
README.md (3)

8-8: LGTM!


98-98: LGTM!


112-147: LGTM!

docs/guides/isolated-runtime.mdx (4)

2-33: LGTM!


35-57: LGTM!


59-64: LGTM!


66-74: LGTM!

docs/reference/api.mdx (4)

11-11: LGTM!


42-42: LGTM!


91-156: LGTM!


436-437: LGTM!

.github/workflows/ci.yml (1)

59-70: LGTM!

tests/version.test.ts (1)

1-12: LGTM!

src/service-resource.ts (1)

1-36: LGTM!

src/setup.ts (3)

49-54: LGTM!


357-361: 🗄️ Data Integrity & Integration

No remaining callers use the removed resource state.

			> Likely an incorrect or invalid review comment.

253-257: 🗄️ Data Integrity & Integration

No change required. resourceAttributes intentionally overrides defaults, including deployment.environment and service.name.

			> Likely an incorrect or invalid review comment.
src/isolated-runtime.ts (3)

35-60: LGTM!


179-221: LGTM!


310-321: LGTM!

tests/isolated-runtime.test.ts (5)

22-29: LGTM!

Also applies to: 63-83


120-155: LGTM!

Also applies to: 164-183


192-252: LGTM!


352-366: LGTM!

Also applies to: 418-426, 446-447, 467-467, 499-499, 509-509


106-118: 📐 Maintainability & Code Quality

No lifecycle reset issue. The hooks clear publicExportedSpans and shut down the active runtime. shutdown() also clears activeHandle.

Comment thread src/index.ts
Comment thread src/isolated-runtime.ts
Comment on lines +112 to +128
export const createIsolatedOtelRuntime = (
options: IsolatedOtelTransport,
resource: Resource,
processors?: {
readonly logRecordProcessors?: readonly LogRecordProcessor[];
readonly spanProcessors?: readonly SpanProcessor[];
}
): OptionOtelHandle => {
): IsolatedOtelHandle => {
const endpoint = options.endpoint.trim();
let endpointProtocol: string;
try {
endpointProtocol = new URL(endpoint).protocol;
} catch {
throw new TypeError("setupOptionOtel: endpoint must be a valid URL");
throw new TypeError("createIsolatedOtel: endpoint must be a valid URL");
}
if (!(endpointProtocol === "http:" || endpointProtocol === "https:")) {
throw new TypeError("setupOptionOtel: endpoint must use http or https");
throw new TypeError("createIsolatedOtel: endpoint must use http or https");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Align the error message prefix with the called function.

createIsolatedOtelRuntime is an exported function, and tests/isolated-runtime.test.ts calls it directly at lines 418 and 446. All validation messages name createIsolatedOtel instead. A caller of createIsolatedOtelRuntime then receives an error that names a different function. Derive the prefix from a constant that matches the throwing function, or state both names.

Also applies to: 141-144

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/isolated-runtime.ts` around lines 112 - 128, Update the validation errors
in createIsolatedOtelRuntime to identify the exported function consistently as
createIsolatedOtelRuntime instead of createIsolatedOtel; apply the same prefix
to all related endpoint validation throws, including the later validation at the
referenced location.

Comment thread src/version.ts
@underthestars-zhy
Ryan Zhu (underthestars-zhy) merged commit 3c109f1 into main Aug 12, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release Fight on!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants