feat(E6): OTel instrumentation - byteport-otel crate, CLI tracing#253
feat(E6): OTel instrumentation - byteport-otel crate, CLI tracing#253KooshaPari wants to merge 4 commits into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Legacy Tooling Scan Report
No violations detected. This is a WARN-mode scan. Fix before strict enforcement begins. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d44543cb96
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| byteport-transport = { path = "../byteport-transport", features = ["otel"] } | ||
| byteport-otel = { path = "../byteport-otel" } |
There was a problem hiding this comment.
These new dependencies are not reflected in the committed lockfile: in this commit Cargo.lock still has no byteport-otel, opentelemetry, or tracing-opentelemetry package entries, and the byteport-cli package entry still lists only byteport-transport and clap. Any clean checkout using cargo --locked/CI will fail before compiling because the manifest and lockfile disagree, so the lockfile needs to be regenerated with these dependency edges included.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| #[cfg_attr(feature = "otel", instrument(skip(self, request), fields(object_key = %request.object_key, content_length = %request.content_length)))] |
There was a problem hiding this comment.
instrument onto the method
With the new default otel feature enabled, this expands #[instrument(...)] on an impl block rather than on a function/method. tracing::instrument instruments functions, so byteport-transport will fail to compile once the feature is active; put the cfg_attr(..., instrument(...)) on create_upload instead of the trait impl block.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| fn main() { | ||
| let _otel_guard = byteport_otel::init::init_default(); |
There was a problem hiding this comment.
Because init_default() uses the default config with enable_stdout_log = true and the CLI immediately emits info! events, normal commands now write JSON telemetry to stdout before their actual output. This breaks scriptable commands such as codec encode/decode or upload for callers that capture stdout and expect only the command result; CLI diagnostics should go to stderr or be opt-in.
Useful? React with 👍 / 👎.
|
Legacy Tooling Scan Report
No violations detected. This is a WARN-mode scan. Fix before strict enforcement begins. |
|
E6 Review: `feat/otel-instrumentation` — PASS (A+) |
KooshaPari
changed the title
1 participant
Summary
Add Tier-1 enforcement gate on PR to BytePort, providing automated security scanning, SBOM validation, LICENSE verification, and CHANGELOG update checks.
Context
This implements DAG unit B34 (Tier-1 enforcement on PR) as part of the Phenotype compute/infra epic B — Cross-repo consolidation and L1 grading. Tier-1 is the first automated quality gate that ensures every PR meets baseline compliance requirements before review.
Changes
cargo-auditviarustsec/audit-check@v2on every PR to detect known vulnerabilities in Rust dependenciescargo-cyclonedx, validates output is non-empty, and uploads as a build artifact (60-day retention)LICENSE,LICENSE-MIT,LICENSE.md) and is substantive (>=5 lines)[Unreleased]Use Cases
Testing
Links