Otel proto

[nikhilsinhaparseable]

Summary by CodeRabbit

• Chores

  • Updated OpenTelemetry monitoring dependency to track upstream main branch so it follows latest releases.

• Bug Fixes

  • When encountering unsupported string-indexed attribute values, the system now emits a clear warning message instead of failing silently.

[coderabbitai]

Walkthrough

Switches opentelemetry-proto to track the official repository main branch and adds logging for an unsupported OtelValue::StringValueStrindex variant; test fixtures updated to include key_strindex on KeyValue entries.

Changes

Cohort / File(s)	Summary
Dependency Configuration Cargo.toml	Changed opentelemetry-proto source from a pinned git revision on a fork to branch = "main" on the official open-telemetry/opentelemetry-rust repo; features unchanged (gen-tonic, with-serde, logs, metrics, trace).
OTEL JSON helpers src/otel/otel_utils.rs	Added handling for OtelValue::StringValueStrindex in collect_json_from_value and collect_json_from_array_value: emits tracing::warn! and avoids producing JSON for this variant.
Traces tests / fixtures src/otel/traces.rs	Updated test fixtures to set key_strindex: 0 on opentelemetry_proto::tonic::common::v1::KeyValue entries for span/sample and resource attributes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: stringify otel attributes of array types #1298 — Modifies src/otel/otel_utils.rs (array/key-value collection helpers), likely related to the OtelValue::StringValueStrindex handling change.
• Protobuf ingestion #1391 — Also updates opentelemetry-proto dependency source and adjusts OTEL KeyValue test data; closely related to the dependency and test fixture changes.

🐰 I sniffed the diff, then gave a hop,
A branch swapped out, a gentle plop,
Warnings whisper where strings can't go,
Tests tuned up to match the flow,
Tiny changes — a celebratory crop! 🎉

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is completely empty, missing all required sections including description of goals, rationale, key changes, testing checklist, and documentation updates.	Add a comprehensive description following the template: explain what changes were made (dependency migration, StringValueStrindex handling, test fixtures), why these changes were made, and confirm testing and documentation requirements.
Title check	❓ Inconclusive	The title 'Otel proto' is vague and generic, failing to clearly describe the specific changes made (dependency source switch, StringValueStrindex handling, and test fixture updates).	Use a more descriptive title that clarifies the main objective, such as 'Switch opentelemetry-proto to official repository and handle StringValueStrindex variant'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Cargo.toml (1)

99-106: ⚠️ Potential issue | 🟠 Major

Non-reproducible builds: pin to a specific commit or tag instead of tracking main.

Using branch = "main" makes builds non-reproducible—different builds at different times will pull different commits, potentially introducing breaking changes silently. This is especially risky for a dependency that generates proto types used extensively throughout the codebase (logs, traces, metrics modules).

Additionally, the comment on line 99 ("add custom branch with fix until it gets merged") is now stale since this points to the official repository's main branch, not a custom fork with a fix. Either update the comment or, preferably, pin to a specific commit hash.

🔧 Suggested fix

-# add custom branch with fix until it gets merged
-opentelemetry-proto = { git = "https://github.com/open-telemetry/opentelemetry-rust/", branch = "main", features = [
+opentelemetry-proto = { git = "https://github.com/open-telemetry/opentelemetry-rust/", rev = "<COMMIT_SHA>", features = [
     "gen-tonic",
     "with-serde",
     "logs",
     "metrics",
     "trace",
 ] }

Replace <COMMIT_SHA> with the specific commit hash you've tested against. Alternatively, consider using a released version via crates.io (latest: 0.31.0) if compatible with your use case.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` around lines 99 - 106, The dependency declaration for
opentelemetry-proto currently tracks branch = "main" which makes builds
non-reproducible and the nearby comment ("add custom branch with fix until it
gets merged") stale; update the Cargo.toml to pin opentelemetry-proto to a
specific commit SHA (or a released crates.io version like 0.31.0) instead of
branch="main", and update the comment to note the chosen commit or version and
why it was selected so future maintainers know it was intentionally pinned;
locate the opentelemetry-proto entry in Cargo.toml and replace branch="main"
with a rev = "<COMMIT_SHA>" (or version = "0.31.0") and adjust the inline
comment accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@Cargo.toml`:
- Around line 99-106: The dependency declaration for opentelemetry-proto
currently tracks branch = "main" which makes builds non-reproducible and the
nearby comment ("add custom branch with fix until it gets merged") stale; update
the Cargo.toml to pin opentelemetry-proto to a specific commit SHA (or a
released crates.io version like 0.31.0) instead of branch="main", and update the
comment to note the chosen commit or version and why it was selected so future
maintainers know it was intentionally pinned; locate the opentelemetry-proto
entry in Cargo.toml and replace branch="main" with a rev = "<COMMIT_SHA>" (or
version = "0.31.0") and adjust the inline comment accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ecc32678-b380-4b51-a1d6-bee37f80f5e0

📥 Commits

Reviewing files that changed from the base of the PR and between f761b2d and 1d52341.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (1)

• Cargo.toml

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/otel/otel_utils.rs`:
- Around line 69-73: The handler for OtelValue::StringValueStrindex currently
only logs a warning and drops the value, which loses map entries and array
slots; instead, preserve output shape by emitting a placeholder scalar
containing the unresolved string-index (e.g., a string like "<str_index: N>" or
"string_index:N") wherever OtelValue::StringValueStrindex is matched so map
entries and array elements remain present; update both matching branches for
OtelValue::StringValueStrindex to construct and emit that placeholder scalar
into the same container (key-value list or array) rather than skipping,
referencing the OtelValue::StringValueStrindex match arms to locate the changes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4a8fd787-1032-4849-b028-b82853dc4085

📥 Commits

Reviewing files that changed from the base of the PR and between 1d52341 and 54a3db7.

📒 Files selected for processing (2)

• src/otel/otel_utils.rs
• src/otel/traces.rs