bazel build //cmd/otel:otel_lib working now #2240
No reviewers
Labels
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
carverauto/serviceradar!2240
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refs/pull/2240/head"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Imported from GitHub pull request.
Original GitHub pull request: #1660
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1660
Original created: 2025-09-24T23:54:54Z
Original updated: 2025-09-25T00:21:25Z
Original head: carverauto/serviceradar:bug/otel_let
Original base: main
Original merged: 2025-09-25T00:21:14Z by @mfreeman451
PR Type
Bug fix
Description
Fix Rust let-chain syntax for Bazel compatibility
Refactor nested if-let patterns to separate statements
Update RPM spec to filter vendor artifacts by architecture
Diagram Walkthrough
File Walkthrough
lib.rs
Fix let-chain syntax in trace processingcmd/otel/src/lib.rs
main.rs
Fix let-chain syntax in configuration loadingcmd/otel/src/main.rs
serviceradar-kong.spec
Filter vendor artifacts by architecturepackaging/specs/serviceradar-kong.spec
Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1660#issuecomment-3331099498
Original created: 2025-09-24T23:55:30Z
PR Reviewer Guide 🔍
Here are some key observations to aid the review process:
Attribute Map Lifetime
Inserting
attr.key.as_str()ands.as_str()intospan_attrsassumes the map stores borrowed str with lifetimes tied toattr/s. Ifspan_attrsisHashMap<String,String>this will not compile; if it’sHashMap<&str,&str>ensure the references remain valid beyond the loop and through later usage.Silent Errors
Multiple nested ifs silently ignore parse/IO errors (KV connect/get/parse). Consider logging at least debug/warn to aid troubleshooting when config cannot be fetched or parsed.
Arch Pattern Matching
The case pattern
*.%{kong_vendor_arch}.rpmrelies on exact_target_cpumatching vendor filenames. Validate compatibility for common values (e.g., x86_64 vs amd64) to avoid skipping valid artifacts.Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1660#issuecomment-3331100585
Original created: 2025-09-24T23:56:23Z
PR Code Suggestions ✨
Explore these optional code suggestions:
✅
Avoid redundant network calls for configurationSuggestion Impact:
The commit refactored the logic to use a single kv.get(CONFIG_PATH).await within a match, eliminating the redundant second get and adding error logging; it also slightly restructured connection handling.code diff:
Refactor the key-value store logic to use a single
kv.get()call followed by amatchstatement. This avoids a redundant network request when checking for andbootstrapping the configuration.
cmd/otel/src/main.rs [25-43]
[Suggestion processed]Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies a redundant network call to the key-value store and proposes an efficient solution using a
matchstatement, which improves performance and code clarity.Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1660#issuecomment-3331150633
Original created: 2025-09-25T00:21:25Z
CI Feedback 🧐
A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
Action: build
Failed stage: Test [❌]
Failure summary:
Bazel failed during analysis because the opam toolchain setup (OCaml 4.14.2 switch) could not be
created inside the CI sandbox. The critical error is:
- In
/home/runner/.bazel/external/tools_opam+/extensions/opam/opam_toolchain_xdg.bzl:242:17, theextension called
fail()with: "opam switch create failed".- The underlying opam failure shows
bwrap: loopback: Failed RTM_NEWADDR: Operation not permittedwhile compilingocaml-base-compiler.4.14.2, indicating bubblewrap/network namespace operation is not permitted inthe GitHub runner environment.
- Due to the failed module extension
@@tools_opam+//extensions:opam.bzl%opam, many targets could not be analyzed, leading to "commandsucceeded, but not all targets were analyzed" and ultimately "No test targets were found, yet
testing was requested".
- Key log lines:
-
/home/runner/.bazel/external/tools_opam+/extensions/opam/opam_toolchain_xdg.bzl:242:17- Error in
fail: opam switch create failed; rc=31; output shows
bwrap: loopback: Failed RTM_NEWADDR: Operationnot permitted.Relevant error logs: