RDKB-65004: Log reduction changes#57
Open
jayasrikadiyal wants to merge 5 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a duplicate log suppression capability to reduce on-device log growth and uploaded log volume, combining (1) in-process suppression in rdk_logger and (2) a pre-upload, file-based deduplication step plus supporting OpenSpec artifacts.
Changes:
- Added a suppression engine (
include/rdk_log_suppression.h,src/rdk_log_suppression.c) and integrated it into the logging path and lifecycle (src/rdk_debug_priv.c,src/rdk_logger_init.c), plus build wiring (src/Makefile.am,configure.ac). - Added a pre-upload dedup utility (
utils/rdk_log_dedup_sync.c) and wrapper script (scripts/logDedup.sh). - Added OpenSpec proposal/design/tasks plus repository automation documentation (GitHub skills/prompts).
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/rdk_log_dedup_sync.c | New standalone tool to collapse consecutive duplicate log lines with a summary marker. |
| src/rdk_logger_init.c | Hooks suppression init/deinit into logger lifecycle (compile-time gated). |
| src/rdk_log_suppression.c | New suppression engine (hash-based pattern tracking with per-category ring buffer). |
| src/rdk_debug_priv.c | Adds suppression check before emitting log4c messages (compile-time gated). |
| src/Makefile.am | Adds suppression engine source file to the library build. |
| scripts/logDedup.sh | Wrapper script to run pre-upload log dedup across a directory. |
| openspec/changes/log-suppression/tasks.md | Implementation task breakdown for the log suppression change. |
| openspec/changes/log-suppression/README.md | Brief readme for the delta spec change folder. |
| openspec/changes/log-suppression/proposal.md | Problem statement and proposed feature changes. |
| openspec/changes/log-suppression/design.md | Technical design decisions/trade-offs for suppression. |
| openspec/changes/log-suppression/.openspec.yaml | OpenSpec change metadata. |
| include/rdk_log_suppression.h | Public API for configuring/checking suppression and retrieving stats. |
| configure.ac | Adds --enable-log-suppression flag and HAVE_LOG_SUPPRESSION define/conditional. |
| .github/skills/openspec-sync-specs/SKILL.md | Documentation for an OpenSpec sync workflow skill. |
| .github/skills/openspec-propose/SKILL.md | Documentation for an OpenSpec proposal workflow skill. |
| .github/skills/openspec-explore/SKILL.md | Documentation for an OpenSpec exploration workflow skill. |
| .github/skills/openspec-archive-change/SKILL.md | Documentation for an OpenSpec archive workflow skill. |
| .github/skills/openspec-apply-change/SKILL.md | Documentation for an OpenSpec apply/implementation workflow skill. |
| .github/prompts/opsx-sync.prompt.md | Prompt doc for syncing delta specs to main specs. |
| .github/prompts/opsx-propose.prompt.md | Prompt doc for generating an OpenSpec proposal. |
| .github/prompts/opsx-explore.prompt.md | Prompt doc for explore mode behavior. |
| .github/prompts/opsx-archive.prompt.md | Prompt doc for archiving OpenSpec changes. |
| .github/prompts/opsx-apply.prompt.md | Prompt doc for applying/implementing OpenSpec tasks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+53
to
+55
| #ifdef HAVE_LOG_SUPPRESSION | ||
| #include "rdk_log_suppression.h" | ||
| #endif |
Author
There was a problem hiding this comment.
Good catch. HAVE_LOG_SUPPRESSION is coming from config.h, so checking it before including config.h can compile the suppression paths out of the translation unit. This affects both src/rdk_debug_priv.c and src/rdk_logger_init.c. I’ll move the config.h include to the top of both files before any HAVE_LOG_SUPPRESSION guards.
Comment on lines
+41
to
+43
| #ifdef HAVE_LOG_SUPPRESSION | ||
| #include "rdk_log_suppression.h" | ||
| #endif |
Comment on lines
+181
to
+183
| if (!g_suppress.initialized || !g_suppress.globally_enabled) { | ||
| return RDK_SUPPRESS_PASS; | ||
| } |
|
|
||
| LOG_DIR="${1:-/rdklogs/logs}" | ||
| THRESHOLD="${2:-2}" | ||
| DEDUP_BIN="/usr/bin/rdkLogDedupSync" |
jayasrikadiyal
changed the title
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+199
to
+217
| if (elapsed > (double)cat->window_sec) { | ||
| /* Window expired - emit summary if we suppressed, then reset */ | ||
| if (entry->repeat_count > cat->threshold) { | ||
| uint32_t suppressed = entry->repeat_count - cat->threshold; | ||
| *repeat_count = suppressed; | ||
| } | ||
| cat->total_summaries++; | ||
| /* Reset entry for new window */ | ||
| entry->first_seen = now; | ||
| entry->last_seen = now; | ||
| entry->repeat_count = 1; | ||
| return RDK_SUPPRESS_EMIT_SUMMARY; | ||
| } | ||
| /* Window expired but below threshold - reset */ | ||
| entry->first_seen = now; | ||
| entry->last_seen = now; | ||
| entry->repeat_count = 1; | ||
| return RDK_SUPPRESS_PASS; | ||
| } |
Comment on lines
+27
to
+40
| LOG_DIR="${1:-/rdklogs/logs}" | ||
| THRESHOLD="${2:-2}" | ||
| DEDUP_BIN="/usr/bin/rdkLogDedupSync" | ||
| DEDUP_COUNT=0 | ||
|
|
||
| if [ ! -d "$LOG_DIR" ]; then | ||
| echo "Error: Log directory not found: $LOG_DIR" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [ ! -x "$DEDUP_BIN" ]; then | ||
| echo "Error: Dedup binary not found or not executable: $DEDUP_BIN" | ||
| exit 1 | ||
| fi |
| PKG_CHECK_MODULES([LOG4C], [log4c >= 1.2.3]) | ||
|
|
||
|
|
||
| dnl Log suppression feature |
Comment on lines
+79
to
+81
| #ifdef HAVE_LOG_SUPPRESSION | ||
| rdk_log_suppression_init(); | ||
| #endif |
Comment on lines
+199
to
+217
| if (elapsed > (double)cat->window_sec) { | ||
| /* Window expired - emit summary if we suppressed, then reset */ | ||
| if (entry->repeat_count > cat->threshold) { | ||
| uint32_t suppressed = entry->repeat_count - cat->threshold; | ||
| *repeat_count = suppressed; | ||
| } | ||
| cat->total_summaries++; | ||
| /* Reset entry for new window */ | ||
| entry->first_seen = now; | ||
| entry->last_seen = now; | ||
| entry->repeat_count = 1; | ||
| return RDK_SUPPRESS_EMIT_SUMMARY; | ||
| } | ||
| /* Window expired but below threshold - reset */ | ||
| entry->first_seen = now; | ||
| entry->last_seen = now; | ||
| entry->repeat_count = 1; | ||
| return RDK_SUPPRESS_PASS; | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+199
to
+217
| if (elapsed > (double)cat->window_sec) { | ||
| /* Window expired - emit summary if we suppressed, then reset */ | ||
| if (entry->repeat_count > cat->threshold) { | ||
| uint32_t suppressed = entry->repeat_count - cat->threshold; | ||
| *repeat_count = suppressed; | ||
| } | ||
| cat->total_summaries++; | ||
| /* Reset entry for new window */ | ||
| entry->first_seen = now; | ||
| entry->last_seen = now; | ||
| entry->repeat_count = 1; | ||
| return RDK_SUPPRESS_EMIT_SUMMARY; | ||
| } | ||
| /* Window expired but below threshold - reset */ | ||
| entry->first_seen = now; | ||
| entry->last_seen = now; | ||
| entry->repeat_count = 1; | ||
| return RDK_SUPPRESS_PASS; | ||
| } |
Comment on lines
+28
to
+31
| * Usage: rdk_log_dedup_sync <input_file> <output_file> [threshold] | ||
| * Exit codes: 0 = success, 1 = error, 2 = no changes needed | ||
|
|
||
| #include <stdio.h> |
Comment on lines
+60
to
+63
| /** | ||
| * Deinitialize the log suppression subsystem. | ||
| * Frees all allocated suppression state. | ||
| */ |
Comment on lines
+27
to
+30
| LOG_DIR="${1:-/rdklogs/logs}" | ||
| THRESHOLD="${2:-2}" | ||
| DEDUP_BIN="${DEDUP_BIN:-/usr/bin/rdkLogDedupSync}" | ||
| DEDUP_COUNT=0 |
Comment on lines
80
to
+84
| /* Perform Dynamic Logger Internal Init */ | ||
| rdk_dyn_log_init(); | ||
| #ifdef HAVE_LOG_SUPPRESSION | ||
| rdk_log_suppression_init(); | ||
| #endif |
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.
Summary
This PR implements a log duplicate suppression framework for rdk_logger to reduce log uploads from devices.
Changes
OpenSpec Proposal (openspec/changes/log-suppression/):
Core Implementation:
include/rdk_log_suppression.h: Public API for log suppression subsystemsrc/rdk_log_suppression.c: Core engine using FNV-1a hash and per-category ring buffer (8 slots)src/rdk_debug_priv.c: Integrated suppression check intordk_dbg_priv_log_msg()after priority filtersrc/rdk_logger_init.c: Wired init/deinit into logger lifecycleconfigure.ac: Added--enable-log-suppressionbuild flagsrc/Makefile.am: Added new source to library buildNvram Sync Dedup:
utils/rdk_log_dedup_sync.c: Pre-upload deduplication utilityscripts/logDedup.sh: Shell wrapper for batch log dedup before uploadKey Design Decisions:
Impact: