Bazel/create pkg #2255

Merged

mfreeman451 merged 10 commits from refs/pull/2255/head into main

2025-10-02 01:59:26 +00:00

mfreeman451 commented

2025-10-02 00:08:49 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1677
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1677
Original created: 2025-10-02T00:08:49Z
Original updated: 2025-10-02T01:59:30Z
Original head: carverauto/serviceradar:bazel/create_pkg
Original base: main
Original merged: 2025-10-02T01:59:26Z by @mfreeman451

PR Type

Enhancement

Description

Implement comprehensive Bazel packaging system for ServiceRadar components
Add eBPF profiler build system with dynamic object loading
Update Docker RBE executor with enhanced toolchain support
Integrate Rust workspace for rperf, sysmon, and profiler components

Diagram Walkthrough

flowchart LR
  A["Bazel Packaging"] --> B["Package Rules"]
  A --> C["Component Configs"]
  D["eBPF Profiler"] --> E["Build Script"]
  D --> F["Dynamic Loading"]
  G["Docker RBE"] --> H["Toolchain Updates"]
  I["Rust Workspace"] --> J["Multiple Crates"]

File Walkthrough

Relevant files

Enhancement

32 files

package_rules.bzl `Create comprehensive Bazel packaging macro system`	+235/-0
build.rs `Implement eBPF object build and loading system`	+81/-2
ebpf_profiler.rs `Update eBPF program loading with dynamic path resolution`	+4/-5
Dockerfile.rbe `Enhance RBE executor with Rust and eBPF toolchains`	+30/-4
BUILD.bazel `Simplify core package using new macro system`	+4/-146
BUILD.bazel `Add Bazel build configuration for rperf checker`	+39/-0
BUILD.bazel `Add Bazel build configuration for sysmon checker`	+37/-0
BUILD.bazel `Add profiler common library build configuration`	+14/-0
BUILD.bazel `Implement profiler binary build with eBPF integration`	+45/-0
BUILD.bazel `Create agent package using macro system`	+9/-0
BUILD.bazel `Create CLI package using macro system`	+9/-0
BUILD.bazel `Create dusk-checker package using macro system`	+9/-0
BUILD.bazel `Create event-writer package using macro system`	+9/-0
BUILD.bazel `Create faker package using macro system`	+9/-0
BUILD.bazel `Create flowgger package using macro system`	+9/-0
BUILD.bazel `Create kong package using macro system`	+9/-0
BUILD.bazel `Create kv package using macro system`	+9/-0
BUILD.bazel `Create mapper package using macro system`	+9/-0
BUILD.bazel `Create nats package using macro system`	+9/-0
BUILD.bazel `Create otel package using macro system`	+9/-0
BUILD.bazel `Create poller package using macro system`	+9/-0
BUILD.bazel `Create profiler package using macro system`	+9/-0
BUILD.bazel `Create proton package using macro system`	+9/-0
BUILD.bazel `Create rperf-checker package using macro system`	+9/-0
BUILD.bazel `Create rperf package using macro system`	+9/-0
BUILD.bazel `Create snmp-checker package using macro system`	+9/-0
BUILD.bazel `Create srql package using macro system`	+9/-0
BUILD.bazel `Create sync package using macro system`	+9/-0
BUILD.bazel `Create sysmon package using macro system`	+9/-0
BUILD.bazel `Create trapd package using macro system`	+9/-0
BUILD.bazel `Create web package using macro system`	+9/-0
BUILD.bazel `Create zen package using macro system`	+9/-0

Configuration changes

7 files

packages.bzl `Define packaging metadata for all ServiceRadar components`	+818/-0
Cargo.toml `Include rperf and sysmon in workspace members`	+0/-4
BUILD.bazel `Update RBE executor image version reference`	+3/-3
BUILD.bazel `Create profiler package exports and alias`	+11/-0
BUILD.bazel `Export profiler proto files for build system`	+2/-0
BUILD.bazel `Export sysmon proto files for build system`	+4/-0
BUILD.bazel `Define packaging build configuration settings`	+11/-0

Dependencies

1 files

MODULE.bazel
Add Rust crate repositories for profiler components +24/-1

Imported from GitHub pull request. Original GitHub pull request: #1677 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1677 Original created: 2025-10-02T00:08:49Z Original updated: 2025-10-02T01:59:30Z Original head: carverauto/serviceradar:bazel/create_pkg Original base: main Original merged: 2025-10-02T01:59:26Z by @mfreeman451 --- ### **PR Type** Enhancement ___ ### **Description** - Implement comprehensive Bazel packaging system for ServiceRadar components - Add eBPF profiler build system with dynamic object loading - Update Docker RBE executor with enhanced toolchain support - Integrate Rust workspace for rperf, sysmon, and profiler components ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Bazel Packaging"] --> B["Package Rules"] A --> C["Component Configs"] D["eBPF Profiler"] --> E["Build Script"] D --> F["Dynamic Loading"] G["Docker RBE"] --> H["Toolchain Updates"] I["Rust Workspace"] --> J["Multiple Crates"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Enhancement</td><td><details><summary>32 files</summary><table> <tr> <td>package_rules.bzl<dd><code>Create comprehensive Bazel packaging macro system</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-3876e101ca949a98325d3dea0371c361af54a507bc700eca353418d22f2d029d">+235/-0</a>  </td> </tr> <tr> <td>build.rs<dd><code>Implement eBPF object build and loading system</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-8b543b27bc19aeba636edcd34b59c29b3816007d059cd43cd251485f86738fbf">+81/-2</a>    </td> </tr> <tr> <td>ebpf_profiler.rs<dd><code>Update eBPF program loading with dynamic path resolution</code>  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-65777d967e1beb73c969136c256d46b88b3af66d3b86d5896ca23e0fac27259a">+4/-5</a>      </td> </tr> <tr> <td>Dockerfile.rbe<dd><code>Enhance RBE executor with Rust and eBPF toolchains</code>              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-40936cbae5822a0a5fa8016befa08eb3a7836c93328e8043dcdfb3885a6201b2">+30/-4</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Simplify core package using new macro system</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-d375384323093ea1f35b57fb77433b4bbc98e26074cb44928d9c0e65644ae96b">+4/-146</a>  </td> </tr> <tr> <td>BUILD.bazel<dd><code>Add Bazel build configuration for rperf checker</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-4a15e4d3ef48348a6039522ddb9a0fc2ca5f1ee56ea80a51de4d2a8100a5cc32">+39/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Add Bazel build configuration for sysmon checker</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-6ab6e69dfb3cd621d100077fa496690634adb5fcd88806f891575024f1835480">+37/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Add profiler common library build configuration</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-941aea21a72c2cb1fd728aed674b766e8da518883358ac516c0b76334359bd78">+14/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Implement profiler binary build with eBPF integration</code>        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-aa5c19161b6b0ff3eaf8e42eaadc9f7a565937f73e40ef061acd18210b072a43">+45/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create agent package using macro system</code>                                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-6527c44ad0baa72b49e3ba46f2d3c1059cf1f2f1839c3250114c1eb7cad1a506">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create CLI package using macro system</code>                                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-ce84c3e01e5f2502882b3d704103769d552bb9cd57faae85243b8ccbaceff0f8">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create dusk-checker package using macro system</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-6e5a3e4871339af1ca093193617a29249c112b8dfdfbb4276b05f11fc6f0c0ba">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create event-writer package using macro system</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-637bea372685cd214af9410ba13e812556ec102bd6523b0e1a8e841eccac58a5">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create faker package using macro system</code>                                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-12243b15ff27c1a939f221d4306e67bae458c41e69bccb2231a098ab481c1040">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create flowgger package using macro system</code>                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-58647ec3764211a2568a16dd5cd9f353c92debfb1516b6142ad678238bf2c472">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create kong package using macro system</code>                                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-ed30439eb82d4b24c455ad038dee0d27a0b5b126be2516355db13877c8a91d73">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create kv package using macro system</code>                                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-a41d7e7fe6eec2a9530f429622a2af905a99a2c8fea69f6581d917dbf52bf602">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create mapper package using macro system</code>                                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-00a527487288c9e59dcaf130caffc697fbebad06b4ff8e105579ab2dd8a78bb6">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create nats package using macro system</code>                                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-eca032c884b0007fd6581f4e75791bbe8072cad315b1e9151b4351491b46b9a3">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create otel package using macro system</code>                                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-4c548fb662d9e34d676a47a3839af1c32dc3fce13e03fe82188679e4bb84d20d">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create poller package using macro system</code>                                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-8cb47636cd4ebcef6d5fdae10955b37141e77b3fcb8bfb3f8cda4413538fe40a">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create profiler package using macro system</code>                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-68eec45264ea1575211ce7065193b506611dfdacf9dca7344c8af6d4c3467dd1">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create proton package using macro system</code>                                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-c70ede39e0625f97495046136ec598e94965e8ba8b4d14d81c1724f38201c79c">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create rperf-checker package using macro system</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-529e23c496597ff4befa7ce81fe904b1375625fdb9a94b2c2ac0dda1c542bfb1">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create rperf package using macro system</code>                                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-1bcdb2e14299a94b346b90b80f88608126540ea34a5bb8299cc6c90183ea1ae1">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create snmp-checker package using macro system</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-3b9c66bce83ca84468b2bfc976ce6046dd04864bcaef206fd94e9fa769437c5c">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create srql package using macro system</code>                                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-6b589982f4e8cfb404d4b5e5aec512e68e29164251d98807df6b29266d99a788">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create sync package using macro system</code>                                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-3cdf1b2a783c5547cdd7eb1d837945d090739f321a2daed445087e1e31692032">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create sysmon package using macro system</code>                                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-841e48997b26f0364ca621dd13d18f7f6b9ac0611c7cf73be0660a3dfe430ff4">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create trapd package using macro system</code>                                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-292254d1667b73273a9a03be82d597f166479d7a51003f0a57d42e1b752a834d">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create web package using macro system</code>                                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-420c787b13a0c75591cf45d6e31626a1d1809a4225c927931a29806f0913f90c">+9/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create zen package using macro system</code>                                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-606abf4e0f1dfdbf3a6eadcc6ea060f936137109a1d25241bd8537d55ce358b6">+9/-0</a>      </td> </tr> </table></details></td></tr><tr><td>Configuration changes</td><td><details><summary>7 files</summary><table> <tr> <td>packages.bzl<dd><code>Define packaging metadata for all ServiceRadar components</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-9bfe2a5141a9e402bb5a5a8fca53b9eea64396ec18108c535392e1054c90b913">+818/-0</a>  </td> </tr> <tr> <td>Cargo.toml<dd><code>Include rperf and sysmon in workspace members</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542">+0/-4</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Update RBE executor image version reference</code>                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-7fc57714ef13c3325ce2a1130202edced92fcccc0c6db34a72f7b57f60d552a3">+3/-3</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Create profiler package exports and alias</code>                                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-97a9259cd922a95e1845b47b64c7fd7c0ea2fa56a7988ca4d6e89a2116b3cc36">+11/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Export profiler proto files for build system</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-dc372981c580a9d68559e21394080f7e824cbe2237cc8508f9dd5c2d70d56b56">+2/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Export sysmon proto files for build system</code>                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-2346a623a60f467f6754b43761b090fc9e47684cd26200715ee855e6348b28e4">+4/-0</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Define packaging build configuration settings</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-b86f28397b1f44e79fc32b86430460bb81842aed46396c96ac5dd35430dde98f">+11/-0</a>    </td> </tr> </table></details></td></tr><tr><td>Dependencies</td><td><details><summary>1 files</summary><table> <tr> <td>MODULE.bazel<dd><code>Add Rust crate repositories for profiler components</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdc">+24/-1</a>    </td> </tr> </table></details></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-10-02 00:10:09 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1677#issuecomment-3358572749
Original created: 2025-10-02T00:10:09Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Build command execution Description: The build script invokes an external 'cargo build' with a toolchain-modifying flag and environment manipulation, which if attacker-controlled environment variables or PATH are present could execute unexpected binaries during build; ensure build environments are trusted and paths are sanitized. build.rs [64-81] Referred Code `let status = Command::new("cargo") .current_dir("..") .env_remove("RUSTUP_TOOLCHAIN") .args(&args) .status(); match status { Ok(status) if status.success() => { let profile_dir = if release { "release" } else { "debug" }; let artifact = Path::new("..") .join("target") .join("bpfel-unknown-none") .join(profile_dir) .join("profiler"); copy_ebpf(&artifact, dest) } Ok(status) => Err(format!("cargo {:?} exited with status {}", args, status).into()), Err(err) => Err(Box::new(err)),`
	Insecure file permissions Description: Packaging rule allows arbitrary file installation with owner/group/mode from config, risking privilege escalation or unsafe permissions if misconfigured (e.g., setting 0777 or root-owned binaries); enforce validation of modes and ownership in packaging pipeline. package_rules.bzl [34-54] Referred Code for key, attr_key in [("mode", "mode"), ("owner", "user"), ("group", "group"), ("rpm_filetag", "rpm_filetag")]: value = entry.get(key) if value: attrs[attr_key] = (value,) if attr_key == "rpm_filetag" else value return pkg_attributes(**attrs) if attrs else None def _normalize_file_entry(entry): src = entry.get("src") or entry.get("target") dest = entry.get("dest") allow_empty = False if src == None or dest == None: fail("File entry must include src and dest: %s" % entry) prefix, basename = _split_dest(dest) attributes = entry.get("attributes") or _attrs_from(entry) return { "src": src, "prefix": prefix, "basename": basename, "attributes": attributes, }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1677#issuecomment-3358572749 Original created: 2025-10-02T00:10:09Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td rowspan=2>⚪</td> <td><details><summary>Build command execution </summary> Description: The build script invokes an external 'cargo build' with a toolchain-modifying flag and environment manipulation, which if attacker-controlled environment variables or PATH are present could execute unexpected binaries during build; ensure build environments are trusted and paths are sanitized. <a href='https://github.com/carverauto/serviceradar/pull/1677/files#diff-8b543b27bc19aeba636edcd34b59c29b3816007d059cd43cd251485f86738fbfR64-R81'>build.rs [64-81]</a> <details open><summary>Referred Code</summary> ```rust let status = Command::new("cargo") .current_dir("..") .env_remove("RUSTUP_TOOLCHAIN") .args(&args) .status(); match status { Ok(status) if status.success() => { let profile_dir = if release { "release" } else { "debug" }; let artifact = Path::new("..") .join("target") .join("bpfel-unknown-none") .join(profile_dir) .join("profiler"); copy_ebpf(&artifact, dest) } Ok(status) => Err(format!("cargo {:?} exited with status {}", args, status).into()), Err(err) => Err(Box::new(err)), ``` </details></details></td></tr> <tr><td><details><summary>Insecure file permissions </summary> Description: Packaging rule allows arbitrary file installation with owner/group/mode from config, risking privilege escalation or unsafe permissions if misconfigured (e.g., setting 0777 or root-owned binaries); enforce validation of modes and ownership in packaging pipeline. <a href='https://github.com/carverauto/serviceradar/pull/1677/files#diff-3876e101ca949a98325d3dea0371c361af54a507bc700eca353418d22f2d029dR34-R54'>package_rules.bzl [34-54]</a> <details open><summary>Referred Code</summary> ```python for key, attr_key in [("mode", "mode"), ("owner", "user"), ("group", "group"), ("rpm_filetag", "rpm_filetag")]: value = entry.get(key) if value: attrs[attr_key] = (value,) if attr_key == "rpm_filetag" else value return pkg_attributes(**attrs) if attrs else None def _normalize_file_entry(entry): src = entry.get("src") or entry.get("target") dest = entry.get("dest") allow_empty = False if src == None or dest == None: fail("File entry must include src and dest: %s" % entry) prefix, basename = _split_dest(dest) attributes = entry.get("attributes") or _attrs_from(entry) return { "src": src, "prefix": prefix, "basename": basename, "attributes": attributes, } ``` </details></details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>⚪</td><td><details><summary>🎫 No ticket provided </summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'>Custom Compliance</td></tr> <tr><td>⚪</td><td><details><summary>No custom compliance provided</summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details>

qodo-code-review[bot] commented

2025-10-02 00:11:26 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1677#issuecomment-3358574844
Original created: 2025-10-02T00:11:26Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Centralizing all packaging configurations is risky The PR centralizes all packaging configurations into a single large file, `packages.bzl`, creating a bottleneck. It is suggested to co-locate each package's configuration with its own build files for better modularity. Examples: packaging/packages.bzl [3-818] `PACKAGES = { "core": { "package_name": "serviceradar-core", "description": "ServiceRadar Core API service", "maintainer": "Michael Freeman <mfreeman@carverauto.dev>", "architecture": "amd64", "section": "utils", "priority": "optional", "deb_depends": ["systemd", "jq"], "rpm_requires": ["systemd", "jq"], ... (clipped 806 lines)` packaging/core/BUILD.bazel [1-9] `load("//packaging:package_rules.bzl", "serviceradar_package_from_config") load("//packaging:packages.bzl", "PACKAGES") package(default_visibility = ["//visibility:public"]) serviceradar_package_from_config( name = "core", config = PACKAGES["core"], )` Solution Walkthrough: Before: `# packaging/packages.bzl PACKAGES = { "core": { "package_name": "serviceradar-core", # ... core package config }, "agent": { "package_name": "serviceradar-agent", # ... agent package config }, # ... and 20+ other packages } # packaging/core/BUILD.bazel load("//packaging:packages.bzl", "PACKAGES") serviceradar_package_from_config( name = "core", config = PACKAGES["core"], )` After: `# packaging/packages.bzl is removed or empty. # packaging/core/BUILD.bazel load("//packaging:package_rules.bzl", "serviceradar_package") CORE_CONFIG = { "package_name": "serviceradar-core", # ... core package config } serviceradar_package( name = "core", **CORE_CONFIG ) # packaging/agent/BUILD.bazel would have its own config.` Suggestion importance[1-10]: 8 __ Why: This is a valid and significant architectural suggestion that correctly identifies a major design flaw in the new packaging system, which could lead to future maintenance and collaboration issues.	Medium
General	Remove redundant binary dependencies Remove the redundant `all_crate_deps(normal = True)` from the `deps` of the `rperf_checker` `rust_binary` target, as they are already provided by the `:rperf_checker_lib` dependency. cmd/checkers/rperf-client/BUILD.bazel [34-39] `rust_binary( name = "rperf_checker", srcs = ["src/main.rs"], edition = "2021", - deps = [":rperf_checker_lib"] + all_crate_deps(normal = True), + deps = [":rperf_checker_lib"], )` Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that the dependencies in the `rust_binary` target are redundant, as they are already included via the `:rperf_checker_lib` dependency, and removing them improves build file clarity.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1677#issuecomment-3358574844 Original created: 2025-10-02T00:11:26Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Centralizing all packaging configurations is risky</summary> ___ **The PR centralizes all packaging configurations into a single large file, <code>packages.bzl</code>, creating a bottleneck. It is suggested to co-locate each package's configuration with its own build files for better modularity.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-9bfe2a5141a9e402bb5a5a8fca53b9eea64396ec18108c535392e1054c90b913R3-R818">packaging/packages.bzl [3-818]</a> </summary> ```starlark PACKAGES = { "core": { "package_name": "serviceradar-core", "description": "ServiceRadar Core API service", "maintainer": "Michael Freeman <mfreeman@carverauto.dev>", "architecture": "amd64", "section": "utils", "priority": "optional", "deb_depends": ["systemd", "jq"], "rpm_requires": ["systemd", "jq"], ... (clipped 806 lines) ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1677/files#diff-d375384323093ea1f35b57fb77433b4bbc98e26074cb44928d9c0e65644ae96bR1-R9">packaging/core/BUILD.bazel [1-9]</a> </summary> ```starlark load("//packaging:package_rules.bzl", "serviceradar_package_from_config") load("//packaging:packages.bzl", "PACKAGES") package(default_visibility = ["//visibility:public"]) serviceradar_package_from_config( name = "core", config = PACKAGES["core"], ) ``` </details> ### Solution Walkthrough: #### Before: ```starlark # packaging/packages.bzl PACKAGES = { "core": { "package_name": "serviceradar-core", # ... core package config }, "agent": { "package_name": "serviceradar-agent", # ... agent package config }, # ... and 20+ other packages } # packaging/core/BUILD.bazel load("//packaging:packages.bzl", "PACKAGES") serviceradar_package_from_config( name = "core", config = PACKAGES["core"], ) ``` #### After: ```starlark # packaging/packages.bzl is removed or empty. # packaging/core/BUILD.bazel load("//packaging:package_rules.bzl", "serviceradar_package") CORE_CONFIG = { "package_name": "serviceradar-core", # ... core package config } serviceradar_package( name = "core", **CORE_CONFIG ) # packaging/agent/BUILD.bazel would have its own config. ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This is a valid and significant architectural suggestion that correctly identifies a major design flaw in the new packaging system, which could lead to future maintenance and collaboration issues. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Remove redundant binary dependencies</summary> ___ **Remove the redundant <code>all_crate_deps(normal = True)</code> from the <code>deps</code> of the <code>rperf_checker</code> <code>rust_binary</code> target, as they are already provided by the <code>:rperf_checker_lib</code> dependency.** [cmd/checkers/rperf-client/BUILD.bazel [34-39]](https://github.com/carverauto/serviceradar/pull/1677/files#diff-4a15e4d3ef48348a6039522ddb9a0fc2ca5f1ee56ea80a51de4d2a8100a5cc32R34-R39) ```diff rust_binary( name = "rperf_checker", srcs = ["src/main.rs"], edition = "2021", - deps = [":rperf_checker_lib"] + all_crate_deps(normal = True), + deps = [":rperf_checker_lib"], ) ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly identifies that the dependencies in the `rust_binary` target are redundant, as they are already included via the `:rperf_checker_lib` dependency, and removing them improves build file clarity. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>