Refactor Telemetry Initialization and Environment Utilities #811
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Reorder functions in envs.rs by type size (8-bit to 64-bit, signed before unsigned) and add missing variants like get_env_opt_u16. - Optimize init_telemetry to support three modes: stdout logging (default error level with span tracing), file rolling logs (size-based with retention), and HTTP-based observability with sub-endpoints (trace, metric, log) falling back to unified endpoint. - Fix stdout logging issue by retaining WorkerGuard in OtelGuard to prevent premature release of async writer threads. - Enhance observability mode with HTTP protocol, compression, and proper resource management. - Update OtelGuard to include tracing_guard for stdout and flexi_logger_handles for file logging. - Improve error handling and configuration extraction in OtelConfig.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF ScorecardScorecard details
Scanned Files
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the observability and telemetry infrastructure to support HTTP-based OpenTelemetry Protocol (OTLP) exporters with separate endpoint configuration for traces, metrics, and logs. The changes include reorganizing environment variable utilities, simplifying telemetry initialization logic, and updating Docker configurations for the observability stack.
Key Changes:
- Migration from gRPC to HTTP-based OTLP exporters with Zstandard compression
- Separate endpoint configuration support for traces, metrics, and logs
- Refactored environment variable utilities with expanded type support (moved from
sys/envsto root-levelenvsmodule) - Simplified telemetry initialization with three distinct modes: observability (HTTP export), file logging, and stdout logging
- Enhanced audit system with OpenTelemetry metrics instrumentation
Reviewed Changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/run.sh | Updated OTLP endpoint configuration examples with separate trace/metric/log endpoints and HTTP port (4318) |
| crates/utils/src/envs.rs | New comprehensive environment variable utility functions supporting multiple numeric types and Option types |
| crates/utils/src/sys/mod.rs | Removed envs module (relocated to root) |
| crates/utils/src/lib.rs | Updated exports to use root-level envs module instead of sys/envs |
| crates/obs/src/telemetry.rs | Major refactoring: split init into three modes (HTTP observability, file logging, stdout), added HTTP OTLP exporters with compression |
| crates/obs/src/config.rs | Added trace/metric/log endpoint fields, simplified env var parsing using new utility functions |
| crates/obs/src/system/mod.rs | Updated to use configurable system metrics interval from environment variables |
| crates/obs/src/metrics/*.rs | Updated import paths from crate::metrics:: to crate:: |
| crates/config/src/observability/mod.rs | Added new endpoint environment variables and metrics configuration constants |
| crates/audit/src/observability.rs | Added OpenTelemetry metrics instrumentation with counter/gauge/histogram support |
| Cargo.toml | Changed opentelemetry-otlp features from gRPC to HTTP with zstd compression |
| .docker/observability/*.yml | Updated Docker images to latest versions and enhanced Prometheus/Grafana configurations with OTLP support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 44 out of 45 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of Change
Related Issues
#796 #802
Summary of Changes
This PR refactors the telemetry system in
crates/obs/src/telemetry.rsand related utilities incrates/utils/src/envs.rsto improve modularity, fix bugs, and enhance functionality. The changes address three primary logging modes with prioritized rules, ensure proper resource lifecycle management, and add support for HTTP-based observability with sub-endpoints.Key Changes:
Environment Functions Ordering (
crates/utils/src/envs.rs):get_env_opt_u16,get_env_opt_u32) for consistency.Telemetry Initialization (
crates/obs/src/telemetry.rs):with_current_spanandwith_span_list).WorkerGuardinOtelGuardto keep async writer threads alive.OtelGuardto managetracing_guardfor stdout andflexi_logger_handlesfor file logging, ensuring RAII-compliant shutdown.Configuration and Dependencies (
crates/obs/src/config.rsandCargo.toml):OtelConfigextraction to handle sub-endpoints correctly.opentelemetry-otlpsupport HTTP features.Motivation:
Testing:
Breaking Changes:
Related Issues:
Checklist
make pre-commitImpact
Additional Notes
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.