Skip to content

Conversation

@yczhang-nv
Copy link
Contributor

@yczhang-nv yczhang-nv commented Apr 30, 2025

Description

This PR makes opentelemetry and phoenix as optional dependencies for Omniverse integration.

  • Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by uv pip install -e '.[telemetry]'.
  • Created src/aiq/utils/optional_imports.py to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
  • When the configuration file contains tracing section but the telemetry dependencies are not installed, TelemetryOptionalImportError will be raised
  • Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
  • Updated logging and documentation

Closes AIQ-1064

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@yczhang-nv yczhang-nv added improvement Improvement to existing functionality non-breaking Non-breaking change labels Apr 30, 2025
@yczhang-nv yczhang-nv requested a review from Copilot April 30, 2025 00:03
@yczhang-nv yczhang-nv self-assigned this Apr 30, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

A PR to make opentelemetry and phoenix dependencies optional, allowing the system to gracefully degrade functionality when these external dependencies are absent.

  • Introduces an optional_imports helper module to centralize optional dependency handling.
  • Updates observability and builder components to use dummy fallback implementations when opentelemetry or phoenix are not available.
  • Adjusts dependency definitions in pyproject.toml and updates installation instructions accordingly.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/aiq/utils/optional_imports.py Adds a centralized mechanism for handling optional imports.
src/aiq/observability/register.py Updates telemetry exporters to use optional imports and fallback logic.
src/aiq/observability/async_otel_listener.py Implements dummy fallback classes for OpenTelemetry when missing.
src/aiq/cli/type_registry.py Incorporates optional import handling for opentelemetry components.
src/aiq/builder/workflow_builder.py Adjusts telemetry setup with optional import fallbacks.
src/aiq/builder/workflow.py Updates usage of opentelemetry with fallback support.
pyproject.toml Moves opentelemetry and phoenix from main dependencies to an extras group.
examples/agents/rewoo/README.md Updates CLI instructions to include syncing of all extras.

@yczhang-nv yczhang-nv requested a review from Copilot April 30, 2025 05:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes OpenTelemetry and Phoenix optional dependencies, ensuring that the AIQ Toolkit works even when these packages are not installed. The changes include implementing safe optional imports, updating telemetry exporter registration and usage, reorganizing dependency declarations in pyproject.toml, and updating documentation to reflect the optional nature of these dependencies.

  • Introduced an optional_imports module to handle optional imports.
  • Updated telemetry exporter functions in observability modules (register.py, async_otel_listener.py, type_registry.py, workflow_builder.py, workflow.py).
  • Moved OpenTelemetry and Arize Phoenix packages to an optional extra ("telemetry") in pyproject.toml and updated related documentation.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/aiq/utils/optional_imports.py New utility functions to safely import optional modules with fallbacks.
src/aiq/observability/register.py Updated telemetry exporter registration to conditionally import modules.
src/aiq/observability/async_otel_listener.py Switched to optional OpenTelemetry imports with dummy fallbacks.
src/aiq/cli/type_registry.py & workflow_builder.py Updated to use optional import pattern for telemetry modules.
pyproject.toml Moved telemetry related dependencies to an optional extra.
docs/source/concepts/observability.md Revised installation instructions to reflect optional telemetry.
examples/agents/rewoo/README.md Updated installation command to sync all groups and extras.

Signed-off-by: Yuchen Zhang <[email protected]>
Copy link
Collaborator

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

Main improvement this PR needs is to localize the imports and to throw errors in invalid configurations (i.e. when users request telemetry but its not installed)

@yczhang-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit da38c41 into NVIDIA:develop May 2, 2025
10 checks passed
@yczhang-nv yczhang-nv deleted the yuchen-make-otel-dep-optional branch May 2, 2025 23:02
yczhang-nv added a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request May 8, 2025
This PR makes `opentelemetry` and `phoenix` as optional dependencies for Omniverse integration.

- Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by `uv pip install -e '.[telemetry]'`.
- Created `src/aiq/utils/optional_imports.py` to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
- When the configuration file contains `tracing` section but the telemetry dependencies are not installed, `TelemetryOptionalImportError` will be raised
- Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
- Updated logging and documentation

Closes [AIQ-1064](https://jirasw.nvidia.com/browse/AIQ-1064)

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: NVIDIA#167
Signed-off-by: Yuchen Zhang <[email protected]>
yczhang-nv added a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request May 9, 2025
This PR makes `opentelemetry` and `phoenix` as optional dependencies for Omniverse integration.

- Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by `uv pip install -e '.[telemetry]'`.
- Created `src/aiq/utils/optional_imports.py` to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
- When the configuration file contains `tracing` section but the telemetry dependencies are not installed, `TelemetryOptionalImportError` will be raised
- Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
- Updated logging and documentation

Closes [AIQ-1064](https://jirasw.nvidia.com/browse/AIQ-1064)

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: NVIDIA#167
Signed-off-by: Yuchen Zhang <[email protected]>
ericevans-nv pushed a commit to ericevans-nv/agent-iq that referenced this pull request Jun 3, 2025
This PR makes `opentelemetry` and `phoenix` as optional dependencies for Omniverse integration.

- Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by `uv pip install -e '.[telemetry]'`.
- Created `src/aiq/utils/optional_imports.py` to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
- When the configuration file contains `tracing` section but the telemetry dependencies are not installed, `TelemetryOptionalImportError` will be raised
- Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
- Updated logging and documentation

Closes [AIQ-1064](https://jirasw.nvidia.com/browse/AIQ-1064)

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: NVIDIA#167
Signed-off-by: Eric Evans <[email protected]>
ericevans-nv pushed a commit to ericevans-nv/agent-iq that referenced this pull request Jun 3, 2025
This PR makes `opentelemetry` and `phoenix` as optional dependencies for Omniverse integration.

- Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by `uv pip install -e '.[telemetry]'`.
- Created `src/aiq/utils/optional_imports.py` to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
- When the configuration file contains `tracing` section but the telemetry dependencies are not installed, `TelemetryOptionalImportError` will be raised
- Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
- Updated logging and documentation

Closes [AIQ-1064](https://jirasw.nvidia.com/browse/AIQ-1064)

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: NVIDIA#167
Signed-off-by: Eric Evans <[email protected]>
AnuradhaKaruppiah pushed a commit to AnuradhaKaruppiah/oss-agentiq that referenced this pull request Aug 4, 2025
This PR makes `opentelemetry` and `phoenix` as optional dependencies for Omniverse integration.

- Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by `uv pip install -e '.[telemetry]'`.
- Created `src/aiq/utils/optional_imports.py` to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
- When the configuration file contains `tracing` section but the telemetry dependencies are not installed, `TelemetryOptionalImportError` will be raised
- Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
- Updated logging and documentation

Closes [AIQ-1064](https://jirasw.nvidia.com/browse/AIQ-1064)

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: NVIDIA#167
scheckerNV pushed a commit to scheckerNV/aiq-factory-reset that referenced this pull request Aug 22, 2025
This PR makes `opentelemetry` and `phoenix` as optional dependencies for Omniverse integration.

- Moved the telemetry dependencies from AIQ core dependency to a separate group of optional dependency, which can be installed by `uv pip install -e '.[telemetry]'`.
- Created `src/aiq/utils/optional_imports.py` to include optional import functionalities and dummy implementations. This module can be reused if we want to add more optional dependencies in the future.
- When the configuration file contains `tracing` section but the telemetry dependencies are not installed, `TelemetryOptionalImportError` will be raised
- Updated files that imports the optional dependencies to gracefully handle the case when those dependencies are not installed
- Updated logging and documentation

Closes [AIQ-1064](https://jirasw.nvidia.com/browse/AIQ-1064)

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: NVIDIA#167
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement to existing functionality non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants