RDKEMW-18181: CPU and Memory metrics - #577
Open
DouglasAdler wants to merge 7 commits into
Open
Conversation
Introduce a private metrics module that lets ready clients receive connected and periodic sample requests, report process CPU metrics back to the server, and log combined client/server CPU usage. Wire the module into client and server IPC setup and update client controller tests/mocks for the new dependency.
…luggable output - Add process memory (VmRSS) to client metrics reporting - Add cgroup memory (v2 with v1 fallback) to server metrics - Implement Welford's algorithm for online mean/variance (MetricsAccumulator) - Add StateMetricsAggregator: accumulates per-state min/max/mean/stddev - Track playback state transitions per session, emit aggregated report on state change (e.g. PLAYING->END_OF_STREAM reports CPU/memory stats) - Track application state transitions (RUNNING/INACTIVE), emit report - Add IMetricsReporter interface with LogMetricsReporter and CompositeMetricsReporter for pluggable output destinations - Add MetricsThresholdChecker with configurable warning/critical levels and 2-sample debounce to avoid alert storms - Only aggregate and check thresholds on PERIODIC samples (not STATE_TRANSITION boundary samples which have unreliable CPU data) - Minimum 100ms elapsed time for CPU percentage calculation to prevent division-by-tiny-delta artifacts - Wire playback state notifications through MediaPipelineClient - Wire application state notifications through SessionManagementServer - Add METRICS_SAMPLE_REASON_STATE_TRANSITION to proto enum - Update mocks for new interface methods
Signed-off-by: Douglas Adler <douglas.adler@yahoo.com>
On transition to INACTIVE, record a server-side memory snapshot after pipelines and shared memory have been freed. This fires unconditionally (before the manager ACK) so it is captured even if the IPC socket breaks. The snapshot reads /proc/self/smaps_rollup and logs: server_mem_kb - VmRSS total cgroup_mem_kb - cgroup memory usage anon_kb - anonymous (= private_dirty_kb) private_dirty_kb - truly committed RAM the OS cannot reclaim private_clean_kb - file-backed, OS-reclaimable shared_clean_kb - loaded .so libs, OS-reclaimable Also call malloc_trim(0) in PlaybackService::switchToInactive() to return heap fragmentation to the OS after pipelines are torn down. Add shm_mem_kb (Pss_Shmem from smaps_rollup) to each periodic sample so the memfd-backed shared transport buffer is visible during playback. Promote reportPeriodicSample log level from INFO to MIL so samples appear in production logs.
Signed-off-by: Douglas Adler <douglas.adler@yahoo.com>
Replace the copied private metrics proto files with symbolic links and remove the duplicated MetricsCollector header content. Move private metrics ownership into PlaybackService so MediaPipeline and WebAudio can share playback-state reporting without passing a raw service pointer through IPC. Keep API-provided video geometry authoritative and use environment geometry only as a fallback. Retain the 15-second sampling interval while limiting active routine logs to 10-minute intervals. Report significant changes early, suppress stable inactive samples, and handle nonresponsive clients. Add unit coverage for IPC, collectors, reporters, services, WebAudio, video geometry, and metrics ownership. Signed-off-by: Douglas Adler <douglas.adler@yahoo.com>
|
Pull request title must follow the pattern: Pull request description must follow the Commit message format for RDK-E:
< JIRA TICKET >: < one line summary of change less than 65 characters > |
|
media/server/main/source/MetricsCollector.cpp:124:9: style: Condition 'becameUnresponsive' is always false [knownConditionTrueFalse] |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new “private” metrics collection pipeline to Rialto so the server can automatically collect CPU/memory from both client and server processes, reporting on significant changes and on state transitions. It also extends state propagation to feed metrics boundaries and adds an environment-driven fallback video geometry path in the GStreamer player.
Changes:
- Introduces a new PrivateMetrics IPC module (proto + client IPC + server IPC/service/main) and integrates it into session lifecycle, media pipeline, and WebAudio paths.
- Adds server-side metrics framework components (collector, reporters, threshold checker, aggregators) plus comprehensive unit tests.
- Adds environment-based default video geometry fallback and improves sink rectangle handling (including
render-rectangle).
Reviewed changes
Copilot reviewed 96 out of 96 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/media/server/service/webAudioPlayerService/WebAudioPlayerServiceTestsFixture.h | Adds PrivateMetricsService mock to WebAudio service tests fixture. |
| tests/unittests/media/server/service/webAudioPlayerService/WebAudioPlayerServiceTestsFixture.cpp | Passes metrics service into WebAudioPlayerService constructor in tests. |
| tests/unittests/media/server/service/sessionServerManager/SessionServerManagerTestsFixture.cpp | Updates expectations to include application state change notifications to session management. |
| tests/unittests/media/server/service/playbackService/PlaybackServiceTestsFixture.h | Adds fixture helper for PrivateMetricsService exposure test. |
| tests/unittests/media/server/service/playbackService/PlaybackServiceTestsFixture.cpp | Adds a new fixture assertion for metrics service exposure. |
| tests/unittests/media/server/service/playbackService/PlaybackServiceTests.cpp | Adds a new gtest validating PrivateMetricsService is exposed. |
| tests/unittests/media/server/service/metrics/PrivateMetricsServiceTests.cpp | New unit tests for service-layer routing and lifecycle handling. |
| tests/unittests/media/server/service/metrics/MetricsClientsTests.cpp | New tests for MediaPipeline/WebAudio metrics client wrappers forwarding + reporting. |
| tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.h | Adds PrivateMetricsService mock to MediaPipeline service tests fixture. |
| tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp | Passes metrics service into MediaPipelineService constructor in tests. |
| tests/unittests/media/server/service/CMakeLists.txt | Registers new service-layer metrics unit tests. |
| tests/unittests/media/server/mocks/service/PrivateMetricsServiceMock.h | Adds mock for IPrivateMetricsService. |
| tests/unittests/media/server/mocks/service/PlaybackServiceMock.h | Extends playback service mock to expose PrivateMetricsService. |
| tests/unittests/media/server/mocks/main/MetricsReporterMock.h | Adds mock for IMetricsReporter. |
| tests/unittests/media/server/mocks/main/MetricsCollectorMock.h | Adds mocks for IMetricsCollector and IMetricsCollectorFactory. |
| tests/unittests/media/server/mocks/main/MetricsCollectorClientMock.h | Adds mock for IMetricsCollectorClient. |
| tests/unittests/media/server/mocks/ipc/SessionManagementServerMock.h | Adds notifyApplicationStateChanged to session management server mock. |
| tests/unittests/media/server/mocks/ipc/PrivateMetricsModuleServiceMock.h | Adds mocks for PrivateMetrics module IPC service + factory. |
| tests/unittests/media/server/main/metrics/MetricsHelpersTests.cpp | New unit tests for accumulator/aggregator/reporters/threshold checker behavior. |
| tests/unittests/media/server/main/metrics/MetricsCollectorTests.cpp | New unit tests for MetricsCollector sampling/state boundaries/timer behavior. |
| tests/unittests/media/server/main/metrics/LogMetricsReporterTests.cpp | New unit tests for log suppression/reporting logic. |
| tests/unittests/media/server/main/CMakeLists.txt | Registers new server-main metrics unit tests. |
| tests/unittests/media/server/ipc/sessionManagementServer/SessionManagementServerTestsFixture.h | Adds PrivateMetrics module/service mocks to session management IPC tests. |
| tests/unittests/media/server/ipc/sessionManagementServer/SessionManagementServerTestsFixture.cpp | Wires PrivateMetrics module factory and client connect/disconnect expectations. |
| tests/unittests/media/server/ipc/privateMetricsModuleService/PrivateMetricsModuleServiceTests.cpp | New unit tests for PrivateMetricsModuleService lifecycle, RPCs, and factory. |
| tests/unittests/media/server/ipc/CMakeLists.txt | Registers new IPC-layer private metrics tests. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/SetupElementTest.cpp | Adds tests for environment fallback video geometry behavior. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp | Updates test to check render-rectangle property absence handling. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h | Adds helpers for fallback geometry setup and pending-geometry checks. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp | Implements new helpers for fallback geometry scenarios. |
| tests/unittests/media/client/mocks/ipc/PrivateMetricsIpcMock.h | Adds client-side mock for IPrivateMetricsIpc. |
| tests/unittests/media/client/mocks/ipc/PrivateMetricsIpcFactoryMock.h | Adds client-side mock for IPrivateMetricsIpcFactory. |
| tests/unittests/media/client/main/clientController/MemoryManagementTest.cpp | Updates client controller tests to include PrivateMetricsIpc construction. |
| tests/unittests/media/client/main/clientController/CreateTest.cpp | Adds PrivateMetricsIpc creation/failure tests and a basic reporting test. |
| tests/unittests/media/client/ipc/privateMetricsIpc/PrivateMetricsIpcTests.cpp | New unit tests for PrivateMetricsIpc RPC + event forwarding behavior. |
| tests/unittests/media/client/ipc/CMakeLists.txt | Registers new client IPC private metrics tests. |
| proto/privatemetricsmodule.proto | Defines new PrivateMetricsModule proto service, messages, and enums. |
| proto/CMakeLists.txt | Adds privatemetricsmodule.proto to protobuf generation list. |
| media/server/service/source/WebAudioPlayerService.h | Adds metrics service dependency to WebAudioPlayerService. |
| media/server/service/source/WebAudioPlayerService.cpp | Wraps WebAudio client with WebAudioPlayerMetricsClient for state reporting. |
| media/server/service/source/WebAudioPlayerMetricsClient.h | New wrapper client that reports WebAudio state transitions to metrics service. |
| media/server/service/source/WebAudioPlayerMetricsClient.cpp | Implements state reporting + forwarding for WebAudio client callbacks. |
| media/server/service/source/SessionServerManager.cpp | Notifies session management server of application state transitions for metrics. |
| media/server/service/source/PrivateMetricsService.h | New service-layer router managing collectors per connected client. |
| media/server/service/source/PrivateMetricsService.cpp | Implements routing, app-state updates, and INACTIVE server memory snapshot logging. |
| media/server/service/source/PlaybackService.h | Adds PrivateMetricsService ownership and getter to PlaybackService. |
| media/server/service/source/PlaybackService.cpp | Instantiates PrivateMetricsService, wires into services, trims heap on inactive. |
| media/server/service/source/MediaPipelineService.h | Adds metrics service dependency to MediaPipelineService. |
| media/server/service/source/MediaPipelineService.cpp | Wraps media pipeline client with MediaPipelineMetricsClient for playback-state reporting. |
| media/server/service/source/MediaPipelineMetricsClient.h | New wrapper client that reports playback state transitions to metrics service. |
| media/server/service/source/MediaPipelineMetricsClient.cpp | Implements playback state reporting + forwarding for media pipeline client callbacks. |
| media/server/service/include/IPrivateMetricsService.h | New service API for client lifecycle, metrics reports, and state notifications. |
| media/server/service/include/IPlaybackService.h | Extends playback service interface to expose PrivateMetricsService. |
| media/server/service/CMakeLists.txt | Adds new service sources for metrics wrappers and service implementation. |
| media/server/main/source/MetricsThresholdChecker.cpp | New threshold checker implementation for metrics alerting. |
| media/server/main/source/LogMetricsReporter.cpp | New log reporter with periodic suppression and change detection. |
| media/server/main/source/CompositeMetricsReporter.cpp | New fan-out reporter to multiple reporters. |
| media/server/main/interface/IMetricsCollectorClient.h | New callback interface for requesting metrics samples from the client via IPC. |
| media/server/main/interface/IMetricsCollector.h | New collector interfaces + factory contract and metrics data struct. |
| media/server/main/interface/IMainThread.h | Adds missing <cstdint> include. |
| media/server/main/include/StateMetricsAggregator.h | New per-state aggregator for metrics statistics. |
| media/server/main/include/MetricsThresholdChecker.h | New threshold config + checker API. |
| media/server/main/include/MetricsCollector.h | New collector class declaration and internal state tracking. |
| media/server/main/include/MetricsAccumulator.h | New running statistics accumulator (Welford). |
| media/server/main/include/LogMetricsReporter.h | New log reporter API. |
| media/server/main/include/IMetricsReporter.h | New reporter interface + report/alert structures. |
| media/server/main/include/CompositeMetricsReporter.h | New composite reporter API. |
| media/server/main/CMakeLists.txt | Adds new server-main metrics sources to build. |
| media/server/ipc/source/SessionManagementServer.cpp | Integrates PrivateMetricsModuleService and forwards app-state changes to it. |
| media/server/ipc/source/PrivateMetricsModuleService.cpp | New IPC module handling RPCs/events and implementing IMetricsCollectorClient. |
| media/server/ipc/source/MediaPipelineModuleService.cpp | Includes PrivateMetrics module header (integration point). |
| media/server/ipc/source/MediaPipelineClient.cpp | Formatting-only constructor signature change. |
| media/server/ipc/source/IpcFactory.cpp | Wires PrivateMetricsModuleServiceFactory into session management server creation. |
| media/server/ipc/interface/ISessionManagementServer.h | Adds notifyApplicationStateChanged to session management server interface. |
| media/server/ipc/include/SessionManagementServer.h | Adds private metrics module factory + module member + notifier method. |
| media/server/ipc/include/PrivateMetricsModuleService.h | New IPC service header + factory + IMetricsCollectorClient implementation. |
| media/server/ipc/include/IPrivateMetricsModuleService.h | New IPC interface for PrivateMetrics module service/factory. |
| media/server/ipc/include/IMediaPipelineModuleService.h | Minor whitespace/formatting adjustment. |
| media/server/ipc/CMakeLists.txt | Adds PrivateMetricsModuleService source to build. |
| media/server/gstplayer/source/tasks/generic/SetupElement.cpp | Applies environment fallback geometry only when API geometry not set. |
| media/server/gstplayer/source/GstGenericPlayer.cpp | Adds env parsing for default geometry, playbin sink overrides, and render-rectangle support. |
| media/server/gstplayer/include/GenericPlayerContext.h | Adds default geometry and an “set by API” atomic flag to context. |
| media/client/main/source/ClientController.cpp | Creates PrivateMetricsIpc and reports CPU/memory samples on server request. |
| media/client/main/include/ClientController.h | Implements IPrivateMetricsIpcClient and adds metrics sampling helpers and member. |
| media/client/ipc/source/PrivateMetricsIpc.cpp | New client IPC module to notify readiness, report metrics RPCs, and process sample request events. |
| media/client/ipc/interface/IPrivateMetricsIpc.h | New client IPC interfaces for metrics module. |
| media/client/ipc/include/PrivateMetricsIpc.h | New client IPC header for metrics module implementation. |
| media/client/ipc/CMakeLists.txt | Adds PrivateMetricsIpc source to build. |
| docs/ServerManagerDesign.html | Adds design documentation (includes metrics-related control/data request discussion). |
| docs/MetricsDesign.md | Adds full system design documentation for metrics gathering. |
| docs/metrics/RialtoMetricsReport.md | Adds a findings/report document describing metrics output and production observations. |
| cmake/wpeframeworkcore-config.cmake | Adds a CMake package config file (currently machine-path specific). |
| cmake/wpeframeworkcom-config.cmake | Adds a CMake package config file (currently machine-path specific). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+26
| #include "MetricsThresholdChecker.h" | ||
|
|
||
| namespace firebolt::rialto::server | ||
| { | ||
| MetricsThresholdChecker::MetricsThresholdChecker(MetricsThresholdConfig config, IMetricsReporter *reporter) | ||
| : m_config{std::move(config)}, m_reporter{reporter} | ||
| { |
Comment on lines
+20
to
+24
| #include "PrivateMetricsService.h" | ||
| #include "RialtoServerLogging.h" | ||
| #include <cinttypes> | ||
| #include <fstream> | ||
| #include <string> |
Comment on lines
+40
to
+54
| void PrivateMetricsService::clientReady(int clientId, | ||
| const std::shared_ptr<firebolt::rialto::server::IMetricsCollectorClient> &client) | ||
| { | ||
| std::lock_guard<std::mutex> lock{m_mutex}; | ||
| auto collector = m_collectorFactory->create(clientId, client, m_currentApplicationState); | ||
| if (collector) | ||
| { | ||
| m_collectors.emplace(clientId, std::move(collector)); | ||
| RIALTO_SERVER_LOG_INFO("MetricsCollector created for client %d", clientId); | ||
| } | ||
| else | ||
| { | ||
| RIALTO_SERVER_LOG_ERROR("Failed to create MetricsCollector for client %d", clientId); | ||
| } | ||
| } |
Comment on lines
+23
to
28
| #include <algorithm> | ||
| #include <chrono> | ||
| #include <cinttypes> | ||
| #include <cstring> | ||
| #include <fstream> | ||
| #include <stdexcept> |
Comment on lines
+28
to
+40
| const char *sampleReasonToString(::firebolt::rialto::MetricsSampleReason reason) | ||
| { | ||
| switch (reason) | ||
| { | ||
| case firebolt::rialto::METRICS_SAMPLE_REASON_CONNECTED: | ||
| return "CONNECTED"; | ||
| case firebolt::rialto::METRICS_SAMPLE_REASON_PERIODIC: | ||
| return "PERIODIC"; | ||
| case firebolt::rialto::METRICS_SAMPLE_REASON_UNKNOWN: | ||
| default: | ||
| return "UNKNOWN"; | ||
| } | ||
| } |
Comment on lines
+151
to
+154
| void PlaybackServiceTests::getPrivateMetricsServiceShouldSucceed() | ||
| { | ||
| EXPECT_NE(&m_sut->getPrivateMetricsService(), nullptr); | ||
| } |
Comment on lines
+48
to
+49
| set_and_check(WPEFRAMEWORK_CORE_INCLUDE_DIRS "/home/dadler/development/rialto/rialto/stubs/wpeframework-core/third-party/Source/") | ||
|
|
Comment on lines
+48
to
+49
| set_and_check(WPEFRAMEWORK_COM_INCLUDE_DIRS "/home/dadler/development/rialto/rialto/stubs/wpeframework-com/third-party/Source/") | ||
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| kServerMetrics.cgroupMemoryUsageKb, kServerMetrics.cgroupMemoryLimitKb); | ||
|
|
||
| std::lock_guard<std::mutex> lock{m_mutex}; | ||
| m_previousSample = PreviousSample{metrics.monotonicTimeMs, metrics.processCpuTimeMs, metrics.processMemoryKb, |
| // Update previous sample | ||
| { | ||
| std::lock_guard<std::mutex> lock{m_mutex}; | ||
| m_previousSample = |
| // Update previous sample | ||
| { | ||
| std::lock_guard<std::mutex> lock{m_mutex}; | ||
| m_previousSample = |
| { | ||
| if (sline.rfind("Pss_Shmem:", 0) == 0) | ||
| { | ||
| std::sscanf(sline.c_str(), "Pss_Shmem: %" SCNu64, &shmMemoryKb); |
| { | ||
| if (line.rfind("VmRSS:", 0) == 0) | ||
| { | ||
| std::sscanf(line.c_str(), "VmRSS: %" SCNu64, &serverMemoryKb); |
| else if (sline.rfind("Private_Clean:", 0) == 0) | ||
| std::sscanf(sline.c_str(), "Private_Clean: %" SCNu64, &privateCleanKb); | ||
| else if (sline.rfind("Private_Dirty:", 0) == 0) | ||
| std::sscanf(sline.c_str(), "Private_Dirty: %" SCNu64, &privateDirtyKb); |
| { | ||
| std::lock_guard<std::mutex> lock{m_mutex}; | ||
| m_clientIds[ipcClient] = kClientId; | ||
| m_ipcClients[kClientId] = ipcClient; |
| auto report{sessionState.aggregator.finalize(kNowMs)}; | ||
| StateTransitionReport transitionReport; | ||
| transitionReport.context = context; | ||
| transitionReport.metrics = report; |
| auto report{m_globalAggregator.finalize(kNowMs)}; | ||
| StateTransitionReport transitionReport; | ||
| transitionReport.context = "global"; | ||
| transitionReport.metrics = report; |
|
|
||
| namespace firebolt::rialto::server | ||
| { | ||
| MetricsThresholdChecker::MetricsThresholdChecker(MetricsThresholdConfig config, IMetricsReporter *reporter) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RDKEMW-18181: CPU and Memory metrics
Reason for change: Add automatic metrics collection for CPU and Memory usage
for both the client and server sessions. Report data on >10% change or state
transition.
Test Procedure: Rialto CI