Repository navigation
ci(rdc): add dynolog integration CI - #11760
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
🟡 Changes recommended
The workflow should run the added unit tests and trigger when projects/amdsmi/** changes.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds CI coverage for building and smoke-testing dynolog’s RDC integration against freshly built RDC artifacts, with Python helpers, tests, and documentation.
Changes:
- Adds a self-hosted GPU GitHub Actions workflow.
- Adds dynolog clone, build, and runtime-validation helpers.
- Adds unit tests and local usage documentation.
- Workflow should also run the helper unit tests and trigger on
projects/amdsmi/**changes.
File summaries
| File | Summary |
|---|---|
projects/rdc/tests/dynolog_integration/test_run_example.py |
Tests runtime-output validation. |
projects/rdc/tests/dynolog_integration/test_build.py |
Tests build helper behavior. |
projects/rdc/tests/dynolog_integration/run_example.py |
Runs and validates the dynolog example. |
projects/rdc/tests/dynolog_integration/README.md |
Documents usage and configuration. |
projects/rdc/tests/dynolog_integration/clone.py |
Clones pinned dynolog source. |
projects/rdc/tests/dynolog_integration/build.py |
Builds the dynolog RDC wrapper. |
projects/rdc/tests/dynolog_integration/_common.py |
Provides shared subprocess and logging utilities. |
projects/rdc/tests/dynolog_integration/__main__.py |
Dispatches CLI subcommands. |
projects/rdc/tests/dynolog_integration/__init__.py |
Defines package documentation. |
.github/workflows/rdc-dynolog-ci.yml |
Builds RDC and dynolog integration on GPU runners; currently omits helper tests and amd-smi source changes from workflow coverage. |
Review details
Suppressed comments (1)
.github/workflows/rdc-dynolog-ci.yml:31
- This trigger omits
projects/amdsmi/**, even though the job builds amd-smi from that tree and links RDC/dynolog against it. Changes to the dependency can therefore break this integration without scheduling the workflow; include the amd-smi source/API paths (or the wholeprojects/amdsmi/**tree) inon.pull_request.paths.
- 'projects/rdc/include/**'
- 'projects/rdc/rdc_libs/**'
- 'projects/rdc/common/**'
- 'projects/rdc/CMakeLists.txt'
- 'projects/rdc/cmake_modules/**'
- 'projects/rdc/tests/dynolog_integration/**'
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
40884e3 to
3b75cf4
Compare
3b75cf4 to
ec95f2c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Runtime failures can incorrectly pass, and the newly added unit tests are not executed by CI.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
.github/workflows/rdc-dynolog-ci.yml:271
- The new helper tests are not executed by this workflow, and the existing RDC CI invokes pytest only for
tests/ci, so these 15 tests can regress without any CI failure. Run the stdlib suite before exercising the integration commands.
- name: Clone dynolog
- Files reviewed: 10/10 changed files
- Comments generated: 3
- Review effort level: Balanced
6e9fd6a to
c7805b0
Compare
Build the dynolog RDC shim (dynolog/src/gpumon/amd) and its RdcWrapperExample against a freshly-built RDC, mirroring the Device Metrics Exporter integration CI. This catches RDC API/ABI/build changes that would break the downstream dynolog integration. - .github/workflows/rdc-dynolog-ci.yml: build gRPC, amd-smi (ESMI) and RDC, then clone dynolog at a pinned ref, build its RDC shim + example, and run the example on GPU. The build is the primary gate; the runtime example soft-passes when GPU metrics are unavailable. - projects/rdc/tests/dynolog_integration: python helpers (clone / build-wrapper / run-example) with stdlib unit tests and a README.
c7805b0 to
23177c3
Compare
The workflow built gRPC 1.67.1 shared from a mutable upstream tag before building RDC, costing ~4.5 minutes and ~110k log lines per run. That is both unnecessary and wrong: TheRock builds RDC against THEROCK_GRPC_VERSION (third-party/grpc, 1.78.1, static), which is also RDC's own GRPC_DESIRED_VERSION default, so the job was validating a gRPC configuration that does not ship. gRPC is a standalone-mode dependency. rdc_libs/CMakeLists.txt installs librdc_client.so into the exported target set only under BUILD_STANDALONE, and it is the only gRPC::grpc++ consumer. dynolog embeds RDC in-process and links the rdc target, whose exported interface is rdc_bootstrap;pthread;amd_smi;cap. Building RDC with BUILD_STANDALONE=OFF therefore leaves librdc.so unchanged, emits an rdcTargets.cmake with no gRPC reference, and removes the dependency outright rather than pinning a second version of it. RDC's standalone/gRPC surface stays covered by rdc-ci.yml. Also drop two dead steps: the workspace is checked out at a known path, so searching for projects/rdc with find is redundant, and marking it safe for git does nothing because git is absent at checkout time and actions/checkout falls back to a tarball with no .git at all. evaluate() reported "no GPU entities were discovered (Monitoring 0 entities)" for runs where that line was never printed, which is a hang in RdcWrapper setup rather than an empty GPU list. Distinguish the two and fail on the hang when a GPU is present: no arch explains a start with neither an entity count nor a fatal, so it must not be excused as field availability.
## Summary Adds a CI workflow that builds the **dynolog** RDC integration shim (`dynolog/src/gpumon/amd`) and its `RdcWrapperExample` against a freshly-built RDC, mirroring the existing Device Metrics Exporter integration CI (`amdsmi-dme-ci.yml`). dynolog runs RDC embedded in-process, so this catches RDC API/ABI/build changes that would break that downstream integration. Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com> [rocm-systems] ROCm/rocm-systems#11760 (commit 9f9214b)
TheRock Submodule Bump ActivityNewest first
|
Summary
Adds a CI workflow that builds the dynolog RDC integration shim
(
dynolog/src/gpumon/amd) and itsRdcWrapperExampleagainst a freshly-builtRDC, mirroring the existing Device Metrics Exporter integration CI
(
amdsmi-dme-ci.yml). dynolog runs RDC embedded in-process, so this catches RDCAPI/ABI/build changes that would break that downstream integration.
JIRA ID: ROCM-31209
What's included
.github/workflows/rdc-dynolog-ci.yml— on RDC PRs todevelop: buildamd-smi (ESMI) and RDC into
/opt/rocm, then clone dynolog at a pinned SHA,build its RDC shim + example, and run the example on GPU.
projects/rdc/tests/dynolog_integration/— python helpers(
clone/build-wrapper/run-example) with stdlib unit tests and aREADME, reproducible locally via
python3 -m dynolog_integration <subcommand>.Design
The primary signal is compiling + linking the RdcWrapper shim against
develop'srdc/rdc.handlibrdc.so— it fails when an RDC change breaksthe dynolog integration surface. The shim exercises 12 RDC entry points,
including the partition/entity API (
rdc_get_num_partition,rdc_get_info_from_entity_index,rdc_get_entity_index_from_info).The on-GPU example run is a bonus smoke test that soft-passes (with a
warning) when GPU metrics are unavailable or a field is unsupported on the
runner arch, so hardware/arch variance doesn't cause false failures. It still
hard-fails on results no arch explains: the binary not starting at all, a
non-capability fatal from
RdcWrapper, the free-running poll loop exiting onits own, or setup never completing within the window (a hang). Build breakage
always gates.
No gRPC is built
RDC is built with
-DBUILD_STANDALONE=OFF. gRPC is a standalone-modedependency:
rdc_libs/CMakeLists.txtinstallslibrdc_client.sointo theexported target set only under
BUILD_STANDALONE, and that is the onlygRPC::grpc++consumer. dynolog embeds RDC and links therdctarget, whoseexported interface is
rdc_bootstrap;pthread;amd_smi;cap. Nothing underrdc_libs/readsBUILD_STANDALONEother than that one gate, solibrdc.soisunchanged either way and the generated
rdcTargets.cmakecarries no gRPCreference.
Building gRPC here would also mean pinning a second version of it: TheRock
builds RDC against
THEROCK_GRPC_VERSION(third-party/grpc/, currently1.78.1, statically linked), which is also RDC's own
GRPC_DESIRED_VERSIONdefault. RDC's standalone/gRPC surface stays covered by
rdc-ci.yml.Test plan
python3 -m unittest dynolog_integration.test_build dynolog_integration.test_run_examplefromprojects/rdc/tests— 19 testscovering the ROCm-version math and example-output verification. ruff
check/format, codespell and actionlint are clean.
RdcWrapperExample OK (GPU present): collected up to 4 metrics from 1 entities.Required config / notes
vars.RUNNER_TYPE(self-hosted GPU runner) — same convention as theDME CI. Optional:
vars.GPU_TARGETS(defaultgfx942),vars.RDC_DOCKER_IMAGE(default
rocm/dev-ubuntu-22.04:7.2).b301ce05b3b25e2f48e4525bfc52a0ba6fe77446; overridable viaworkflow_dispatch.libgoogle-glog-dev(itslibunwind-devdependency is unavailable there), so the workflow builds glog v0.6.0 from
source into
/usr/local.