Feature/prototype ts #33
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a new evaluation tagging system in the FI Core package, refactors OpenTelemetry registration to support eval tag validation and custom template checks, and adds new OpenAI tracing examples and dependencies. Several package configurations are updated for dependency alignment, and exports are expanded for broader type and utility access. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FITracerProvider
participant FI Core (otel.ts)
participant CustomEvalAPI
User->>FI Core (otel.ts): register({ projectName, evalTags, ... })
FI Core (otel.ts)->>FI Core (otel.ts): prepareEvalTags(evalTags)
FI Core (otel.ts)->>FI Core (otel.ts): Validate evalTags, check for duplicates
alt evalTags present
FI Core (otel.ts)->>CustomEvalAPI: checkCustomEvalConfigExists(projectName, evalTags)
CustomEvalAPI-->>FI Core (otel.ts): { exists: true/false }
alt exists
FI Core (otel.ts)->>FI Core (otel.ts): Log error (continue registration)
end
end
FI Core (otel.ts)->>FITracerProvider: new FITracerProvider({ ...preparedEvalTags })
FITracerProvider-->>User: TracerProvider instance
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
typescript/packages/fi-core/src/otel.ts (1)
700-705:⚠️ Potential issueAddress TODO comments before merging.
The TODO comments indicate incomplete implementation, particularly for
prepareEvalTags.The TODOs mention:
- Implementing prepareEvalTags (but it's already imported from fi_types)
- Implementing checkCustomEvalConfigExists (but it's already implemented)
- Refining error handling and logging
- Adding tests
These comments appear outdated. Either remove them if the work is complete or create issues to track the remaining work.
🧹 Nitpick comments (2)
typescript/packages/fi-semantic-conventions/package.json (1)
45-45: Add newline at end of file."publishConfig": { "access": "public" } -} +} +typescript/packages/traceai_openai/examples/package.json (1)
13-15: Include TS runtime in devDependencies
Addts-node(or similar) to enable running TypeScript examples without a separate compilation step."devDependencies": { - "typescript": "^5.8.3" + "typescript": "^5.8.3", + "ts-node": "^10.9.1" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
typescript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
typescript/packages/fi-core/package.json(3 hunks)typescript/packages/fi-core/src/fi_types.ts(1 hunks)typescript/packages/fi-core/src/index.ts(1 hunks)typescript/packages/fi-core/src/otel.ts(5 hunks)typescript/packages/fi-semantic-conventions/package.json(4 hunks)typescript/packages/traceai_langchain/examples/chat.ts(1 hunks)typescript/packages/traceai_langchain/package.json(2 hunks)typescript/packages/traceai_langchain/tsconfig.json(1 hunks)typescript/packages/traceai_openai/README.md(2 hunks)typescript/packages/traceai_openai/examples/basic-otel-test.ts(2 hunks)typescript/packages/traceai_openai/examples/example.ts(1 hunks)typescript/packages/traceai_openai/examples/package.json(1 hunks)typescript/packages/traceai_openai/package.json(3 hunks)typescript/packages/traceai_openai/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
typescript/packages/traceai_openai/examples/example.ts (1)
typescript/packages/traceai_openai/src/instrumentation.ts (1)
OpenAIInstrumentation(97-562)
typescript/packages/fi-core/src/fi_types.ts (2)
python/fi_instrumentation/fi_types.py (1)
ModelChoices(418-423)typescript/packages/fi-core/src/otel.ts (2)
CheckCustomEvalTemplateExistsResponse(518-524)checkCustomEvalTemplateExists(601-688)
🪛 Biome (1.9.4)
typescript/packages/fi-core/src/fi_types.ts
[error] 474-474: Invalid typeof comparison value: this expression is not a string literal
not a string literal
(lint/suspicious/useValidTypeof)
🔇 Additional comments (28)
typescript/packages/traceai_langchain/tsconfig.json (2)
4-4: Explicit outDir path is clear
Using"./dist"makes the output directory resolution explicit and matches the package.json configuration.
6-6: Explicit node type inclusion
Adding"node"to thetypesarray ensures that Node.js global declarations are available during compilation.typescript/packages/traceai_langchain/examples/chat.ts (1)
4-4: LGTM! Proper package import alignment.The change from relative import to published package import is correct and aligns with the package.json updates. This ensures the example demonstrates proper usage of the published package.
typescript/packages/traceai_langchain/package.json (4)
3-3: Version bump looks appropriate.The patch version increment from 0.1.0 to 0.1.1 follows semantic versioning conventions.
10-11: Script changes look reasonable.The addition of the lint script and the change from
pnpmtonpmin the prepare script are appropriate updates. The lint script using eslint is a good practice for code quality.
26-32: Keywords reorganization is cosmetic.Moving the keywords array below the bugs field is a minor organizational change with no functional impact.
5-6:Details
✅ Verification successful
Verify build output structure matches simplified entry points.
The entry points have been simplified from
"dist/src/index.js"to"dist/index.js"and similarly for types. Ensure that the TypeScript build configuration outputs files to the expecteddist/directory structure without thesrc/subdirectory.
🏁 Script executed:
#!/bin/bash # Description: Verify the TypeScript build configuration matches the new entry points # Expected: tsconfig.json should output to dist/ directly, not dist/src/ # Check if tsconfig.json exists and examine outDir configuration if [ -f "typescript/packages/traceai_langchain/tsconfig.json" ]; then echo "=== TypeScript Configuration ===" cat typescript/packages/traceai_langchain/tsconfig.json | jq '.compilerOptions.outDir // "not specified"' cat typescript/packages/traceai_langchain/tsconfig.json | jq '.compilerOptions.rootDir // "not specified"' else echo "tsconfig.json not found in traceai_langchain package" fi # Check if there's a build script and see the actual build output structure echo "=== Current dist directory structure ===" if [ -d "typescript/packages/traceai_langchain/dist" ]; then find typescript/packages/traceai_langchain/dist -type f -name "*.js" -o -name "*.d.ts" | head -10 else echo "dist directory not found" fiLength of output: 579
TSConfig output matches simplified entry points
Your
tsconfig.jsonspecifies
compilerOptions.outDir = "./dist"compilerOptions.rootDir = "./src"This configuration will compile
src/index.tstodist/index.jsand emitdist/index.d.ts, aligning exactly with"main": "dist/index.js", "types": "dist/index.d.ts"Once you run your build (e.g.,
npm run buildortsc -b), confirm thatdist/index.jsanddist/index.d.tsare generated at the repository root’sdist/directory.typescript/packages/fi-core/src/fi_types.ts (2)
1-86: Enums are well-structured and consistent with Python implementation.The evaluation system enums are comprehensive and properly typed. The
ModelChoicesenum correctly matches the Python implementation.
458-477: Field type validation logic is correct.The type validation function properly handles JavaScript types including special array handling.
🧰 Tools
🪛 Biome (1.9.4)
[error] 474-474: Invalid
typeofcomparison value: this expression is not a string literalnot a string literal
(lint/suspicious/useValidTypeof)
typescript/packages/traceai_openai/tsconfig.json (1)
6-7: LGTM! Node types addition is appropriate.Adding Node.js type definitions is correct for a package that uses Node.js features and aligns with the OpenTelemetry integration enhancements.
typescript/packages/fi-semantic-conventions/package.json (2)
2-4: Package updates look good overall.The version bump and dependency updates align with other packages in the monorepo. Including the "src" directory in files is consistent with the packaging approach.
Also applies to: 13-16, 34-37
36-36:Details
❓ Verification inconclusive
Verify uuid version compatibility.
You're adding
uuidat version^11.1.0. Please ensure this major version is compatible with your Node.js version requirements and doesn't introduce breaking changes compared to other packages in the monorepo that might use different uuid versions.
🏁 Script executed:
#!/bin/bash # Check if other packages use uuid and their versions echo "Checking uuid usage across the repository:" fd -e json package.json | xargs grep -l '"uuid"' | while read -r file; do echo "File: $file" grep '"uuid"' "$file" | grep -v node_modules done # Check Node.js version requirements echo -e "\nChecking Node.js version requirements:" fd -e json package.json typescript/ | xargs grep -h '"node":' | sort -uLength of output: 985
Confirm uuid@^11.1.0 consistency and Node.js compatibility
You’re already using
uuid@^11.1.0in the following packages, so the version is consistent across the monorepo:
- typescript/package.json
- typescript/packages/fi-core/package.json
- typescript/packages/fi-semantic-conventions/package.json
However, this major version bump may drop support for older Node.js runtimes or introduce breaking changes. I didn’t find any
engines.nodefields in your package.json files—please verify that your CI and production environments meet uuid@11’s minimum Node.js version requirement, and consider adding anengines.nodeentry or updating your documentation accordingly.typescript/packages/fi-core/src/index.ts (1)
5-5: Expose new evaluation tag types
Re-exporting./fi_typescentralizes evaluation-related enums and interfaces. Confirm thatfi_types.tsexists and only intended symbols are exposed publicly.typescript/packages/traceai_openai/README.md (3)
17-17: Switch code block language to JavaScript
Aligns with CommonJSrequireusage in the example.
22-24: Convert ES module imports to CommonJSrequire
Consistent with runtime environments that don't use ESM. Reviewed import order and dependencies – looks good.
44-44: Use synchronousrequirefor OpenAI client
Ensures instrumentation is registered before client initialization. Approved.typescript/packages/fi-core/package.json (3)
3-3: Bump package version to 0.1.1
Version bump follows semver for the new feature release.
16-16: Publish source files
Adding"src"tofilesensures that TypeScript definitions are available downstream.
28-31: Workspace reference and new dependencies
Good to adduuidand core OpenTelemetry packages. Ensure@traceai/fi-semantic-conventionsis present in the workspace.typescript/packages/traceai_openai/package.json (6)
3-3: Bump package version to 0.1.2
Follows semver for incremental enhancements.
11-11: Confirmpreparescript change
Switched frompnpm run buildtonpm run build. Verify this aligns with the monorepo’s primary package manager.
15-15: Include source directory in distribution
Publishing"src"alongside"dist"helps consumers access TypeScript definitions. Approved.
28-32: Update workspace dependencies
Switching@traceai/fi-coreand@traceai/fi-semantic-conventionsto workspace references is correct for local development.
35-35: Bumpopenaipeer dependency
Ensures compatibility with the latest OpenAI SDK.
39-44: Align devDependency versions
Dev dependencies for OpenTelemetry tooling are at1.25.1, but the exporter is0.50.0. Confirm the exporter’s major/minor version compatibility with the OTEL core modules or adjust versions accordingly.typescript/packages/traceai_openai/examples/basic-otel-test.ts (1)
37-60: Well-structured evaluation tags configuration.The implementation correctly demonstrates the new evaluation tagging system with proper use of the
EvalTagclass and related enums.typescript/packages/fi-core/src/otel.ts (2)
10-10: LGTM! Clean import of evaluation types.The modularization of evaluation-related types into
fi_typesimproves code organization.
431-452: Consider error handling implications of fire-and-forget pattern.The asynchronous
checkCustomEvalConfigExistsis called without awaiting, which means errors are only logged and won't prevent registration. Ensure this aligns with the intended behavior.This pattern allows the SDK to continue initializing even if the custom eval config check fails. Verify this is the desired behavior by checking if there are any downstream dependencies on this check.
| "rootDir": "./src", | ||
| "types": ["node"] | ||
| "types": ["node"], | ||
| "composite": true |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure referenced projects support composite builds
You've enabled "composite": true here, which requires that all referenced projects also have "composite": true in their own tsconfig.json files. Please verify that fi-core and fi-semantic-conventions tsconfigs are configured accordingly to avoid build-time errors.
🏁 Script executed:
#!/bin/bash
# Verify composite is enabled in referenced packages
for pkg in fi-core fi-semantic-conventions; do
echo "Checking $pkg"
if grep -q '"composite": true' "typescript/packages/$pkg/tsconfig.json"; then
echo " ✓ composite enabled"
else
echo " ✗ composite missing"
fi
doneLength of output: 473
Add composite: true to referenced project tsconfigs
Both fi-core and fi-semantic-conventions lack "composite": true, which is required when your root config enables composite builds.
Please update each package’s tsconfig.json, for example:
-
typescript/packages/fi-core/tsconfig.json
{ "compilerOptions": { + "composite": true, // …existing options }, // … } -
typescript/packages/fi-semantic-conventions/tsconfig.json
{ "compilerOptions": { + "composite": true, // …existing options }, // … }
📝 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.
| "composite": true | |
| // File: typescript/packages/fi-core/tsconfig.json | |
| { | |
| "compilerOptions": { | |
| + "composite": true, | |
| // …existing options | |
| }, | |
| // … | |
| } |
| "composite": true | |
| // File: typescript/packages/fi-semantic-conventions/tsconfig.json | |
| { | |
| "compilerOptions": { | |
| + "composite": true, | |
| // …existing options | |
| }, | |
| // … | |
| } |
🤖 Prompt for AI Agents
In typescript/packages/traceai_langchain/tsconfig.json at line 7, you have
enabled "composite": true, which requires all referenced projects to also have
"composite": true in their tsconfig.json files. To fix this, add "composite":
true to the tsconfig.json files of both typescript/packages/fi-core and
typescript/packages/fi-semantic-conventions. This ensures compatibility and
prevents build-time errors related to composite project references.
| [EvalName.IS_JSON]: {}, | ||
| [EvalName.ENDS_WITH]: { | ||
| case_sensitive: { type: 'boolean', default: true, required: false }, | ||
| substring: { type: 'string', default: null, required: true } |
There was a problem hiding this comment.
Review default values for required configuration fields.
Several configuration fields are marked as required: true but have default: null. This could lead to runtime errors if these fields are not provided. Consider either:
- Setting
required: falsefor fields with null defaults - Providing meaningful default values instead of null
- Removing the default property for required fields
Examples:
- Line 139:
substringin ENDS_WITH - Line 159:
substringin STARTS_WITH - Line 163:
urlin API_CALL - Line 176:
eval_promptin AGENT_AS_JUDGE - Line 188:
keywordin CONTAINS
Also applies to: 159-159, 163-163, 172-172, 176-176, 188-188
🤖 Prompt for AI Agents
In typescript/packages/fi-core/src/fi_types.ts at line 139 and similarly at
lines 159, 163, 176, and 188, fields marked as required have default values set
to null, which can cause runtime errors. To fix this, either remove the default
property for these required fields, set required to false if null is an
acceptable default, or provide meaningful non-null default values to ensure safe
usage.
| constructor(params: { | ||
| type: EvalTagType; | ||
| value: EvalSpanKind; | ||
| eval_name: EvalName | string; | ||
| custom_eval_name: string; | ||
| config?: Record<string, any>; | ||
| mapping?: Record<string, string>; | ||
| score?: number; | ||
| rationale?: string | null; | ||
| metadata?: Record<string, any> | null; | ||
| model?: ModelChoices; | ||
| }) { | ||
| this.type = params.type; | ||
| this.value = params.value; | ||
| this.eval_name = params.eval_name; | ||
| this.custom_eval_name = params.custom_eval_name ?? (params.eval_name as string); | ||
| this.config = params.config || {}; | ||
| this.mapping = params.mapping || {}; | ||
| this.score = params.score; | ||
| this.rationale = params.rationale; | ||
| this.metadata = params.metadata; | ||
| this.model = params.model; | ||
| this.validate(); | ||
| } |
There was a problem hiding this comment.
Fix async validation in constructor.
The constructor calls an async validate() method without awaiting it. This can lead to:
- Validation errors not being caught during construction
- Unhandled promise rejections
- Race conditions where the object is used before validation completes
Consider these solutions:
- Make validation synchronous and move async checks to a separate initialization method
- Use a factory pattern with an async static method to create validated instances
- Document that users must await a separate
init()method after construction
Example factory pattern:
+ static async create(params: {
+ type: EvalTagType;
+ value: EvalSpanKind;
+ eval_name: EvalName | string;
+ // ... other params
+ }): Promise<EvalTag> {
+ const tag = new EvalTag(params);
+ await tag.validate();
+ return tag;
+ }
constructor(params: {
// ... params
}) {
// ... assignments
- this.validate();
}📝 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.
| constructor(params: { | |
| type: EvalTagType; | |
| value: EvalSpanKind; | |
| eval_name: EvalName | string; | |
| custom_eval_name: string; | |
| config?: Record<string, any>; | |
| mapping?: Record<string, string>; | |
| score?: number; | |
| rationale?: string | null; | |
| metadata?: Record<string, any> | null; | |
| model?: ModelChoices; | |
| }) { | |
| this.type = params.type; | |
| this.value = params.value; | |
| this.eval_name = params.eval_name; | |
| this.custom_eval_name = params.custom_eval_name ?? (params.eval_name as string); | |
| this.config = params.config || {}; | |
| this.mapping = params.mapping || {}; | |
| this.score = params.score; | |
| this.rationale = params.rationale; | |
| this.metadata = params.metadata; | |
| this.model = params.model; | |
| this.validate(); | |
| } | |
| // Add a factory method to ensure async validation is awaited | |
| static async create(params: { | |
| type: EvalTagType; | |
| value: EvalSpanKind; | |
| eval_name: EvalName | string; | |
| custom_eval_name: string; | |
| config?: Record<string, any>; | |
| mapping?: Record<string, string>; | |
| score?: number; | |
| rationale?: string | null; | |
| metadata?: Record<string, any> | null; | |
| model?: ModelChoices; | |
| }): Promise<EvalTag> { | |
| const tag = new EvalTag(params); | |
| await tag.validate(); | |
| return tag; | |
| } | |
| constructor(params: { | |
| type: EvalTagType; | |
| value: EvalSpanKind; | |
| eval_name: EvalName | string; | |
| custom_eval_name: string; | |
| config?: Record<string, any>; | |
| mapping?: Record<string, string>; | |
| score?: number; | |
| rationale?: string | null; | |
| metadata?: Record<string, any> | null; | |
| model?: ModelChoices; | |
| }) { | |
| this.type = params.type; | |
| this.value = params.value; | |
| this.eval_name = params.eval_name; | |
| this.custom_eval_name = params.custom_eval_name ?? (params.eval_name as string); | |
| this.config = params.config || {}; | |
| this.mapping = params.mapping || {}; | |
| this.score = params.score; | |
| this.rationale = params.rationale; | |
| this.metadata = params.metadata; | |
| this.model = params.model; | |
| // removed: this.validate(); | |
| } |
🤖 Prompt for AI Agents
In typescript/packages/fi-core/src/fi_types.ts around lines 507 to 530, the
constructor calls an async validate() method without awaiting it, causing
potential unhandled promise rejections and race conditions. To fix this, remove
the async validate() call from the constructor and instead implement a static
async factory method that creates an instance, performs the async validation,
and returns the validated instance. Alternatively, separate the async validation
into an init() method that users must explicitly await after construction, and
document this requirement clearly.
| "dependencies": { | ||
| "@opentelemetry/api": "^1.9.0", | ||
| "@opentelemetry/exporter-trace-otlp-grpc": "^0.200.0", | ||
| "@opentelemetry/resources": "^2.0.0", | ||
| "@opentelemetry/sdk-trace-base": "^2.0.0", | ||
| "@opentelemetry/sdk-trace-node": "^2.0.0", | ||
| "@posthog/ai": "^3.3.2", | ||
| "@traceai/fi-core": "workspace:*", | ||
| "@traceai/openai": "workspace:*", | ||
| "dotenv": "^16.5.0" | ||
| }, |
There was a problem hiding this comment.
Verify required OpenTelemetry dependencies
The examples register instrumentations (e.g., @opentelemetry/instrumentation) but it’s not listed here. Add it if used in example scripts to avoid runtime errors.
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/package.json lines 2 to 12, the
dependencies list is missing the @opentelemetry/instrumentation package which is
required by the example scripts for registering instrumentations. Add
"@opentelemetry/instrumentation" with an appropriate version to the dependencies
section to ensure the examples run without runtime errors.
| "@types/node": "^20.0.0", | ||
| "@opentelemetry/context-async-hooks": "^1.25.1", | ||
| "@opentelemetry/resources": "^1.19.0", | ||
| "@opentelemetry/sdk-trace-base": "^1.19.0", | ||
| "@opentelemetry/sdk-trace-node": "^1.19.0", | ||
| "@opentelemetry/semantic-conventions": "^1.19.0" |
There was a problem hiding this comment.
Inconsistent OpenTelemetry versions
Dev dependencies mix 1.25.x and 1.19.x across OTEL packages. This can lead to type mismatches or runtime issues. Align all OpenTelemetry modules to a single matching version.
🤖 Prompt for AI Agents
In typescript/packages/fi-core/package.json around lines 36 to 41, the
OpenTelemetry package versions are inconsistent, mixing 1.25.x and 1.19.x
versions. To fix this, update all OpenTelemetry dependencies to use the same
version number, preferably the latest stable one, ensuring all
"@opentelemetry/*" packages have identical version strings to avoid type
mismatches and runtime issues.
| @@ -1,7 +1,6 @@ | |||
| import { register, ProjectType } from "@traceai/fi-core"; | |||
| import { register, ProjectType, EvalSpanKind, EvalName, EvalTag, EvalTagType } from "@traceai/fi-core"; | |||
There was a problem hiding this comment.
Add missing import for OpenTelemetry diagnostics.
The code uses diag on line 6 but doesn't import it.
Add the missing import:
-import { register, ProjectType, EvalSpanKind, EvalName, EvalTag, EvalTagType } from "@traceai/fi-core";
+import { register, ProjectType, EvalSpanKind, EvalName, EvalTag, EvalTagType } from "@traceai/fi-core";
+import { diag, DiagConsoleLogger, DiagLogLevel } from "@opentelemetry/api";Also applies to: 6-6
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/basic-otel-test.ts at line 1 and
line 6, the code uses the `diag` object but does not import it. Add an import
statement for `diag` from the appropriate OpenTelemetry package at the top of
the file to ensure `diag` is defined and can be used without errors.
| // 1. Register FI Core TracerProvider (sets up exporter) | ||
| const tracerProvider = register({ | ||
| projectName: "test-4", | ||
| projectType: "observe", |
There was a problem hiding this comment.
Use ProjectType enum instead of string literal.
For type safety and consistency, use the imported ProjectType enum.
- projectType: "observe",
+ projectType: ProjectType.OBSERVE,📝 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.
| projectType: "observe", | |
| projectType: ProjectType.OBSERVE, |
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/example.ts at line 16, replace
the string literal "observe" assigned to projectType with the corresponding
value from the imported ProjectType enum. This ensures type safety and
consistency by using ProjectType.observe instead of the raw string.
| // 2. Register OpenAI Instrumentation *BEFORE* importing/using OpenAI client | ||
| console.log("Registering OpenAI Instrumentation..."); | ||
| registerInstrumentations({ | ||
| tracerProvider: tracerProvider as any, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid type assertion with as any.
The type assertion suggests a type mismatch between the tracer provider returned by register() and what registerInstrumentations() expects.
Remove the type assertion - the types should be compatible:
- tracerProvider: tracerProvider as any,
+ tracerProvider: tracerProvider,If there's a genuine type incompatibility, it should be addressed at the type definition level rather than using type assertions.
📝 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.
| tracerProvider: tracerProvider as any, | |
| tracerProvider: tracerProvider, |
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/example.ts at line 24, remove the
type assertion `as any` from the `tracerProvider` property. Instead, ensure that
the `tracerProvider` returned by `register()` is correctly typed to match what
`registerInstrumentations()` expects. If a type mismatch exists, fix the type
definitions or adjust the code to align types properly rather than using `as
any`.
| }); | ||
| console.log("OpenAI client initialized",process.env.OPENAI_API_KEY); | ||
| const response = await openai.chat.completions.create({ | ||
| model: "gpt-4.1-mini", |
There was a problem hiding this comment.
Fix invalid OpenAI model name.
The model name "gpt-4.1-mini" appears to be incorrect.
Use a valid model name:
- model: "gpt-4.1-mini",
+ model: "gpt-4o-mini",📝 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.
| model: "gpt-4.1-mini", | |
| model: "gpt-4o-mini", |
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/example.ts at line 36, the model
name "gpt-4.1-mini" is invalid. Replace it with a valid OpenAI model name such
as "gpt-4" or "gpt-4o-mini" depending on the intended usage. Ensure the model
string matches one of the officially supported OpenAI model identifiers.
| const batchExporter = new HTTPSpanExporter({ | ||
| endpoint: (tracerProvider as any)._endpoint, | ||
| headers: exporterHeaders, | ||
| verbose: verbose | ||
| }); | ||
| const batchProcessor = new OTelBatchSpanProcessor(batchExporter); | ||
| (tracerProvider as any)._registeredSpanProcessors = []; | ||
|
|
||
| (tracerProvider as any)._registeredSpanProcessors = []; | ||
| (tracerProvider as any)._defaultProcessorAttached = false; | ||
| tracerProvider.addSpanProcessor(batchProcessor); |
There was a problem hiding this comment.
Avoid accessing private properties with type assertions.
Accessing private properties using type assertions breaks encapsulation and is fragile.
Consider exposing the endpoint through a public getter method or refactoring to avoid the need for private property access:
- const batchExporter = new HTTPSpanExporter({
- endpoint: (tracerProvider as any)._endpoint,
- headers: exporterHeaders,
- verbose: verbose
- });
+ // The endpoint should be accessible through a public API
+ const batchExporter = new HTTPSpanExporter({
+ endpoint: constructFullEndpoint(optEndpoint),
+ headers: exporterHeaders,
+ verbose: verbose
+ });Also remove the private property manipulation:
- (tracerProvider as any)._registeredSpanProcessors = [];
- (tracerProvider as any)._defaultProcessorAttached = false;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In typescript/packages/fi-core/src/otel.ts around lines 484 to 493, avoid
accessing private properties of tracerProvider using type assertions as it
breaks encapsulation. Refactor the code to use a public getter method or another
public API to obtain the endpoint instead of (tracerProvider as any)._endpoint.
Also, remove direct manipulation of private properties like
_registeredSpanProcessors and _defaultProcessorAttached by using appropriate
public methods or redesigning the logic to avoid touching these internals.
Pull Request
Description
Describe the changes in this pull request:
Checklist
Related Issues
Closes #<issue_number>
##Linear
Fixes TH-258
Summary by CodeRabbit