Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade OTel SDK version to 1.30.0 #2375

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patcher9
Copy link
Contributor

@patcher9 patcher9 commented Mar 15, 2025

Hi,

I am the maintainer of OpenLIT and we recently have had some issues raised in our repo. With our alignment with the OpenTelemetry community, We have started to use Events API in our SDK but when integrating OpenLIT with CrewAI it fails as CrewAI doesnt yet support latest OTel SDKs versions (that have Events API)

ref https://github.com/openlit/openlit/issues

@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #2375

Overview

This pull request updates the OpenTelemetry (OTel) SDK dependencies in the project's pyproject.toml file from version 1.22.0 to 1.30.0. This change is significant as it ensures that the application benefits from the latest improvements and bug fixes released in the OpenTelemetry SDK.

Code Quality Findings

1. Version Update Assessment

The proposed version updates are consistently applied to all OpenTelemetry components:

- "opentelemetry-api>=1.22.0",
- "opentelemetry-sdk>=1.22.0",
- "opentelemetry-exporter-otlp-proto-http>=1.22.0",
+ "opentelemetry-api>=1.30.0",
+ "opentelemetry-sdk>=1.30.0",
+ "opentelemetry-exporter-otlp-proto-http>=1.30.0",

Positive Aspects:

  • ✅ The upgrade promotes uniformity across related components of OpenTelemetry.
  • ✅ Retains the flexible version constraint with >=, providing access to future updates.
  • ✅ Aligns with best practices by moving to a more stable version that likely includes numerous fixes and enhancements.

2. Suggestions for Improvement

2.1 Version Pinning

Using >= can create compatibility issues if future versions introduce breaking changes. Thus, I recommend tightening the version constraints:

dependencies = [
    "opentelemetry-api>=1.30.0,<2.0.0",
    "opentelemetry-sdk>=1.30.0,<2.0.0",
    "opentelemetry-exporter-otlp-proto-http>=1.30.0,<2.0.0",
]

2.2 Changelog Documentation

It's vital to document the changes associated with the update to inform future maintainers:

# OpenTelemetry SDK 1.30.0 upgrade notes:
# - Includes performance improvements
# - Enhanced compatibility with latest instrumentation

2.3 Testing Dependencies

To ensure that relevant functionalities work as expected following the upgrade, consider adding testing dependencies:

[tool.poetry.group.test.dependencies]
opentelemetry-test-utils = ">=1.30.0"

Historical Context from Related PRs

Although I couldn't fetch related PRs due to access issues, understanding patterns from past updates is essential for dependence on OpenTelemetry:

  • Similar dependencies updates have been managed carefully in previous PRs, emphasizing the importance of thorough testing and documentation.

Implications for Related Files

Given the context around telemetry and monitoring configurations:

  1. Verify additional documentation covering OpenTelemetry setup and nuances between versions, especially in configuration settings.
  2. Ensure related instrumentation packages are aligned with the new version of the OpenTelemetry SDK.

Additional Suggestions

  1. Documentation Update: Ensure all aspects of OpenTelemetry usage in the project documentation are updated to reflect the latest changes and any new features.
  2. Migration Guide: Publish a migration guide for users upgrading from older versions to highlight changes and configuration adjustments necessary.
  3. Integration Testing: Run integration tests thoroughly to prevent regressions due to new features or changes in behavior resulting from the update.

Conclusion

The pull request presents a well-structured and sound upgrade plan for OpenTelemetry SDK dependencies. However, applying more specific version constraints, documenting changes clearly, and ensuring comprehensive testing will bolster the project's maintainability and stability.

Final Recommendation

Approve with suggestions:

  • Implement the suggested version constraints.
  • Add documentation updates reflecting the changes in OpenTelemetry.
  • Ensure thorough testing before merging to maintain functionality and stability.

By addressing the recommendations made, this PR will enhance the quality and robustness of the project.

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.

2 participants