Skip to content

Conversation

@NVJKKartik
Copy link
Contributor

@NVJKKartik NVJKKartik commented Jun 17, 2025

Pull Request

Description

Describe the changes in this pull request:

  • What feature/bug does this PR address?
  • Provide any relevant links or screenshots.

Checklist

  • Code compiles correctly.
  • Created/updated tests.
  • Linting and formatting applied.
  • Documentation updated.

Related Issues

Closes #<issue_number>

Summary by CodeRabbit

  • New Features

    • Added support for exporting OpenTelemetry spans over gRPC transport in both Python and TypeScript libraries.
    • Introduced new configuration options to select between HTTP and gRPC span export.
    • Provided new gRPC-based span exporters and corresponding protobuf definitions for span data transfer.
    • Added example scripts demonstrating gRPC tracing setup and usage.
    • Exposed Transport enum for easy selection of export transport in Python package.
  • Chores

    • Integrated protobuf and gRPC code generation and dependencies into the TypeScript build process.
    • Updated project dependencies to include gRPC and OpenTelemetry gRPC exporters.

@NVJKKartik NVJKKartik requested a review from sarthakFuture June 17, 2025 06:57
@coderabbitai
Copy link

coderabbitai bot commented Jun 17, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This change introduces gRPC-based OpenTelemetry span exporting to both Python and TypeScript codebases. It adds protobuf service definitions, generated gRPC client/server code, new exporter classes, and updates span registration and provider logic to support a transport option for HTTP or gRPC. Example scripts, build steps, and dependency configurations are updated accordingly.

Changes

File(s) Change Summary
python/fi_instrumentation/grpc/tracer.proto,
typescript/packages/fi-core/src/grpc/tracer.proto
Added gRPC protobuf definitions for ObservationSpanController service with CreateOtelSpan RPC and messages.
python/fi_instrumentation/grpc/tracer_pb2.py,
python/fi_instrumentation/grpc/tracer_pb2_grpc.py
Added auto-generated Python protobuf and gRPC stub files for the new service and messages.
python/fi_instrumentation/otel.py Added GRPCSpanExporter, updated providers/processors to support transport selection between HTTP and gRPC.
python/fi_instrumentation/settings.py Added function to retrieve gRPC collector endpoint from environment.
testing_grpc.py New script demonstrating gRPC-based span export using the updated instrumentation.
typescript/packages/fi-core/package.json Added gRPC/protobuf build steps and dependencies for TypeScript, integrated codegen into build process.
typescript/packages/fi-core/src/otel.ts Added GRPCSpanExporter, transport selection logic, and endpoint handling for gRPC in the tracing provider.
typescript/packages/traceai_openai/examples/basic-otel-test.ts Updated example to use gRPC transport and changed project registration options.
typescript/packages/traceai_openai/examples/check_grpc.ts New example demonstrating OpenTelemetry tracing with gRPC exporter and OpenAI integration.
python/pyproject.toml Added gRPC and OpenTelemetry gRPC exporter dependencies for Python.
python/fi_instrumentation/init.py Exported Transport enum from the package root for public use.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant TracerProvider
    participant Exporter (HTTP/GRPC)
    participant gRPCServer

    App->>TracerProvider: register({ transport: HTTP/GRPC })
    App->>TracerProvider: getTracer()
    App->>TracerProvider: startSpan()
    App->>TracerProvider: endSpan()
    TracerProvider->>Exporter (HTTP/GRPC): export(span)
    alt HTTP
        Exporter (HTTP/GRPC)->>HTTP Collector: POST /spans
        HTTP Collector-->>Exporter (HTTP/GRPC): Response
    else GRPC
        Exporter (HTTP/GRPC)->>gRPCServer: CreateOtelSpan(otel_data_list)
        gRPCServer-->>Exporter (HTTP/GRPC): CreateOtelSpanResponse(ids)
    end
    Exporter (HTTP/GRPC)-->>TracerProvider: Export result
Loading

Poem

A hop and a skip, with gRPC we go,
Spans leap through the wire, in structured proto flow.
From Python to TypeScript, the exporters unite,
Tracing our journeys by day and by night.
With endpoints and channels, the bunnies now cheer—
Observability’s magic is finally here!
🐇✨


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

🧹 Nitpick comments (6)
testing_grpc.py (3)

1-3: Unused os import

os is never referenced – drop it to keep Ruff happy.

-import os
 from dotenv import load_dotenv
 load_dotenv()

12-15: Extraneous f-string & cosmetic project name

The f prefix does nothing here and triggers Ruff F541.

-    project_name=f"Hack -2",
+    project_name="Hack -2",

18-18: Unused imports

Status and StatusCode are imported but never used.

-from opentelemetry.trace.status import Status, StatusCode
typescript/packages/fi-core/package.json (1)

7-10: Document proto build prerequisite

protoc must be present in CI/dev machines or the build will fail.
Consider adding "protoc" as a devDependency via @protobuf-ts/protoc CLI wrapper or document the requirement in CONTRIBUTING.

typescript/packages/fi-core/src/otel.ts (1)

709-721: Type-unsafe cast leaks internal API

(tracerProvider as any).endpoint reaches into private members.
Expose a typed getter on FITracerProvider instead to avoid brittle any casts.

python/fi_instrumentation/otel.py (1)

793-796: Simplify conditional logic.

Remove unnecessary else after return statements.

 def _exporter_transport(exporter: SpanExporter) -> str:
     if isinstance(exporter, _HTTPSpanExporter):
         return "HTTP"
     if isinstance(exporter, _GRPCSpanExporter):
         return "gRPC"
-    else:
-        return exporter.__class__.__name__
+    return exporter.__class__.__name__
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42a3f94 and 191b76a.

⛔ Files ignored due to path filters (4)
  • typescript/packages/fi-core/src/generated/google/protobuf/struct.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/index.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/tracer.client.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/tracer.ts is excluded by !**/generated/**
📒 Files selected for processing (11)
  • 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)
  • testing_grpc.py (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/traceai_openai/examples/basic-otel-test.ts (3 hunks)
  • typescript/packages/traceai_openai/examples/check_grpc.ts (1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.26.0)
typescript/packages/traceai_openai/examples/check_grpc.ts

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 Buf (1.54.0)
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)

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)

🪛 Ruff (0.11.9)
python/fi_instrumentation/grpc/tracer_pb2.py

25-25: google.protobuf.struct_pb2 imported but unused

Remove unused import: google.protobuf.struct_pb2

(F401)

testing_grpc.py

1-1: os imported but unused

Remove unused import: os

(F401)


12-12: f-string without any placeholders

Remove extraneous f prefix

(F541)


18-18: opentelemetry.trace.status.Status imported but unused

Remove unused import

(F401)


18-18: opentelemetry.trace.status.StatusCode imported but unused

Remove unused import

(F401)

python/fi_instrumentation/otel.py

535-535: No explicit stacklevel keyword argument found

Set stacklevel=2

(B028)

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)

🪛 Pylint (3.3.7)
python/fi_instrumentation/grpc/tracer_pb2.py

[error] 9-9: No name 'runtime_version' in module 'google.protobuf'

(E0611)

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] 487-487: Consider using '{}' instead of a call to 'dict'.

(R1735)


[refactor] 501-501: Consider using '{}' instead of a call to 'dict'.

(R1735)


[refactor] 526-526: Too many local variables (24/15)

(R0914)


[refactor] 793-796: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

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)

🔇 Additional comments (7)
typescript/packages/traceai_openai/examples/basic-otel-test.ts (1)

37-40: Ensure env var alignment when switching to gRPC transport

You now set transport: Transport.GRPC but the script still relies on FI_BASE_URL in the logs.
Confirm that register() reads FI_GRPC_URL (not FI_BASE_URL) or pass an explicit grpcEndpoint here; otherwise the exporter may hit the wrong host.

python/fi_instrumentation/grpc/tracer_pb2.py (1)

1-42: Skip generated file from lint

The file is protobuf-generated; suppress Ruff/Pylint for this path instead of editing the code.

python/fi_instrumentation/grpc/tracer_pb2_grpc.py (1)

1-98: Exclude generated gRPC stubs from lint

Same rationale as tracer_pb2.py; update lint config instead of touching generated code.

typescript/packages/fi-core/src/otel.ts (1)

255-260: Header injection may be ignored

GrpcTransport’s constructor doesn’t expose a meta option in current @protobuf-ts/grpc-transport (it expects mergeCallOptions).
Please verify that authentication headers are actually transmitted; otherwise switch to per-call metadata:

client.createOtelSpan(request, { meta: customHeaders })
python/fi_instrumentation/otel.py (3)

65-68: LGTM! Clean enum implementation.

The Transport enum provides a clear way to specify the transport method for span exporting.


82-82: Good backward-compatible API design.

Adding the transport parameter with a default value maintains backward compatibility.


811-818: Verify endpoint normalization logic for gRPC.

The function now always applies HTTP endpoint construction via _construct_http_endpoint, even when called for gRPC transport. This might not be appropriate for gRPC endpoints.

#!/bin/bash
# Description: Check how _normalized_endpoint is used in the codebase

# Check all usages of _normalized_endpoint
rg -A 5 '_normalized_endpoint'

# Check if gRPC endpoints go through this normalization
ast-grep --pattern 'get_env_grpc_collector_endpoint()'

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (5)
typescript/packages/fi-core/src/grpc/tracer.proto (1)

3-3: Package declaration mismatch with directory structure.

The package tfc.tracer; conflicts with the file path typescript/packages/fi-core/src/grpc. Buf and protoc require the package name to map directly to the directory hierarchy (e.g., tfc/tracer). Please either update the package to reflect this path or move the file under tfc/tracer relative to the repo root, then regenerate the TypeScript stubs.

python/fi_instrumentation/otel.py (4)

357-365: Debug print should be removed or gated behind a logger.

The "~~~~~~ endpoint" statement pollutes stdout in production and was already flagged earlier.


534-535: Add stacklevel=2 to warning for correct caller context.

Same issue noted previously; still unresolved.

-            warnings.warn("gRPC Exporter endpoint is not configured. Cannot export spans.")
+            warnings.warn(
+                "gRPC Exporter endpoint is not configured. Cannot export spans.",
+                stacklevel=2,
+            )

582-584: gRPC channel is always insecure – make security configurable.

Sending auth headers over an insecure channel exposes credentials. Support TLS based on endpoint scheme or env flag.


603-606: Replace traceback prints with structured logging.

Raw print(traceback.format_exc()) hampers observability and mixes with stdout. Use logger.exception instead.

🧹 Nitpick comments (8)
typescript/packages/fi-core/src/grpc/tracer.proto (1)

7-9: Prefer semicolon syntax for empty RPC methods.

When no RPC options are needed, it’s more idiomatic and concise to use the semicolon form:

rpc CreateOtelSpan(CreateOtelSpanRequest) returns (CreateOtelSpanResponse);
testing_grpc.py (4)

1-3: Remove unused os import.

os is imported but never referenced. Clean-up lowers cognitive load and silences Ruff F401.

-import os

12-12: Drop superfluous f-string prefix.

The literal "Hack -2" contains no placeholders; the f prefix is unnecessary and flagged by Ruff F541.

-    project_name=f"Hack -2",
+    project_name="Hack -2",

18-18: Prune unused Status / StatusCode imports.

Neither symbol is referenced. Delete to avoid dead code warnings.

-from opentelemetry.trace.status import Status, StatusCode

20-27: Flush spans before exit for deterministic export.

The script exits immediately after the loop; pending spans may remain in exporters’ queues (especially when batching). Consider an explicit shutdown/flush:

# after the loop
trace_provider.shutdown()  # or tracer.provider.shutdown()
print("done")
python/fi_instrumentation/otel.py (3)

8-8: Remove unused time import.

time isn’t referenced after previous timing logs were deleted. Drop it to silence Ruff F401.

-import time

222-225: parsed_url is computed but never used.

_normalized_endpoint returns (parsed_url, endpoint) yet only endpoint is consumed. Eliminate the unused variable or use it.

-            parsed_url, endpoint = _normalized_endpoint(endpoint)
-            exporter: SpanExporter = HTTPSpanExporter(endpoint=endpoint)
+            _, endpoint = _normalized_endpoint(endpoint)
+            exporter: SpanExporter = HTTPSpanExporter(endpoint=endpoint)

770-775: Unreachable else after returns.

if/return chains make the final else redundant. Simplify:

-    if isinstance(exporter, _HTTPSpanExporter):
-        return "HTTP"
-    if isinstance(exporter, _GRPCSpanExporter):
-        return "gRPC"
-    else:
-        return exporter.__class__.__name__
+    if isinstance(exporter, _HTTPSpanExporter):
+        return "HTTP"
+    if isinstance(exporter, _GRPCSpanExporter):
+        return "gRPC"
+    return exporter.__class__.__name__
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 191b76a and c74bceb.

⛔ Files ignored due to path filters (5)
  • python/poetry.lock is excluded by !**/*.lock
  • typescript/packages/fi-core/src/generated/google/protobuf/struct.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/index.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/tracer.client.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/tracer.ts is excluded by !**/generated/**
📒 Files selected for processing (9)
  • python/fi_instrumentation/otel.py (12 hunks)
  • python/fi_instrumentation/settings.py (1 hunks)
  • python/pyproject.toml (2 hunks)
  • testing_grpc.py (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/traceai_openai/examples/basic-otel-test.ts (3 hunks)
  • typescript/packages/traceai_openai/examples/check_grpc.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • python/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (5)
  • typescript/packages/traceai_openai/examples/check_grpc.ts
  • python/fi_instrumentation/settings.py
  • typescript/packages/traceai_openai/examples/basic-otel-test.ts
  • typescript/packages/fi-core/package.json
  • typescript/packages/fi-core/src/otel.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
testing_grpc.py (2)
python/fi_instrumentation/otel.py (1)
  • Transport (65-67)
python/fi_instrumentation/instrumentation/_spans.py (1)
  • set_input (51-59)
🪛 Ruff (0.11.9)
python/fi_instrumentation/otel.py

8-8: time imported but unused

Remove unused import: time

(F401)


534-534: No explicit stacklevel keyword argument found

Set stacklevel=2

(B028)

testing_grpc.py

1-1: os imported but unused

Remove unused import: os

(F401)


12-12: f-string without any placeholders

Remove extraneous f prefix

(F541)


18-18: opentelemetry.trace.status.Status imported but unused

Remove unused import

(F401)


18-18: opentelemetry.trace.status.StatusCode imported but unused

Remove unused import

(F401)

🪛 Pylint (3.3.7)
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] 487-487: Consider using '{}' instead of a call to 'dict'.

(R1735)


[refactor] 500-500: Consider using '{}' instead of a call to 'dict'.

(R1735)


[refactor] 772-775: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

🪛 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)

🔇 Additional comments (1)
typescript/packages/fi-core/src/grpc/tracer.proto (1)

1-2: Proto definitions look correct.

The syntax declaration, import of google/protobuf/struct.proto, and the message definitions align with the Python implementation and are valid for TS gRPC code generation.

Also applies to: 5-6, 11-17

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
typescript/packages/fi-core/src/grpc/tracer.proto (1)

3-3: Fix package-directory mismatch to keep buf/protoc happy.

package tfc.tracer; requires the file to live under tfc/tracer (Buf PACKAGE_DIRECTORY_MATCH). Either:

  1. Change the declaration to reflect the actual path:
-package tfc.tracer;
+package fi_core.grpc;
  1. Or move the file to tfc/tracer/tracer.proto and regenerate stubs.

Not resolving this blocks code-gen and CI.

🧹 Nitpick comments (4)
testing_grpc.py (3)

1-3: Remove unused os import.

os is imported but never referenced, triggering Ruff F401 and adding noise.

-import os
-from dotenv import load_dotenv
-load_dotenv()
+from dotenv import load_dotenv
+load_dotenv()

11-14: Drop the stray f-string prefix.

project_name=f"Hack -2" has no interpolation; the f is unnecessary (Ruff F541).

-    project_name=f"Hack -2",
+    project_name="Hack -2",

18-18: Prune unused OpenTelemetry status imports.

Status and StatusCode are never used (Ruff F401). Delete the import or wire it into span error handling.

-from opentelemetry.trace.status import Status, StatusCode
typescript/packages/fi-core/src/grpc/tracer.proto (1)

7-9: Consider defining request/response at the top-level package.

Placing messages in the same package as the service avoids future naming collisions if multiple services are added.

No action required now—flagging for awareness.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c74bceb and d9319ab.

⛔ Files ignored due to path filters (4)
  • typescript/packages/fi-core/src/generated/google/protobuf/struct.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/index.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/tracer.client.ts is excluded by !**/generated/**
  • typescript/packages/fi-core/src/generated/tracer.ts is excluded by !**/generated/**
📒 Files selected for processing (8)
  • python/fi_instrumentation/__init__.py (2 hunks)
  • python/fi_instrumentation/settings.py (1 hunks)
  • testing_grpc.py (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/traceai_openai/examples/basic-otel-test.ts (3 hunks)
  • typescript/packages/traceai_openai/examples/check_grpc.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • python/fi_instrumentation/init.py
🚧 Files skipped from review as they are similar to previous changes (5)
  • typescript/packages/traceai_openai/examples/basic-otel-test.ts
  • python/fi_instrumentation/settings.py
  • typescript/packages/traceai_openai/examples/check_grpc.ts
  • typescript/packages/fi-core/package.json
  • typescript/packages/fi-core/src/otel.ts
🧰 Additional context used
🪛 Ruff (0.11.9)
testing_grpc.py

1-1: os imported but unused

Remove unused import: os

(F401)


12-12: f-string without any placeholders

Remove extraneous f prefix

(F541)


18-18: opentelemetry.trace.status.Status imported but unused

Remove unused import

(F401)


18-18: opentelemetry.trace.status.StatusCode imported but unused

Remove unused import

(F401)

🪛 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)

🔇 Additional comments (1)
testing_grpc.py (1)

20-27: Ensure spans flush before process exit.

Because batching is enabled, the program may terminate before the exporter drains its queue, losing spans. Add an explicit flush/shutdown after the loop.

 for i in range(2):
     ...
 
-print("done")
+trace_provider.force_flush()  # or `.shutdown()` if supported
+print("done (flushed)")

@NVJKKartik NVJKKartik changed the base branch from main to dev June 18, 2025 07:46
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
typescript/packages/traceai_openai/examples/check_grpc.ts (1)

50-98: Spans leak on errors – wrap work in try … finally

If openai.chat.completions.create (or any other awaited call) throws, execution jumps past
grandchildSpan.end() / childSpan.end() / parentSpan.end() leaving spans open.

-const grandchildSpan = tracer.startSpan(/* … */);
-
-await context.with(trace.setSpan(context.active(), grandchildSpan), async () => {
-    // work …
-});
-grandchildSpan.end();
+const grandchildSpan = tracer.startSpan(/* … */);
+try {
+    await context.with(trace.setSpan(context.active(), grandchildSpan), async () => {
+        // work …
+    });
+} finally {
+    grandchildSpan.end();
+}

Do the same for childSpan and parentSpan loops to guarantee closure.
This also satisfies previous review feedback.

python/fi_instrumentation/otel.py (1)

581-598: Authentication sent over grpc.insecure_channel

The exporter always creates an insecure channel yet attaches auth headers,
exposing secrets on the wire.
Please switch to grpc.secure_channel when the endpoint is TLS (or make it configurable).

🧹 Nitpick comments (3)
typescript/packages/traceai_openai/examples/check_grpc.ts (1)

10-18: Externalise gRPC endpoint

Hard-coding the collector URL makes local / staging testing harder.
Expose it via an env-var with a sensible default.

-        endpoint: "https://grpc.futureagi.com:50051",
+        endpoint: process.env.FI_GRPC_ENDPOINT ?? "https://grpc.futureagi.com:50051",
python/fi_instrumentation/otel.py (2)

533-534: Add stacklevel=2 to warning

Without stacklevel, the warning message points here instead of the caller,
making debugging harder.

-            warnings.warn("gRPC Exporter endpoint is not configured. Cannot export spans.")
+            warnings.warn(
+                "gRPC Exporter endpoint is not configured. Cannot export spans.",
+                stacklevel=2,
+            )

768-774: Unreachable else branch

return already exits the function; the else is dead code.

-    if isinstance(exporter, _HTTPSpanExporter):
-        return "HTTP"
-    if isinstance(exporter, _GRPCSpanExporter):
-        return "gRPC"
-    else:
-        return exporter.__class__.__name__
+    if isinstance(exporter, _HTTPSpanExporter):
+        return "HTTP"
+    if isinstance(exporter, _GRPCSpanExporter):
+        return "gRPC"
+    return exporter.__class__.__name__
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba6cc9b and 7d22251.

📒 Files selected for processing (2)
  • python/fi_instrumentation/otel.py (12 hunks)
  • typescript/packages/traceai_openai/examples/check_grpc.ts (1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
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/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 (1)
python/fi_instrumentation/otel.py (1)

8-8: Remove unused import time

time is imported but never referenced.
Delete to keep the module tidy.
[ suggest_nitpick ]

Comment on lines +20 to +27
const provider = new NodeTracerProvider({
resource: resourceFromAttributes({
"project_name": "test_project_2_grpc",
"project_type": "observe",

}),
spanProcessors: [otlpProcessor],
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Invalid spanProcessors key – processor is silently ignored

NodeTracerProvider’s config type ( NodeTracerConfig / TracerConfig ) does not contain a spanProcessors field, so the exporter you create here is never registered.
As a result, no spans will be sent.

-const provider = new NodeTracerProvider({
-    resource: resourceFromAttributes({
-        "project_name": "test_project_2_grpc",
-        "project_type": "observe",
-
-    }),
-    spanProcessors: [otlpProcessor],
-});
+
+const provider = new NodeTracerProvider({
+    resource: resourceFromAttributes({
+        project_name: "test_project_2_grpc",
+        project_type: "observe",
+    }),
+});
+
+provider.addSpanProcessor(otlpProcessor);
📝 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.

Suggested change
const provider = new NodeTracerProvider({
resource: resourceFromAttributes({
"project_name": "test_project_2_grpc",
"project_type": "observe",
}),
spanProcessors: [otlpProcessor],
});
const provider = new NodeTracerProvider({
resource: resourceFromAttributes({
project_name: "test_project_2_grpc",
project_type: "observe",
}),
});
provider.addSpanProcessor(otlpProcessor);
🤖 Prompt for AI Agents
In typescript/packages/traceai_openai/examples/check_grpc.ts around lines 20 to
27, the NodeTracerProvider is incorrectly configured with a non-existent
'spanProcessors' key, causing the span processor to be ignored and no spans to
be sent. To fix this, remove the 'spanProcessors' field from the constructor
options and instead register the span processor explicitly by calling
provider.addSpanProcessor(otlpProcessor) after creating the provider instance.

@sarthakFuture sarthakFuture merged commit 913b58b into dev Jun 18, 2025
1 check passed
@coderabbitai coderabbitai bot mentioned this pull request Jun 18, 2025
Merged
4 tasks
@nik13 nik13 deleted the feature/ts-grpc branch July 1, 2025 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants