FFI/CGO sysmon-vm for osx/arm64 #2305
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!2305
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refs/pull/2305/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: #1744
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1744
Original created: 2025-10-11T07:02:28Z
Original updated: 2025-10-12T18:22:12Z
Original head: carverauto/serviceradar:1743-chore-investigate-using-fficgo-instead-of-exec-for-hostfreq-mac
Original base: main
PR Type
Enhancement
Description
Replace exec-based hostfreq with embedded CGO/FFI implementation
Integrate Objective-C++ IOReport sampler directly into Go binary
Remove standalone hostfreq binary and launchd service dependencies
Simplify macOS deployment to single checker process
Diagram Walkthrough
File Walkthrough
6 files
Replace exec-based hostfreq with embedded collectorAdd CGO bridge for macOS IOReportAdd stub implementation for non-Darwin platformsConvert standalone binary to CGO libraryAdd C header for CGO interfaceAdd C++ wrapper for CGO compilation1 files
Update test mocks for embedded collector10 files
Remove hostfreq binary installation and serviceRemove hostfreq from packaging pipelineUpdate CI paths for moved C++ codeRemove hostfreq build targetsRemove hostfreq Bazel targetsRemove hostfreq MakefileRemove hostfreq launchd service definitionRemove hostfreq environment variableRemove hostfreq from Bazel packagingAdd CGO and C++ library targets4 files
Update documentation for embedded samplerUpdate documentation for embedded approachUpdate architecture documentationUpdate runbook for simplified deploymentImported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1744#issuecomment-3393003618
Original created: 2025-10-11T07:03:18Z
PR Compliance Guide 🔍
Below is a summary of compliance checks for this PR:
FFI memory management
Description: C-allocated buffers are returned to Go via pointers (
hostfreq_collect_json) and requireprecise lifetime management; while
hostfreq_freeis provided and used in Go, any futuremisuse or missing frees would cause memory leaks or use-after-free—careful auditing of all
call paths that receive
out_json/out_erroris required.hostfreq_darwin.mm [421-486]
Referred Code
Privilege requirements
Description: Use of private macOS IOReport APIs requires elevated privileges; misconfiguration could
lead to permission errors or encourage users to run the service with excessive privileges,
increasing attack surface—documented, but operational hardening is advised.
hostfreq_darwin.mm [338-366]
Referred Code
🎫 #1743
returns expected frequencies.
with network/firewall constraints.
Codebase context is not defined
Follow the guide to enable codebase context checks.
No custom compliance provided
Follow the guide to enable custom compliance check.
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/1744#issuecomment-3393004225
Original created: 2025-10-11T07:04:18Z
PR Code Suggestions ✨
Explore these optional code suggestions:
Provide an error on memory allocation failure
In
hostfreq_collect_json, ifmallocfails, set an "out of memory" message inout_errorto provide a more specific error to the Go caller.pkg/cpufreq/hostfreq_darwin.mm [437-443]
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies a gap in error reporting where a memory allocation failure is not explicitly communicated to the caller, and the proposed fix improves diagnostics for the Go-side consumer.
Remove redundant nil check for C string
Remove the redundant
nilcheck foroutJSONincollectViaHostfreq, asC.GoString(nil)already produces an empty string that is handled by a subsequentcheck.
pkg/cpufreq/hostfreq_darwin.go [79-86]
Suggestion importance[1-10]: 2
__
Why: While the suggestion is technically correct that
C.GoString(nil)returns an empty string, removing thenilcheck would merge two distinct error conditions into one, losing the specific error message "hostfreq returned no data".Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1744#issuecomment-3393684923
Original created: 2025-10-11T21:59:10Z
CI Feedback 🧐
(Feedback updated until commit
github.com/carverauto/serviceradar@d3813e348d)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
Action: build
Failed stage: Test [❌]
Failed test name: ""
Failure summary:
The Bazel build failed due to environment/toolchain incompatibilities on the runner:
- GCC is too
old for multiple dependencies (Abseil, Protobuf, etc.). Many C++ compilations error with "This
package requires GCC 7 or higher." and Protobuf asserts "GCC 7.3 and newer" (e.g.,
external/abseil-cpp+/absl/base/policy_checks.h:57:2,external/protobuf+/src/google/protobuf/compiler/BUILD.bazel:115).- The C toolchain resolution also
had issues (initial error: unable to find a CC toolchain for platform
@@rules_go+//go/toolchain:darwin_arm64, exec platform@@//build/rbe:rbe_platformatexternal/rules_cc+/cc/BUILD:145:19), indicating misconfigured/unsupported toolchain selection inremote execution.
- Multiple C compilations failed due to "absolute path inclusion(s) found" (e.g.,
external/zlib+/BUILD.bazel:91:11,third_party/cwalk/BUILD.bazel:7:11,protobuf+/third_party/utf8_range), where headers from/usr/lib/gcc/x86_64-linux-gnu/5/...wereincluded via absolute paths, which Bazel forbids under the configured toolchain.
- An OCaml PPX step
failed due to missing GLIBC version on the runner:
/lib/x86_64-linux-gnu/libm.so.6: version'GLIBC_2.29' not foundwhen runningexternal/tools_opam++opam+opam.lwt_ppx/lib/ppx.exe(BUILD.bazel:150:13), indicating the host system glibc is too old for the provided prebuilt PPX.
Overall, the CI runner environment (GCC 5.x, older glibc) and toolchain configuration are
incompatible with the repository’s dependencies and Bazel toolchain expectations, causing widespread
compile-time failures.
Relevant error logs:
Imported GitHub PR comment.
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1744#issuecomment-3395195960
Original created: 2025-10-12T18:22:09Z
abandoning, pulled these changes into a different branch, fixed issues and merged, this is stale.
Pull request closed