Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the Phase 1 “SDK Foundation” for the OTel Web SDK by introducing a new factory-based createOTelWebSdk() entry point and associated interfaces, while removing the legacy OTelSdk dynamicProto-based class and wiring in unit tests.
Changes:
- Added
createOTelWebSdk()factory and newIOTelWebSdk/IOTelWebSdkConfigpublic interfaces. - Removed legacy
OTelSdk+ related interfaces/exports and updatedshared/otel-corepublic exports accordingly. - Fixed
createContext().setValue()to return a new context instance and added extensive unit coverage for the new SDK.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/otel-core/src/utils/DataCacheHelper.ts | Changes cache namespace versioning behavior (currently hardcodes a version string). |
| shared/otel-core/src/otel/sdk/OTelWebSdk.ts | Adds the new SDK factory implementation (tracer + logger access, lifecycle APIs). |
| shared/otel-core/src/otel/sdk/OTelSdk.ts | Removes the legacy dynamicProto-based SDK class. |
| shared/otel-core/src/otel/api/context/context.ts | Fixes context immutability behavior for setValue(). |
| shared/otel-core/src/interfaces/otel/trace/IOTelTracerCtx.ts | Updates interface documentation wording. |
| shared/otel-core/src/interfaces/otel/config/IOTelWebSdkConfig.ts | Introduces SDK configuration interface for dependency injection. |
| shared/otel-core/src/interfaces/otel/IOTelWebSdk.ts | Introduces the main SDK interface (tracer/logger/lifecycle/config). |
| shared/otel-core/src/interfaces/otel/IOTelSdkCtx.ts | Removes legacy SDK context interface. |
| shared/otel-core/src/interfaces/otel/IOTelSdk.ts | Removes legacy SDK interface. |
| shared/otel-core/src/index.ts | Updates public exports to expose the new SDK API and remove the old one. |
| shared/otel-core/Tests/Unit/src/sdk/OTelWebSdk.Tests.ts | Adds unit tests covering construction, tracing, logging, flush/shutdown, and config. |
| shared/otel-core/Tests/Unit/src/index.tests.ts | Registers the new OTelWebSdk test suite. |
Comment on lines
+109
to
+111
| if (!config.performanceNow) { | ||
| handleError(_handlers, "createOTelWebSdk: performanceNow must be provided"); | ||
| } |
There was a problem hiding this comment.
The code validates performanceNow as a required config dependency, but the value is never used anywhere in OTelWebSdk.ts. Either wire performanceNow into span/log timing (or wherever it’s intended to be used) or make it optional so consumers aren’t forced to pass a dead field.
Suggested change
| if (!config.performanceNow) { | |
| handleError(_handlers, "createOTelWebSdk: performanceNow must be provided"); | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1: SDK Foundation (Critical)
Component Location Description
createOTelWebSdk() src/otel/sdk/ Main SDK entry point factory
IOTelWebSdkConfig src/interfaces/otel/config/ SDK configuration interface
Deprecate OTelSdk class src/otel/sdk/OTelSdk.ts Remove DynamicProto usage
Deliverable: Functional SDK that can be instantiated with trace+log providers.