Dev #56
Dev #56
Conversation
…n constraints in pyproject.toml and poetry.lock
[Feature] PortKey Integration
Readme Update
[feature] gPRC intergation
Add Transport to exports and update gRPC collector endpoint URL
[Feature] Added Support for Grpc in Typescript
WalkthroughThis update introduces gRPC transport support for OpenTelemetry span exporting in both Python and TypeScript packages, alongside the existing HTTP transport. It adds gRPC exporter implementations, protocol buffer definitions, and generated client/server code. The update also refines build scripts, improves module system support, and adds new Portkey AI instrumentation for Python. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant TracerProvider
participant SpanExporter (HTTP/GRPC)
participant Collector (HTTP/gRPC)
App->>TracerProvider: register({transport: HTTP/GRPC, ...})
TracerProvider->>SpanExporter: instantiate (HTTPSpanExporter or GRPCSpanExporter)
App->>TracerProvider: startSpan(...)
TracerProvider->>SpanExporter: export(span)
alt HTTP Transport
SpanExporter->>Collector: send spans via HTTP
else gRPC Transport
SpanExporter->>Collector: send spans via gRPC (CreateOtelSpan)
end
Possibly related PRs
Poem
✨ 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: 16
🔭 Outside diff range comments (2)
typescript/packages/traceai_anthropic/package.json (1)
17-23:preparescript hard-codes pnpm – will fail fornpm/yarnusers cloning from GitHubWhen the package is installed directly from the Git repository (
npm i github:future-agi/traceAI), thepreparescript runs before dev-deps are installed and beforepnpmis guaranteed to exist, causing installation to blow up.Either:
- Replace with a portable command (
npm run build) and rely on the workspace tooling in the monorepo, or- Gate the script with a check (
[ -x "$(command -v pnpm)" ] && pnpm run build).typescript/packages/fi-core/src/otel.ts (1)
534-569:printTracingDetails()builds the message but never outputs itThe method constructs
detailsMsgyet neither logs nor returns it, so the call in the constructor is a no-op.- let detailsMsg = `${detailsHeader}\n`; + let detailsMsg = `${detailsHeader}\n`; … - detailsMsg += "| \n"; - if (this.defaultProcessorAttached) { - detailsMsg += "| Using a default SpanProcessor. `addSpanProcessor` will overwrite this default.\n"; - } + detailsMsg += "| \n"; + if (this.defaultProcessorAttached) { + detailsMsg += "| Using a default SpanProcessor. `addSpanProcessor` will overwrite this default.\n"; + } + + diag.info(detailsMsg); // or console.logWithout this, users lose the helpful startup diagnostics.
♻️ Duplicate comments (1)
typescript/packages/fi-semantic-conventions/package.json (1)
17-23: Sameprepare-script portability concern as raised fortraceai_anthropicThe package will fail to install from Git for users without
pnpm. See earlier comment for mitigation.
🧹 Nitpick comments (22)
typescript/packages/traceai_openai/tsconfig.esnext.json (1)
6-7: ValidatetsBuildInfoFileandbaseUrlconsistency across packages
Setting a dedicatedtsBuildInfoFilefor ESNext outputs and definingbaseUrltosrcstreamlines builds and imports. Ensure all other TS packages (fi-core,fi-semantic-conventions, etc.) mirror these settings to avoid drift. Consider addingpathsmappings or inline comments to document the import strategy.Run:
#!/bin/bash # Verify compilerOptions in tsconfig.esnext.json across the monorepo grep -R '"tsBuildInfoFile":' -n typescript/packages grep -R '"baseUrl": "./src"' -n typescript/packagespython/fi_instrumentation/settings.py (1)
18-20: Fix return type annotation for gRPC endpoint function.
get_env_grpc_collector_endpointalways returns a string (via default fallback), so the return type can bestrinstead ofOptional[str].typescript/packages/traceai_anthropic/tsconfig.esm.json (1)
7-7: Add baseUrl for consistent module resolution
Setting"baseUrl": "./src"standardizes imports relative to thesrcdirectory. Consider adding correspondingpathsmappings if you’re using custom aliases (e.g.,"@anthropic/*": ["*"]).python/pyproject.toml (1)
31-32: Consider pinning gRPC dependencies.Using version wildcards (
"*") forgrpcioandgrpcio-toolscan lead to non-reproducible builds. Pin to a known-good version or add a minimum version constraint.python/frameworks/portkey/pyproject.toml (1)
12-16: Loosened dependency ranges could propagate breaking changes
portkey-ai = ">0.1.0"andfi-instrumentation-otel = "0.1.7"allow any future 1.x/2.x releases, potentially introducing incompatibilities. Consider adopting caret ranges or upper bounds (e.g.^0.1.0,<0.2) until the APIs stabilise. This is especially relevant for instrumentation packages that rely on specific public surfaces.typescript/packages/traceai_openai/examples/basic-otel-test.ts (1)
37-40: Hard-codingprojectName: "GRPC"may confuse downstream analytics
projectNameis typically a user / service identifier, not a transport mode. Consider reverting to the real service name and letting thetransportfield communicate the channel.typescript/packages/fi-core/README.md (1)
37-40: CommonJS example language tag.
Consider changing the fence fromtypescript tojavascript for therequiresnippet.typescript/packages/fi-semantic-conventions/README.md (1)
24-27: CommonJS snippet language tag.
Changetypescript tojavascript for clarity.typescript/package.json (1)
7-10: Modularized build scripts improve clarity.
Good separation of core vs. consumer builds and explicit ordering.Consider adding a
cleanscript (e.g.,rimraf dist) before builds to avoid stale artifacts.typescript/packages/traceai_openai/examples/check_grpc.ts (2)
11-18: Verify the gRPC endpoint configuration.The endpoint uses
https://scheme with port 50051, which is typically a gRPC port. gRPC usually runs on HTTP/2 without TLS (http://) or with TLS but not using the https:// scheme.Also consider making the endpoint configurable via environment variable for flexibility.
const otlpProcessor = new SimpleSpanProcessor( new GRPCSpanExporter({ - endpoint: "https://grpc.futureagi.com:50051", + endpoint: process.env.GRPC_ENDPOINT || "grpc.futureagi.com:50051", headers: { "X-Api-Key": process.env.FI_API_KEY || "", "X-Secret-Key": process.env.FI_SECRET_KEY || "", }, }) );
118-127: Clean up commented code in error handling.The error handling section contains commented-out code that should either be enabled or removed for clarity.
Either enable the error handling or remove it entirely:
} catch (error) { console.error("Error in main execution:", error); - // Even in error handling, we can work with active spans - // const activeSpan = trace.getActiveSpan(); - // if (activeSpan) { - // activeSpan.recordException(error as Error); - // activeSpan.setStatus({ - // code: 2, - // message: `Main execution failed: ${(error as Error).message}` - // }); - // } + const activeSpan = trace.getActiveSpan(); + if (activeSpan) { + activeSpan.recordException(error as Error); + activeSpan.setStatus({ + code: 2, + message: `Main execution failed: ${(error as Error).message}` + }); + } }typescript/packages/traceai_anthropic/package.json (2)
5-15: Missing"default"and sub-path conditions inexportsThe new
exportsblock only maps the package root. Consumers who deep-import (e.g.@traceai/anthropic/request) will break once you start publishing additional files. It is safer to add a catch-all ("./*": "./dist/esm/*") or enumerate future sub-paths up-front and, for completeness, expose a"default"key to satisfy bundlers that still look for it."exports": { ".": { "types": "./dist/src/index.d.ts", "import": "./dist/esm/index.js", "require": "./dist/src/index.js" + }, + "./*": { + "import": "./dist/esm/*.js", + "require": "./dist/src/*.js" } },
47-50: Unpinned@types/node~v22 may drift ahead of active LTS
@types/node@^22targets Node 22, which is not LTS yet. If the runtime matrix still supports 18/20, pinning to the lowest supported major (e.g.^20) avoids accidental use of future-only APIs during development.python/frameworks/portkey/traceai_portkey/_with_span.py (1)
38-45: Swallowing all exceptions may hide instrumentation failuresCatching bare
Exceptioninsiderecord_exception,add_event, andset_attributesprevents crashes but also masks programmer errors (e.g. wrong attribute types). Consider logging atlogger.warningand re-raising inDEBUGmode or behind an env-flag so that issues surface in tests.typescript/packages/fi-core/package.json (1)
40-55: Large set of heavy runtime deps – consider marking gRPC & protobuf as optional
@grpc/grpc-js,@protobuf-ts/*, etc. inflate install size for users that only need HTTP transport. Marking them under"optionalDependencies"or splitting the gRPC exporter into a secondary package keeps the core lean.typescript/packages/traceai_openai/package.json (1)
38-43: Peer dependency range on@traceai/fi-coreis behind new version
fi-corejust bumped to0.1.12, but the range here is^0.1.11. While semver satisfies it, updating the manifest avoids confusion.python/fi_instrumentation/grpc/tracer_pb2_grpc.py (1)
3-6: Silence linters for generated code, don’t hand-editStatic-analysis warnings (
F401forwarnings, etc.) come from generated code. Prefer adding a one-liner header such as
# flake8: noqa/# ruff: noqato avoid manual edits that will be wiped on regen.python/frameworks/portkey/traceai_portkey/_wrappers.py (1)
27-42: Remove dead code
_flatten()isn’t referenced anywhere in the package—safe to drop to reduce surface area.typescript/packages/fi-core/src/otel.ts (1)
480-483: Operator-precedence makes theverbosedefault hard to readconst verbose = config.verbose ?? getEnv("FI_VERBOSE_PROVIDER")?.toLowerCase() === "true" ?? false;Because
===binds tighter than??, the expression is correct but non-obvious. Wrap the equality in parentheses (or extract to a helper) to avoid future misreads.python/fi_instrumentation/otel.py (3)
8-8: Remove unused import
timeis imported but never used.
532-536: Addstacklevelwhen emitting warnings
warnings.warn(...)defaults to reporting the warning at the exporter's line, not the caller’s.
Addstacklevel=2so users see where they called the exporter.
771-774: Unnecessaryelseafterreturnif isinstance(exporter, _HTTPSpanExporter): return "HTTP" if isinstance(exporter, _GRPCSpanExporter): return "gRPC" else: # <- can be dropped return exporter.__class__.__name__Removing the redundant
elseimproves readability and matches Pylint’s suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
python/poetry.lockis excluded by!**/*.locktypescript/packages/fi-core/src/generated/google/protobuf/struct.tsis excluded by!**/generated/**typescript/packages/fi-core/src/generated/index.tsis excluded by!**/generated/**typescript/packages/fi-core/src/generated/tracer.client.tsis excluded by!**/generated/**typescript/packages/fi-core/src/generated/tracer.tsis excluded by!**/generated/**
📒 Files selected for processing (43)
python/fi_instrumentation/__init__.py(2 hunks)python/fi_instrumentation/fi_types.py(1 hunks)python/fi_instrumentation/grpc/tracer.proto(1 hunks)python/fi_instrumentation/grpc/tracer_pb2.py(1 hunks)python/fi_instrumentation/grpc/tracer_pb2_grpc.py(1 hunks)python/fi_instrumentation/otel.py(12 hunks)python/fi_instrumentation/settings.py(1 hunks)python/frameworks/portkey/README.md(1 hunks)python/frameworks/portkey/pyproject.toml(1 hunks)python/frameworks/portkey/traceai_portkey/__init__.py(1 hunks)python/frameworks/portkey/traceai_portkey/_request_attributes_extractor.py(1 hunks)python/frameworks/portkey/traceai_portkey/_response_attributes_extractor.py(1 hunks)python/frameworks/portkey/traceai_portkey/_utils.py(1 hunks)python/frameworks/portkey/traceai_portkey/_with_span.py(1 hunks)python/frameworks/portkey/traceai_portkey/_wrappers.py(1 hunks)python/frameworks/portkey/traceai_portkey/version.py(1 hunks)python/pyproject.toml(3 hunks)typescript/package.json(1 hunks)typescript/packages/fi-core/README.md(1 hunks)typescript/packages/fi-core/package.json(2 hunks)typescript/packages/fi-core/src/grpc/tracer.proto(1 hunks)typescript/packages/fi-core/src/otel.ts(12 hunks)typescript/packages/fi-core/tsconfig.esm.json(1 hunks)typescript/packages/fi-core/tsconfig.esnext.json(1 hunks)typescript/packages/fi-core/tsconfig.json(1 hunks)typescript/packages/fi-semantic-conventions/README.md(1 hunks)typescript/packages/fi-semantic-conventions/package.json(2 hunks)typescript/packages/fi-semantic-conventions/tsconfig.esm.json(1 hunks)typescript/packages/fi-semantic-conventions/tsconfig.esnext.json(1 hunks)typescript/packages/fi-semantic-conventions/tsconfig.json(1 hunks)typescript/packages/traceai_anthropic/README.md(1 hunks)typescript/packages/traceai_anthropic/package.json(3 hunks)typescript/packages/traceai_anthropic/tsconfig.esm.json(1 hunks)typescript/packages/traceai_anthropic/tsconfig.esnext.json(1 hunks)typescript/packages/traceai_anthropic/tsconfig.json(1 hunks)typescript/packages/traceai_openai/README.md(1 hunks)typescript/packages/traceai_openai/examples/basic-otel-test.ts(3 hunks)typescript/packages/traceai_openai/examples/check_grpc.ts(1 hunks)typescript/packages/traceai_openai/package.json(2 hunks)typescript/packages/traceai_openai/tsconfig.esm.json(1 hunks)typescript/packages/traceai_openai/tsconfig.esnext.json(1 hunks)typescript/packages/traceai_openai/tsconfig.json(1 hunks)typescript/tsconfig.json(1 hunks)
🧰 Additional context used
🪛 Buf (1.54.0)
typescript/packages/fi-core/src/grpc/tracer.proto
3-3: Files with package "tfc.tracer" must be within a directory "tfc/tracer" relative to root but were in directory "typescript/packages/fi-core/src/grpc".
(PACKAGE_DIRECTORY_MATCH)
python/fi_instrumentation/grpc/tracer.proto
3-3: Files with package "tfc.tracer" must be within a directory "tfc/tracer" relative to root but were in directory "python/fi_instrumentation/grpc".
(PACKAGE_DIRECTORY_MATCH)
🪛 Ruff (0.11.9)
python/fi_instrumentation/grpc/tracer_pb2_grpc.py
4-4: warnings imported but unused
Remove unused import: warnings
(F401)
21-21: f-string without any placeholders
Remove extraneous f prefix
(F541)
python/fi_instrumentation/grpc/tracer_pb2.py
25-25: google.protobuf.struct_pb2 imported but unused
Remove unused import: google.protobuf.struct_pb2
(F401)
python/frameworks/portkey/traceai_portkey/_wrappers.py
44-44: _WithTracer is an abstract base class, but it has no abstract methods or properties
(B024)
147-147: return inside finally blocks cause exceptions to be silenced
(B012)
python/fi_instrumentation/otel.py
8-8: time imported but unused
Remove unused import: time
(F401)
533-533: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
🪛 Pylint (3.3.7)
python/fi_instrumentation/grpc/tracer_pb2_grpc.py
[refactor] 28-28: Class 'ObservationSpanControllerStub' inherits from object, can be safely removed from bases in python3
(R0205)
[refactor] 28-28: Too few public methods (0/2)
(R0903)
[refactor] 44-44: Class 'ObservationSpanControllerServicer' inherits from object, can be safely removed from bases in python3
(R0205)
[refactor] 44-44: Too few public methods (1/2)
(R0903)
[refactor] 69-69: Class 'ObservationSpanController' inherits from object, can be safely removed from bases in python3
(R0205)
[refactor] 73-73: Too many arguments (10/5)
(R0913)
[refactor] 73-73: Too many positional arguments (10/5)
(R0917)
[refactor] 69-69: Too few public methods (1/2)
(R0903)
python/fi_instrumentation/grpc/tracer_pb2.py
[error] 9-9: No name 'runtime_version' in module 'google.protobuf'
(E0611)
python/frameworks/portkey/traceai_portkey/_wrappers.py
[error] 67-77: Context manager 'generator' doesn't implement enter and exit.
(E1129)
[refactor] 44-44: Too few public methods (0/2)
(R0903)
[refactor] 97-97: Either all return statements in a function should return an expression, or none of them should.
(R1710)
[refactor] 91-91: Too few public methods (1/2)
(R0903)
python/frameworks/portkey/traceai_portkey/_request_attributes_extractor.py
[refactor] 72-72: Too many branches (13/12)
(R0912)
python/fi_instrumentation/otel.py
[error] 37-39: No name 'grpc' in module 'opentelemetry.exporter.otlp.proto'
(E0611)
[error] 50-50: No name 'Struct' in module 'google.protobuf.struct_pb2'
(E0611)
[refactor] 486-486: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 499-499: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 771-774: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🔇 Additional comments (34)
typescript/packages/fi-semantic-conventions/tsconfig.esm.json (1)
7-7: Align module resolution baseUrl across ESM builds.
Adding"baseUrl": "./src"ensures consistent imports from thesrcdirectory across the monorepo’s ESM packages.typescript/packages/traceai_anthropic/README.md (1)
15-46: Great addition of Module System Support section.
This new documentation clarifies import styles for both CommonJS and ESM, aligning with other TraceAI packages.typescript/packages/traceai_openai/tsconfig.esm.json (1)
7-7: Standardize baseUrl in TypeScript config.
Setting"baseUrl": "./src"here brings it in line with other packages and simplifies module resolution.python/frameworks/portkey/traceai_portkey/version.py (1)
1-1: Verify version alignment with package metadata.
Ensure this__version__ = "0.1.0"matches the version defined inpyproject.tomlfor thetraceAI-portkeypackage.typescript/packages/fi-semantic-conventions/tsconfig.json (1)
5-6: Standardize rootDir and baseUrl
Changing"rootDir"to the project root and adding"baseUrl": "./src"aligns this package with the monorepo convention. Please verify that imports in bothsrc/andtest/folders still resolve correctly under the new configuration.typescript/packages/fi-core/tsconfig.esnext.json (1)
6-7: Align ESNext config with baseUrl addition
Updating thetsBuildInfoFilepath avoids collisions and adding"baseUrl": "./src"brings import consistency. Confirm that your build and aliasing tools (e.g.,tsc-alias) respect this new baseUrl.typescript/packages/traceai_anthropic/tsconfig.esnext.json (1)
6-7: Enable baseUrl for ESNext build
Introducing"baseUrl": "./src"ensures imports resolve fromsrcin the ESNext bundle. Double-check that composite project references and any inter-package imports still resolve as expected.typescript/packages/fi-semantic-conventions/tsconfig.esnext.json (1)
6-7: Integrate baseUrl in ESNext configuration
Adding"baseUrl": "./src"aligns this package with the rest of the monorepo. Validate that any path aliases or post-build scripts are updated to reflect this new base directory.python/fi_instrumentation/fi_types.py (1)
158-161: New attribute addition is correct.The constant
PROMPT_IDwith its docstring is well-placed and follows existing conventions for span attributes.python/pyproject.toml (1)
3-3: Version bump looks good.Updating the project version to
0.1.8aligns with the new feature set.typescript/packages/traceai_openai/tsconfig.json (1)
5-7: TSConfig update is valid.Changing
rootDirto".", addingbaseUrl, and including projectreferencesimproves module resolution and supports incremental builds as intended.Also applies to: 9-12
typescript/packages/fi-core/tsconfig.esm.json (1)
2-10: ESM config update is correct.Adding
baseUrl: "./src"aligns with other package configs and standardizes imports.typescript/packages/fi-core/tsconfig.json (1)
5-6: rootDir “.” + baseUrl “./src” may unintentionally widen the compile scopeWith
rootDirat the package root, every TS/JS file under the directory (e.g. build scripts, generated code,dist, config files) becomes a candidate for compilation/emission unless explicitly excluded. At the same timebaseUrl: "./src"assumes that non-relative imports resolve fromsrc/, so modules that live alongsidesrc(tests, utilities) will not resolve without a relative path.
Please double-check that:
- All non-relative imports really live inside
src/.excludeor stricterincludepatterns prevent pulling indist, generated protobuf, etc.- The composite build (if any) still produces declarations in the expected folder.
If any of the above is a concern, consider reverting
rootDirto"src"or adding explicitexcludepatterns.python/fi_instrumentation/__init__.py (1)
27-28: Publicly exportingTransportlooks goodExposing the enum through
__all__keeps the public API coherent with the new gRPC feature set. No further action required.typescript/packages/traceai_openai/README.md (5)
15-18: Clear Module System heading and intro.
The new “Module System Support” section is concise and informative.
19-24: ESM import example is accurate.
The TypeScript snippet correctly demonstrates ESM usage.
25-29: CommonJS import example is correct.
The JavaScript snippet shows how to userequire.
31-42: TypeScript configuration guidance is spot on.
Recommendedtsconfig.jsonoptions are clear and align with cross–module compatibility.
45-46: Module setting note adds flexibility.
Including guidance on the"module"field is helpful.python/frameworks/portkey/README.md (3)
1-4: Intro and title clearly describe the Portkey instrumentation.
The header and brief description set the context well.
7-12: Requirements and install snippet look good.
Dependencies and pip install instructions are correctly listed.
17-52: Usage example is comprehensive and correct.
Imports, project registration, instrumentation, and client code are clearly demonstrated.typescript/packages/fi-core/README.md (4)
28-31: Module System Support section added.
Heading and description are consistent with other packages.
32-36: ESM usage example is accurate.
Theimportstatement forfi-coreis clear.
42-54: TypeScript compiler options are well defined.
Recommendedtsconfig.jsonsettings ensure interoperability.
56-57: Flexibility note onmodulefield.
Useful reminder for varying project requirements.typescript/packages/fi-semantic-conventions/README.md (4)
9-12: Added yarn and pnpm installation variants.
Provides comprehensive package manager options.
15-18: Module System Support section introduced.
Consistent with ecosystem-wide docs.
19-22: ESM import snippet is correct.
Showcases proper syntax for module imports.
29-41: TypeScript configuration block is accurate.
All recommended compiler options are present.python/fi_instrumentation/grpc/tracer_pb2.py (1)
4-4: ```shell
#!/bin/bashLocate all .proto files in the repository
find . -type f -name '*.proto'
</details> <details> <summary>typescript/packages/fi-core/package.json (2)</summary> `17-21`: **`build:proto` script may fail – `protoc` needs the TS plugin explicitly** `@protobuf-ts/protoc` ships `protoc` but *not* the `protoc-gen-ts` plugin automatically. Typical invocation: ```bash protoc \ --plugin=protoc-gen-ts="$(npm root)/.bin/protoc-gen-ts" \ --ts_out src/generated \ --proto_path src/grpc src/grpc/tracer.protoPlease verify the current command generates code; otherwise CI will fail on a clean clone.
57-65: Dev-only tooling is fine – but double-check they’re hoisted
@protobuf-ts/*dev deps have to be available for thebuild:protoscript executed inprepare. In a non-workspace install this might fail. Confirm the publish flow pre-builds artefacts so consumers aren’t exposed.python/frameworks/portkey/traceai_portkey/_response_attributes_extractor.py (1)
14-18: Good use of shared utilsRe-using
_as_output_attributeskeeps serialization logic consistent.
| { | ||
| "files": [], | ||
| "references": [ | ||
| { "path": "./packages/fi-core" }, | ||
| { "path": "./packages/fi-semantic-conventions" }, | ||
| { "path": "./packages/traceai_openai" }, | ||
| { "path": "./packages/traceai_anthropic" }, | ||
| { "path": "./packages/traceai_langchain" } | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
Missing composite flag for project references.
Tomplate for a composite project must set "compilerOptions": { "composite": true } to enable project references.
Apply this diff:
{
- "files": [],
+ "compilerOptions": {
+ "composite": true
+ },
+ "files": [],
"references": [
{ "path": "./packages/fi-core" },
{ "path": "./packages/fi-semantic-conventions" },
{ "path": "./packages/traceai_openai" },
{ "path": "./packages/traceai_anthropic" },
{ "path": "./packages/traceai_langchain" }
]
}📝 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.
| { | |
| "files": [], | |
| "references": [ | |
| { "path": "./packages/fi-core" }, | |
| { "path": "./packages/fi-semantic-conventions" }, | |
| { "path": "./packages/traceai_openai" }, | |
| { "path": "./packages/traceai_anthropic" }, | |
| { "path": "./packages/traceai_langchain" } | |
| ] | |
| } | |
| { | |
| "compilerOptions": { | |
| "composite": true | |
| }, | |
| "files": [], | |
| "references": [ | |
| { "path": "./packages/fi-core" }, | |
| { "path": "./packages/fi-semantic-conventions" }, | |
| { "path": "./packages/traceai_openai" }, | |
| { "path": "./packages/traceai_anthropic" }, | |
| { "path": "./packages/traceai_langchain" } | |
| ] | |
| } |
🤖 Prompt for AI Agents
In typescript/tsconfig.json lines 1 to 10, the project references are missing
the required "composite": true flag in the compilerOptions. Add a
"compilerOptions" section at the root level of the tsconfig.json with
"composite": true to enable proper project references compilation.
| "rootDir": ".", | ||
| "baseUrl": "./src" | ||
| }, | ||
| "files": [], | ||
| "include": ["src/**/*.ts", "test/**/*.ts"], | ||
| "references": [] | ||
| "references": [ | ||
| { "path": "../fi-core" }, | ||
| { "path": "../fi-semantic-conventions" } | ||
| ] |
There was a problem hiding this comment.
Same rootDir/baseUrl concern and missing composite: true for project references
As with fi-core, widening rootDir to "." risks compiling non-source files, while baseUrl: "./src" expects absolute imports to start inside src/.
Additionally, the new references array implies a composite project, but compilerOptions.composite is absent. TypeScript will error if a referenced project is composite and the referencing one is not.
"compilerOptions": {
"outDir": "dist",
+ "composite": true,
"rootDir": ".",
"baseUrl": "./src"
},Verify that all referenced packages expose tsconfig.json with "composite": true as well.
📝 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.
| "rootDir": ".", | |
| "baseUrl": "./src" | |
| }, | |
| "files": [], | |
| "include": ["src/**/*.ts", "test/**/*.ts"], | |
| "references": [] | |
| "references": [ | |
| { "path": "../fi-core" }, | |
| { "path": "../fi-semantic-conventions" } | |
| ] | |
| "compilerOptions": { | |
| "outDir": "dist", | |
| "composite": true, | |
| "rootDir": ".", | |
| "baseUrl": "./src" | |
| }, | |
| "include": ["src/**/*.ts", "test/**/*.ts"], | |
| "references": [ | |
| { "path": "../fi-core" }, | |
| { "path": "../fi-semantic-conventions" } | |
| ] |
🤖 Prompt for AI Agents
In typescript/packages/traceai_anthropic/tsconfig.json around lines 5 to 12,
adjust the rootDir to a more specific source directory instead of ".", to avoid
compiling non-source files, and ensure baseUrl aligns with this rootDir for
consistent module resolution. Also, add "composite": true to the compilerOptions
since the project uses references, making it a composite project. Finally,
verify that each referenced package's tsconfig.json also has "composite": true
set to maintain compatibility.
| @@ -1,4 +1,4 @@ | |||
| import { register, ProjectType, EvalSpanKind, EvalName, EvalTag, EvalTagType, ModelChoices } from "@traceai/fi-core"; | |||
| import { register, ProjectType, EvalSpanKind, EvalName, EvalTag, EvalTagType, ModelChoices, Transport } from "@traceai/fi-core"; | |||
There was a problem hiding this comment.
Several imported symbols are now unused – breaks with noUnusedLocals
EvalSpanKind, EvalName, EvalTag, EvalTagType, and ModelChoices are only used in the commented-out block. With noUnusedLocals enabled (as in many strict tsconfigs) the example will not compile.
-import { register, ProjectType, EvalSpanKind, EvalName, EvalTag, EvalTagType, ModelChoices, Transport } from "@traceai/fi-core";
+import { register, ProjectType, Transport } from "@traceai/fi-core";Alternatively, uncomment the evalTags section.
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/basic-otel-test.ts at line 1,
several imported symbols (EvalSpanKind, EvalName, EvalTag, EvalTagType, and
ModelChoices) are unused due to being only referenced in commented-out code,
causing compilation errors with noUnusedLocals enabled. To fix this, either
remove these unused imports from the import statement or uncomment the related
evalTags code block that uses them, ensuring all imported symbols are actively
used.
| @@ -0,0 +1,18 @@ | |||
| syntax = "proto3"; | |||
|
|
|||
| package tfc.tracer; | |||
There was a problem hiding this comment.
Fix the package directory mismatch.
The protobuf package tfc.tracer should align with the directory structure. The file is located in typescript/packages/fi-core/src/grpc/ but the package suggests it should be in tfc/tracer/. This mismatch can cause issues with code generation and import paths.
Consider either:
- Change the package to match the current directory structure (e.g.,
fi_core.grpc) - Move the file to the appropriate directory structure matching the package name
🧰 Tools
🪛 Buf (1.54.0)
3-3: Files with package "tfc.tracer" must be within a directory "tfc/tracer" relative to root but were in directory "typescript/packages/fi-core/src/grpc".
(PACKAGE_DIRECTORY_MATCH)
🤖 Prompt for AI Agents
In typescript/packages/fi-core/src/grpc/tracer.proto at line 3, the protobuf
package declaration 'tfc.tracer' does not match the file's directory structure,
which can cause code generation and import issues. To fix this, either update
the package declaration to reflect the current directory structure, such as
'fi_core.grpc', or move the tracer.proto file into a directory path that matches
the package name 'tfc/tracer'.
| @@ -0,0 +1,18 @@ | |||
| syntax = "proto3"; | |||
|
|
|||
| package tfc.tracer; | |||
There was a problem hiding this comment.
Fix the package directory mismatch.
Similar to the TypeScript proto file, the package tfc.tracer doesn't match the directory structure python/fi_instrumentation/grpc/. This should be addressed for consistency.
🧰 Tools
🪛 Buf (1.54.0)
3-3: Files with package "tfc.tracer" must be within a directory "tfc/tracer" relative to root but were in directory "python/fi_instrumentation/grpc".
(PACKAGE_DIRECTORY_MATCH)
🤖 Prompt for AI Agents
In python/fi_instrumentation/grpc/tracer.proto at line 3, the package
declaration `tfc.tracer` does not match the directory structure
`python/fi_instrumentation/grpc/`. Update the package name to reflect the
directory path accurately, ensuring consistency between the package declaration
and the file location.
| if isinstance((tools := invocation_params.pop("tools", None)), Iterable): | ||
| for i, tool in enumerate(tools): | ||
| yield f"llm.tools.{i}.tool.json_schema", safe_json_dumps(tool) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against str mistakenly treated as an iterable of tools
isinstance(tools, Iterable) will accept str, resulting in per-character JSON dumps.
- if isinstance((tools := invocation_params.pop("tools", None)), Iterable):
+ if isinstance((tools := invocation_params.pop("tools", None)), Iterable) and not isinstance(tools, (str, bytes)):📝 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.
| if isinstance((tools := invocation_params.pop("tools", None)), Iterable): | |
| for i, tool in enumerate(tools): | |
| yield f"llm.tools.{i}.tool.json_schema", safe_json_dumps(tool) | |
| if isinstance((tools := invocation_params.pop("tools", None)), Iterable) and not isinstance(tools, (str, bytes)): | |
| for i, tool in enumerate(tools): | |
| yield f"llm.tools.{i}.tool.json_schema", safe_json_dumps(tool) |
🤖 Prompt for AI Agents
In python/frameworks/portkey/traceai_portkey/_request_attributes_extractor.py
around lines 50 to 53, the code checks if tools is an Iterable but does not
exclude str, causing each character of a string to be treated as a separate
tool. Fix this by adding an additional check to ensure tools is not an instance
of str before iterating over it.
| ), | ||
| status=trace_api.Status(status_code=trace_api.StatusCode.OK), | ||
| ) | ||
| except Exception: | ||
| logger.exception(f"Failed to finalize response of type {type(response)}") | ||
| span.finish_tracing() | ||
| finally: | ||
| return response No newline at end of file |
There was a problem hiding this comment.
Let exceptions propagate—avoid return in finally
A return inside finally will silence any exception raised in the preceding block, contradicting OTEL error semantics.
- finally:
- return response
+ finally:
+ # Returning outside the try/finally lets any error bubble up.
+ pass
+ return response📝 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.
| ), | |
| status=trace_api.Status(status_code=trace_api.StatusCode.OK), | |
| ) | |
| except Exception: | |
| logger.exception(f"Failed to finalize response of type {type(response)}") | |
| span.finish_tracing() | |
| finally: | |
| return response | |
| ), | |
| status=trace_api.Status(status_code=trace_api.StatusCode.OK), | |
| ) | |
| except Exception: | |
| logger.exception(f"Failed to finalize response of type {type(response)}") | |
| span.finish_tracing() | |
| finally: | |
| # Returning outside the try/finally lets any error bubble up. | |
| pass | |
| return response |
🧰 Tools
🪛 Ruff (0.11.9)
147-147: return inside finally blocks cause exceptions to be silenced
(B012)
🤖 Prompt for AI Agents
In python/frameworks/portkey/traceai_portkey/_wrappers.py around lines 140 to
147, remove the return statement from the finally block to avoid suppressing
exceptions raised in the try or except blocks. Instead, place the return
statement after the try-except-finally structure so exceptions can propagate
properly while still ensuring span.finish_tracing() is called in the except
block.
| private getClient(): ObservationSpanControllerClient { | ||
| if (!this.client) { | ||
| // Default to insecure credentials (matching Python implementation) | ||
| // Most gRPC servers, even remote ones, often use insecure connections | ||
| const credentials = grpc.credentials.createInsecure(); | ||
|
|
||
| if (this.verbose) { | ||
| diag.info(`GRPCSpanExporter: Connecting to ${this.originalEndpoint} with insecure credentials`); | ||
| } | ||
|
|
||
| const transport = new GrpcTransport({ | ||
| host: this.originalEndpoint, | ||
| channelCredentials: credentials, | ||
| meta: this.customHeaders, | ||
| }); | ||
| this.client = new ObservationSpanControllerClient(transport); | ||
| } | ||
| return this.client; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use secure gRPC credentials for production endpoints
createInsecure() will silently downgrade TLS and fail against most public endpoints that expect TLS ‒ including grpc.futureagi.com:50051.
Either detect localhost/127.0.0.1 to allow insecure channels or use grpc.credentials.createSsl() (optionally reading FI_GRPC_USE_INSECURE) so that production traffic is encrypted.
🤖 Prompt for AI Agents
In typescript/packages/fi-core/src/otel.ts around lines 241 to 259, the code
currently uses grpc.credentials.createInsecure() unconditionally, which causes
TLS to be disabled and fails against most public endpoints. Modify the getClient
method to detect if the endpoint is localhost or 127.0.0.1 and only then use
insecure credentials; otherwise, use grpc.credentials.createSsl() to enable
secure TLS connections. Optionally, add support for an environment variable like
FI_GRPC_USE_INSECURE to override this behavior for testing or development
purposes.
| if (transport === Transport.GRPC) { | ||
| exporter = new GRPCSpanExporter({ endpoint, headers, verbose: config.verbose }); | ||
| } else { | ||
| exporter = new HTTPSpanExporter({ endpoint, headers, verbose: config.verbose }); | ||
| } |
There was a problem hiding this comment.
Pass the computed verbose flag, not the possibly-undefined config.verbose
When transport === Transport.GRPC you do new GRPCSpanExporter({ … verbose: config.verbose });.
If the env variable supplied the verbosity flag, config.verbose is still undefined, so the exporter never sees it. Pass the resolved verbose variable instead.
- exporter = new GRPCSpanExporter({ endpoint, headers, verbose: config.verbose });
+ exporter = new GRPCSpanExporter({ endpoint, headers, verbose });Apply the same change for the HTTP branch.
📝 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.
| if (transport === Transport.GRPC) { | |
| exporter = new GRPCSpanExporter({ endpoint, headers, verbose: config.verbose }); | |
| } else { | |
| exporter = new HTTPSpanExporter({ endpoint, headers, verbose: config.verbose }); | |
| } | |
| if (transport === Transport.GRPC) { | |
| exporter = new GRPCSpanExporter({ endpoint, headers, verbose }); | |
| } else { | |
| exporter = new HTTPSpanExporter({ endpoint, headers, verbose }); | |
| } |
🤖 Prompt for AI Agents
In typescript/packages/fi-core/src/otel.ts around lines 499 to 503, the verbose
flag passed to both GRPCSpanExporter and HTTPSpanExporter is using
config.verbose, which may be undefined if the environment variable was used to
set verbosity. Replace config.verbose with the computed verbose variable in both
exporter constructors to ensure the correct verbosity flag is passed.
|
|
||
| with grpc.insecure_channel(self._endpoint) as channel: | ||
| stub = tracer_pb2_grpc.ObservationSpanControllerStub(channel) | ||
| metadata = list(self._headers) if self._headers else None | ||
|
|
||
| otel_data_list = [] | ||
| for span_data_item in spans_data: | ||
| s = Struct() | ||
| s.update(span_data_item) | ||
| otel_data_list.append(s) | ||
|
|
||
| request = tracer_pb2.CreateOtelSpanRequest( | ||
| otel_data_list=otel_data_list | ||
| ) | ||
| span_exporter = HTTPSpanExporter(endpoint=endpoint, headers=headers) | ||
| super().__init__(span_exporter) | ||
|
|
||
| try: | ||
| stub.CreateOtelSpan(request, metadata=metadata) | ||
| except grpc.RpcError as e: | ||
| print(f"Failed to export spans via gRPC: {e.details()}") | ||
|
|
||
| return SpanExportResult.SUCCESS | ||
|
|
||
| except Exception as e: | ||
| import traceback |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inefficient & insecure gRPC channel handling
grpc.insecure_channelis used for every export – TLS is bypassed and a new TCP connection is created per batch, hurting throughput.- Opening a channel for each call allocates sockets and threads unnecessarily.
Refactor to:
• Reuse a single channel/stub kept on the exporter instance.
• Use grpc.secure_channel with proper roots unless an explicit FI_GRPC_USE_INSECURE flag is set.
🤖 Prompt for AI Agents
In python/fi_instrumentation/otel.py around lines 580 to 603, the code
inefficiently creates a new insecure gRPC channel for each export call, which
bypasses TLS and causes overhead by opening new TCP connections repeatedly.
Refactor by initializing a single gRPC channel and stub as instance variables in
the exporter class constructor, reusing them for all export calls. Replace
grpc.insecure_channel with grpc.secure_channel using proper root certificates
unless an explicit FI_GRPC_USE_INSECURE flag is set to allow insecure
connections. This will improve security and performance by reusing connections
and enabling TLS by default.
Pull Request
Description
Describe the changes in this pull request:
Checklist
Related Issues
Closes #<issue_number>
Summary by CodeRabbit
New Features
Enhancements
Documentation
Chores