Generalize setupOptionOtel as an independent runtime - #22
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesOption runtime isolation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant setupOptionOtel
participant OptionRuntime
participant ConfiguredPropagator
Application->>setupOptionOtel: provide resourceAttributes and traceparentHeader
setupOptionOtel->>ConfiguredPropagator: validate traceparentHeader
setupOptionOtel->>OptionRuntime: create isolated Resource and runtime
OptionRuntime->>ConfiguredPropagator: extract or inject configured header
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/option-runtime.ts`:
- Around line 122-132: Update the traceparentHeader validation in the
option-runtime setup path to reject the trimmed value “traceparent”
case-insensitively, while preserving existing invalid-header handling. Add
coverage for both “traceparent” and “TraceParent” to verify they are rejected.
🪄 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: 18a10c62-3708-47c9-b030-7803e943e9eb
📒 Files selected for processing (9)
README.mddocs/concepts/architecture.mdxdocs/guides/option-runtime.mdxdocs/reference/api.mdxpackage.jsonsrc/option-runtime.tssrc/setup.tssrc/version.tstests/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
interfacefor defining object shapes in TypeScript rather thantypealiases
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Useas constconst 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
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, and never usevar
Alwaysawaitpromises in async functions and use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code withtry-catchblocks
Don't use async functions as Promise executors
Files:
src/version.tssrc/setup.tstests/option-runtime.test.tssrc/option-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}: Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks 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
Addrel="noopener"when usingtarget="_blank"on links
AvoiddangerouslySetInnerHTMLunless absolutely necessary
Don't useeval()or assign directly todocument.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 (indexfiles that re-export everything)
Use proper image components (for example, Next.js<Image>) over<img>tags
Usenext/heador the App Router metadata API for head elements
Use Server Components for async data fetching instead of async Client Components
Files:
src/version.tssrc/setup.tstests/option-runtime.test.tssrc/option-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/option-runtime.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,jsx,ts,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests; use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat; avoid excessivedescribenesting
Files:
tests/option-runtime.test.ts
🔇 Additional comments (9)
src/option-runtime.ts (1)
20-23: LGTM!Also applies to: 36-51, 66-68, 165-191, 303-310
docs/reference/api.mdx (1)
93-110: LGTM!Also applies to: 144-145
README.md (1)
8-8: LGTM!Also applies to: 98-98, 119-140
docs/concepts/architecture.mdx (1)
60-63: LGTM!docs/guides/option-runtime.mdx (1)
6-15: LGTM!Also applies to: 24-39, 56-67
src/setup.ts (1)
22-22: LGTM!Also applies to: 356-366
tests/option-runtime.test.ts (1)
29-116: LGTM!Also applies to: 127-397
package.json (1)
3-3: LGTM!src/version.ts (1)
1-1: LGTM!
There was a problem hiding this comment.
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 `@tests/option-runtime.test.ts`:
- Around line 177-184: Update the resource assertion in the option runtime test
to require exact equality with only the explicit service.name and
service.version attributes, replacing the partial toMatchObject check while
preserving the existing span-count assertion.
🪄 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: 0c646675-3d89-44c0-a7fc-48c3f5498a0c
📒 Files selected for processing (4)
docs/guides/option-runtime.mdxdocs/reference/api.mdxsrc/option-runtime.tstests/option-runtime.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/reference/api.mdx
- docs/guides/option-runtime.mdx
- src/option-runtime.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
interfacefor defining object shapes in TypeScript rather thantypealiases
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Useas constconst 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
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, and never usevar
Alwaysawaitpromises in async functions and use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code withtry-catchblocks
Don't use async functions as Promise executors
Files:
tests/option-runtime.test.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}: Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks 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
Addrel="noopener"when usingtarget="_blank"on links
AvoiddangerouslySetInnerHTMLunless absolutely necessary
Don't useeval()or assign directly todocument.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 (indexfiles that re-export everything)
Use proper image components (for example, Next.js<Image>) over<img>tags
Usenext/heador the App Router metadata API for head elements
Use Server Components for async data fetching instead of async Client Components
Files:
tests/option-runtime.test.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 insideit()ortest()blocks
Avoid done callbacks in async tests; use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat; avoid excessivedescribenesting
Files:
tests/option-runtime.test.ts
🔇 Additional comments (1)
tests/option-runtime.test.ts (1)
20-20: LGTM!Also applies to: 36-61, 85-87
| expect(publicExportedSpans).toHaveLength(1); | ||
| expect(publicExportedSpans[0]?.resource.attributes).toMatchObject({ | ||
| "service.name": "option-service", | ||
| "service.version": "1.2.3", | ||
| }); | ||
| expect( | ||
| publicExportedSpans[0]?.resource.attributes["main.runtime"] | ||
| ).toBeUndefined(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete option resource.
toMatchObject accepts unspecified resource attributes. The test can pass if the option runtime retains an unintended attribute, such as the main runtime's deployment.environment.
Assert that resource.attributes equals the two supplied attributes. This verifies the contract that the option runtime uses only its explicit resource attributes.
Proposed test change
- expect(publicExportedSpans[0]?.resource.attributes).toMatchObject({
+ expect(publicExportedSpans[0]?.resource.attributes).toEqual({
"service.name": "option-service",
"service.version": "1.2.3",
});
- expect(
- publicExportedSpans[0]?.resource.attributes["main.runtime"]
- ).toBeUndefined();📝 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.
| expect(publicExportedSpans).toHaveLength(1); | |
| expect(publicExportedSpans[0]?.resource.attributes).toMatchObject({ | |
| "service.name": "option-service", | |
| "service.version": "1.2.3", | |
| }); | |
| expect( | |
| publicExportedSpans[0]?.resource.attributes["main.runtime"] | |
| ).toBeUndefined(); | |
| expect(publicExportedSpans).toHaveLength(1); | |
| expect(publicExportedSpans[0]?.resource.attributes).toEqual({ | |
| "service.name": "option-service", | |
| "service.version": "1.2.3", | |
| }); |
🤖 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 `@tests/option-runtime.test.ts` around lines 177 - 184, Update the resource
assertion in the option runtime test to require exact equality with only the
explicit service.name and service.version attributes, replacing the partial
toMatchObject check while preserving the existing span-count assertion.
Summary
Validation
Related
Summary by CodeRabbit
New Features
Documentation
Tests