Dev #35
Dev #35
Conversation
Version upgrade - ts
fixing the bug there config could not be set in span
|
Caution Review failedThe pull request is closed. WalkthroughThe updates enhance context propagation and metadata extraction in the Python LangChain tracer, refine TypeScript OpenTelemetry dependencies and instrumentation, and add a new OpenAI tracing example. TypeScript code also simplifies instrumentation type signatures. No major control flow changes are introduced, but context handling and dependency management are improved. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OpenAIExample
participant FITracerProvider
participant OpenAIInstrumentation
participant OpenAIClient
User->>OpenAIExample: Run chat.ts
OpenAIExample->>FITracerProvider: register({ projectName, ... })
OpenAIExample->>OpenAIInstrumentation: register()
OpenAIExample->>OpenAIClient: import and initialize
OpenAIExample->>OpenAIClient: createChatCompletion()
OpenAIClient-->>OpenAIExample: Return response
OpenAIExample->>User: Log response
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (66)
✨ Finishing Touches
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: 6
🧹 Nitpick comments (4)
typescript/packages/fi-core/package.json (1)
30-30: Consider the implications of exact version pinning.The change from
^0.1.0to0.1.0for@traceai/fi-semantic-conventionsremoves automatic patch updates. Ensure this exact pinning is intentional and consider whether patch updates should be allowed for internal packages.typescript/packages/traceai_openai/examples/chat.ts (1)
14-18: Consider moving setup inside the main function.The current module-level setup could cause issues with initialization order. Consider moving the tracer registration and instrumentation setup inside the
main()function for better control flow.+async function main() { + // 1. Register FI Core TracerProvider (sets up exporter) + const tracerProvider = register({ + projectName: "test4", + projectType: ProjectType.OBSERVE, + sessionName: "test-session-" + Date.now(), + }); + + // 2. Register OpenAI Instrumentation *BEFORE* importing/using OpenAI client + console.log("Registering OpenAI Instrumentation..."); + registerInstrumentations({ + tracerProvider: tracerProvider, + instrumentations: [new OpenAIInstrumentation()], + }); + + const OpenAI = (await import('openai')).default;python/frameworks/langchain/traceai_langchain/_tracer.py (2)
926-951: Comprehensive metadata handling with good backward compatibility.The enhanced metadata implementation:
- Maps standard fields to conventional attribute names (e.g.,
user_id→user.id)- Maintains backward compatibility with legacy session ID keys
- Includes both individual attributes and full JSON
Consider prefixing custom metadata fields to avoid potential attribute name conflicts:
else: # Add other metadata with metadata prefix to avoid conflicts - yield f"{key}", str_value + yield f"metadata.{key}", str_value
1043-1087: Function works correctly but has high cyclomatic complexity.The
_filter_imagesfunction now returns four values and handles various message formats. While functional, the static analysis correctly identifies high branch complexity (15 branches).Consider extracting helper functions to reduce complexity:
def _extract_message_content(message: dict) -> Tuple[List[dict], List[str], List[str]]: """Extract content from a single message.""" filtered_data = [] images = [] eval_input = [] kwargs = message.get("kwargs", {}) if not isinstance(kwargs, dict): return filtered_data, images, eval_input content = kwargs.get("content") if isinstance(content, str): filtered_data.append({"type": "text", "text": content}) eval_input.append(content) elif isinstance(content, list): for obj in content: # Process content items... return filtered_data, images, eval_input def _filter_images(messages: Any) -> Tuple[Any, List[str], str, str]: """Simplified main function using helper.""" if not messages or not isinstance(messages, list): return messages, [], "", "" all_filtered_data = [] all_images = [] all_eval_input = [] for message_group in messages: if isinstance(message_group, list): for message in message_group: if isinstance(message, dict): data, imgs, eval = _extract_message_content(message) all_filtered_data.extend(data) all_images.extend(imgs) all_eval_input.extend(eval) # Rest of the logic...🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 1052-1082: Too many nested blocks (7/5)
(R1702)
[refactor] 1052-1082: Too many nested blocks (7/5)
(R1702)
[refactor] 1043-1043: Too many branches (15/12)
(R0912)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
python/frameworks/langchain/traceai_langchain/_tracer.py(10 hunks)typescript/packages/fi-core/package.json(1 hunks)typescript/packages/traceai_openai/examples/basic-otel-test.ts(1 hunks)typescript/packages/traceai_openai/examples/chat.ts(1 hunks)typescript/packages/traceai_openai/package.json(2 hunks)typescript/packages/traceai_openai/src/instrumentation.ts(2 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
python/frameworks/langchain/traceai_langchain/_tracer.py
[refactor] 1043-1043: Too many branches (15/12)
(R0912)
🔇 Additional comments (5)
typescript/packages/traceai_openai/examples/basic-otel-test.ts (1)
35-35: LGTM! Project name version update.The version bump from v3 to v4 aligns with the broader updates in this PR.
typescript/packages/traceai_openai/src/instrumentation.ts (2)
97-97: LGTM! Type signature simplification.Removing the explicit generic type parameter simplifies the class declaration while maintaining functionality. The TypeScript compiler can still infer the correct types.
129-137: LGTM! Simplified method signature.The removal of explicit return type annotation and generic type parameter makes the code cleaner while preserving type safety through TypeScript's type inference.
python/frameworks/langchain/traceai_langchain/_tracer.py (2)
87-89: Good improvement to align with standard dictionary behavior.The addition of the
defaultparameter to thegetmethod makes_DictWithLockmore compatible with the standard dictionary interface.
195-208: Excellent implementation of context preservation.The context capturing mechanism ensures that span attributes reflect the context at span start rather than at span end, which is crucial for accurate tracing in concurrent environments. The thread-safe implementation using
_DictWithLockis appropriate.Also applies to: 249-256
| "@opentelemetry/api": "^1.9.0", | ||
| "@opentelemetry/core": "^1.25.1", | ||
| "@traceai/fi-semantic-conventions": "0.1.0" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify OpenTelemetry package version compatibility.
The OpenTelemetry packages have version mismatches that could cause compatibility issues:
@opentelemetry/api: ^1.9.0@opentelemetry/core: ^1.25.1- Other OpenTelemetry packages in devDependencies: ^1.19.0 and ^1.25.1
Ensure these version differences are intentional and compatible.
🏁 Script executed:
#!/bin/bash
# Check OpenTelemetry package compatibility and latest versions
echo "Checking OpenTelemetry package versions..."
# Check latest versions of each package
packages=("@opentelemetry/api" "@opentelemetry/core" "@opentelemetry/context-async-hooks" "@opentelemetry/resources" "@opentelemetry/sdk-trace-base" "@opentelemetry/sdk-trace-node" "@opentelemetry/semantic-conventions")
for package in "${packages[@]}"; do
echo "Checking $package..."
curl -s "https://registry.npmjs.org/$package/latest" | jq -r '.version' | sed "s/^/$package: /"
doneLength of output: 2475
To get an accurate picture of every OpenTelemetry package version in your fi-core module, let’s extract both dependencies and devDependencies directly from package.json:
🏁 Script executed:
#!/bin/bash
set -e
FILE="typescript/packages/fi-core/package.json"
echo "OpenTelemetry versions in $FILE:"
jq -r '
(.dependencies // {}) + (.devDependencies // {})
| to_entries[]
| select(.key | test("^@opentelemetry/"))
| "\(.key): \(.value)"
' "$FILE"Length of output: 648
Align OpenTelemetry dependencies to a single minor version
The current fi-core package.json mixes several OpenTelemetry minor versions, which can lead to internal API mismatches at runtime:
@opentelemetry/api: ^1.9.0@opentelemetry/core&@opentelemetry/context-async-hooks: ^1.25.1@opentelemetry/resources,@opentelemetry/sdk-trace-base,@opentelemetry/sdk-trace-node,@opentelemetry/semantic-conventions: ^1.19.0
Action items:
- Consolidate all
@opentelemetry/*packages to the same minor version (ideally the latest stable, e.g. ^1.25.x or newer). - Ensure your test/integration suite covers end-to-end tracing to catch any breaking changes.
- Confirm removal of
@opentelemetry/sdk-nodeis intentional—if you rely on automatic Node SDK initialization, add it back or initialize the tracer manually.
🤖 Prompt for AI Agents
In typescript/packages/fi-core/package.json around lines 28 to 30, the
OpenTelemetry package versions are inconsistent across dependencies and
devDependencies, which can cause runtime API mismatches. To fix this, update all
@opentelemetry/* package versions to the same minor version, preferably the
latest stable such as ^1.25.x, ensuring uniformity. Also verify if the removal
of @opentelemetry/sdk-node is intentional; if automatic Node SDK initialization
is needed, add it back or implement manual tracer initialization. Finally, run
your test and integration suites to confirm tracing functionality remains
intact.
| // 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
Remove unnecessary type assertion.
The type casting tracerProvider as any suggests a type compatibility issue. Consider fixing the underlying type issue instead of using type assertion.
- tracerProvider: tracerProvider as any,
+ tracerProvider: tracerProvider,📝 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/chat.ts at line 23, remove the
unnecessary type assertion 'as any' on tracerProvider. Instead, investigate and
resolve the underlying type compatibility issue by ensuring that tracerProvider
matches the expected type without casting. Adjust the type definitions or the
tracerProvider assignment as needed to align types correctly.
| apiKey: 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 the invalid OpenAI model name.
The model "gpt-4.1-mini" doesn't exist. This should likely be "gpt-4o-mini" or another valid OpenAI model.
- 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/chat.ts at line 34, the model
name "gpt-4.1-mini" is invalid. Replace it with a valid OpenAI model name such
as "gpt-4o-mini" or another correct model identifier to ensure the API call
works properly.
| async function main() { | ||
| const openai = new OpenAI({ | ||
| apiKey: process.env.OPENAI_API_KEY, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing environment variables.
The code doesn't handle the case where OPENAI_API_KEY is missing, which would cause a runtime error.
async function main() {
+ if (!process.env.OPENAI_API_KEY) {
+ console.error("OPENAI_API_KEY environment variable must be set.");
+ process.exit(1);
+ }
+
const openai = new OpenAI({
apiKey: process.env.OPENAI_API_KEY,
});📝 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.
| async function main() { | |
| const openai = new OpenAI({ | |
| apiKey: process.env.OPENAI_API_KEY, | |
| }); | |
| async function main() { | |
| if (!process.env.OPENAI_API_KEY) { | |
| console.error("OPENAI_API_KEY environment variable must be set."); | |
| process.exit(1); | |
| } | |
| const openai = new OpenAI({ | |
| apiKey: process.env.OPENAI_API_KEY, | |
| }); | |
| // ... | |
| } |
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/chat.ts around lines 29 to 32,
the code initializes the OpenAI client using the OPENAI_API_KEY environment
variable without checking if it is set. Add a check before creating the OpenAI
instance to verify that process.env.OPENAI_API_KEY is defined. If it is missing,
throw an informative error or log a clear message and exit the process to
prevent runtime errors caused by an undefined API key.
| "@opentelemetry/exporter-trace-otlp-proto": "^0.50.0", | ||
| "@opentelemetry/resources": "^1.25.1", | ||
| "@opentelemetry/sdk-trace-base": "^1.25.1", | ||
| "@opentelemetry/sdk-trace-node": "^1.25.1", | ||
| "@opentelemetry/semantic-conventions": "^1.25.1" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure devDependencies match runtime dependency versions.
The devDependencies use different version patterns than the main dependencies:
- Main dependencies use older versions (e.g.,
@opentelemetry/api@^1.9.0) - DevDependencies use newer versions (e.g.,
@opentelemetry/resources@^1.25.1)
This version mismatch between dependencies and devDependencies could cause issues during development vs. production.
Consider aligning the OpenTelemetry package versions for consistency:
"dependencies": {
- "@opentelemetry/api": "^1.9.0",
- "@opentelemetry/instrumentation": "^0.46.0",
+ "@opentelemetry/api": "^1.9.0",
+ "@opentelemetry/instrumentation": "^0.52.0",
"@opentelemetry/core": "^1.25.1",
"@traceai/fi-core": "0.1.0",
"@traceai/fi-semantic-conventions": "0.1.0"
},Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/package.json around lines 39 to 43, the
versions of OpenTelemetry packages in devDependencies do not match those in the
main dependencies, causing potential inconsistencies. Update the devDependencies
to use the same version numbers as the main dependencies for all OpenTelemetry
packages to ensure consistency between development and production environments.
| "@opentelemetry/api": "^1.9.0", | ||
| "@opentelemetry/instrumentation": "^0.46.0", | ||
| "@opentelemetry/core": "^1.25.1", | ||
| "@traceai/fi-core": "0.1.0", | ||
| "@traceai/fi-semantic-conventions": "0.1.0" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify OpenTelemetry version compatibility across packages.
The dependency updates show mixed version patterns:
@opentelemetry/apiis at^1.9.0@opentelemetry/instrumentationis at^0.46.0(downgraded from^0.48.0)@opentelemetry/coreis at^1.25.1
This version mismatch could lead to compatibility issues. Additionally, using exact versions (0.1.0) for internal packages removes flexibility for patch updates.
Run the following script to check for OpenTelemetry version consistency recommendations:
🌐 Web query:
What are the recommended version combinations for @opentelemetry/api, @opentelemetry/instrumentation, and @opentelemetry/core packages to ensure compatibility?
💡 Result:
To ensure compatibility among the @opentelemetry/api, @opentelemetry/core, and @opentelemetry/instrumentation packages, it's important to align their versions appropriately. The OpenTelemetry JavaScript project maintains a compatibility matrix to guide users in selecting compatible versions.
As of June 2025, the latest stable versions are:
@opentelemetry/api: 1.9.0@opentelemetry/core: 1.28.0@opentelemetry/instrumentation: 0.55.0
These versions are compatible and should work seamlessly together. It's advisable to use matching versions across these packages to avoid potential issues.
For detailed information on version compatibility and to stay updated with the latest releases, refer to the OpenTelemetry JavaScript GitHub repository. (github.com)
Additionally, the OpenTelemetry JavaScript documentation provides guidance on supported versions and best practices. (opentelemetry.io)
By adhering to these recommendations, you can ensure a stable and compatible setup for your OpenTelemetry instrumentation in JavaScript.
Citations:
- 1: https://github.com/open-telemetry/opentelemetry-js?utm_source=openai
- 2: https://opentelemetry.io/docs/languages/js/?utm_source=openai
Align OpenTelemetry package versions and relax internal package pins
To ensure compatibility, update your OpenTelemetry dependencies to the latest stable, matching versions, and allow patch releases for internal packages:
• Upgrade OpenTelemetry packages:
- Keep
"@opentelemetry/api": "^1.9.0" - Change
"@opentelemetry/instrumentation"→^0.55.0 - Change
"@opentelemetry/core"→^1.28.0
• Relax internal package pins to allow patch updates:
- Change
"@traceai/fi-core"→^0.1.0 - Change
"@traceai/fi-semantic-conventions"→^0.1.0
Suggested diff:
"@opentelemetry/api": "^1.9.0",
- "@opentelemetry/instrumentation": "^0.46.0",
- "@opentelemetry/core": "^1.25.1",
- "@traceai/fi-core": "0.1.0",
- "@traceai/fi-semantic-conventions": "0.1.0"
+ "@opentelemetry/instrumentation": "^0.55.0",
+ "@opentelemetry/core": "^1.28.0",
+ "@traceai/fi-core": "^0.1.0",
+ "@traceai/fi-semantic-conventions": "^0.1.0"📝 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.
| "@opentelemetry/api": "^1.9.0", | |
| "@opentelemetry/instrumentation": "^0.46.0", | |
| "@opentelemetry/core": "^1.25.1", | |
| "@traceai/fi-core": "0.1.0", | |
| "@traceai/fi-semantic-conventions": "0.1.0" | |
| "@opentelemetry/api": "^1.9.0", | |
| "@opentelemetry/instrumentation": "^0.55.0", | |
| "@opentelemetry/core": "^1.28.0", | |
| "@traceai/fi-core": "^0.1.0", | |
| "@traceai/fi-semantic-conventions": "^0.1.0" |
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/package.json around lines 27 to 31, the
OpenTelemetry package versions are inconsistent and internal package versions
are pinned exactly, which can cause compatibility and update issues. Update
"@opentelemetry/instrumentation" to "^0.55.0" and "@opentelemetry/core" to
"^1.28.0" to align with "@opentelemetry/api" at "^1.9.0". Also, change
"@traceai/fi-core" and "@traceai/fi-semantic-conventions" versions from "0.1.0"
to "^0.1.0" to allow patch updates and improve flexibility.
…on and usage instructions, and adding environment variable setup and quickstart guide.
Pull Request
Description
Describe the changes in this pull request:
Checklist
Related Issues
Closes #<issue_number>
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores