fix: add 'all' option to loglevel filter and update default of time range filter as 10mins#486
Conversation
📝 WalkthroughWalkthroughAdds an "ALL" sentinel to log-level selection, implements toggling to select/clear all log levels, introduces clearLogs to reset log state when no levels are selected, adjusts fetch behavior to avoid requesting when logLevels is empty, and changes default timeRange from "1h" to "10m" across URL/filter hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant LogsFilter as LogsFilter (UI)
participant Page as ObservabilityRuntimeLogsPage
participant URLHook as useUrlFiltersForRuntimeLogs
participant RuntimeHook as useProjectRuntimeLogs
participant API as Runtime Logs API
User->>LogsFilter: Click "ALL" / toggle
LogsFilter->>LogsFilter: handleAllLogLevelsToggle()
LogsFilter->>Page: onFiltersChange({ logLevel: [] } or full list)
Page->>URLHook: updateFilters(...)
URLHook->>URLHook: write logLevel (remove if all / empty string if none)
Page->>Page: useEffect observes filters change
alt filters.logLevel length === 0
Page->>RuntimeHook: clearLogs()
RuntimeHook->>RuntimeHook: reset logs, totalCount, hasMore, bump generation
else
Page->>RuntimeHook: fetchLogs(true)
RuntimeHook->>API: GET /logs?logLevels=[...]
API-->>RuntimeHook: return logs
RuntimeHook->>RuntimeHook: if generation matches -> update logs state
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts (1)
43-49: Consider validating parsed log levels againstLOG_LEVELS.The parsing logic doesn't validate that parsed levels are actually valid log levels. If a user manually edits the URL with invalid values (e.g.,
?logLevel=INVALID,ERROR), they'd pass through to filters. While the backend likely ignores invalid levels, validating here would provide consistency with the serialization logic at lines 134-136 which explicitly checks againstLOG_LEVELS.🛡️ Optional defensive fix
const logLevel = logLevelParam === null ? [...LOG_LEVELS] : logLevelParam .split(',') .map(level => level.trim()) - .filter(Boolean); + .filter(level => LOG_LEVELS.includes(level as any));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts` around lines 43 - 49, The current parsing of logLevelParam produces values without validating them against LOG_LEVELS; update the logic in useUrlFiltersForRuntimeLogs (the logLevel variable construction using logLevelParam) to filter the parsed levels so only entries present in LOG_LEVELS are kept (e.g., after .map(level => level.trim()) add .filter(level => LOG_LEVELS.includes(level)) ), while preserving the fallback when logLevelParam is null to return [...LOG_LEVELS]. This ensures parsed URL values match the same validation used during serialization.plugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.ts (1)
116-119: Inconsistent "all levels selected" check compared to URL filter hook.This check only compares array length, whereas
useUrlFiltersForRuntimeLogs.tsat lines 134-136 uses a thorough check withLOG_LEVELS.every(l => newFilters.logLevel!.includes(l)). Iffilters.logLevelsomehow contains invalid values with the same count asLOG_LEVELS, this would incorrectly send an empty array to the backend.Consider aligning the validation logic for consistency:
♻️ Proposed fix to align validation
// If all log levels are selected, pass an empty array to reduce search complexity on the backend const queryOptions = { limit, startTime, endTime, logLevels: - filters.logLevel.length === LOG_LEVELS.length + filters.logLevel.length === LOG_LEVELS.length && + LOG_LEVELS.every(l => filters.logLevel.includes(l)) ? [] : filters.logLevel,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.ts` around lines 116 - 119, The current check in useProjectRuntimeLogs (building logLevels from filters.logLevel) only compares lengths and can misclassify invalid entries as "all selected"; update it to mirror useUrlFiltersForRuntimeLogs by validating with LOG_LEVELS.every(l => filters.logLevel!.includes(l)) (or equivalent) so you only send an empty array when every known level is present, referencing the symbols filters.logLevel and LOG_LEVELS in the transformation logic inside useProjectRuntimeLogs.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsx`:
- Around line 80-82: The mapping of logLevels (logLevels:
filters.logLevel.length === LOG_LEVELS.length ? [] : filters.logLevel) collapses
both “all selected” and “none selected” to [], causing useRuntimeLogs and its
refresh/poll paths to treat “none selected” as “all” and fetch logs; change the
logic to preserve a distinct sentinel for “none selected” (e.g., use null or a
dedicated flag) or pass the full filters.logLevel array plus an explicit
noLogLevelsSelected boolean, then update the consumer hook useRuntimeLogs and
any fetch/poll entrypoints (including the refresh/live handlers) to check that
noLogLevelsSelected guard before issuing requests so that an empty selection
does not trigger fetches.
In `@plugins/openchoreo-observability/src/hooks/useRuntimeLogs.ts`:
- Around line 312-316: clearLogs currently resets state but doesn't stop or
invalidate in-flight fetches from fetchLogs, so stale results can overwrite the
cleared state; modify clearLogs to invalidate ongoing requests by either (a)
incrementing a shared request version/token (e.g., a ref like fetchVersionRef)
and have fetchLogs check that token before applying results, or (b) store and
abort the current AbortController and create a new controller when clearing;
update fetchLogs to respect the version/AbortController and skip calling
setLogs/setTotalCount/setHasMore if the request is stale or aborted (refer to
clearLogs and fetchLogs identifiers).
In `@plugins/openchoreo-observability/src/hooks/useUrlFilters.ts`:
- Line 5: Update the stale JSDoc default to match the runtime constant
DEFAULT_TIME_RANGE (now '10m') in useUrlFilters.ts: find the JSDoc that still
references '1h' and change that text to '10m' (or reference DEFAULT_TIME_RANGE)
so the comment aligns with the actual DEFAULT_TIME_RANGE constant and the
behavior of useUrlFilters.
In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts`:
- Around line 23-24: Update the JSDoc for useUrlFiltersForRuntimeLogs to reflect
the current default timeRange value: change the documented default from '1h' to
'10m' (the comment that currently reads "- `timeRange`: Time range value
(defaults to '1h')" should be updated to "- `timeRange`: Time range value
(defaults to '10m')"); ensure the rest of the param descriptions (e.g.,
`logLevel`) remain unchanged so the docs match the implementation.
---
Nitpick comments:
In `@plugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.ts`:
- Around line 116-119: The current check in useProjectRuntimeLogs (building
logLevels from filters.logLevel) only compares lengths and can misclassify
invalid entries as "all selected"; update it to mirror
useUrlFiltersForRuntimeLogs by validating with LOG_LEVELS.every(l =>
filters.logLevel!.includes(l)) (or equivalent) so you only send an empty array
when every known level is present, referencing the symbols filters.logLevel and
LOG_LEVELS in the transformation logic inside useProjectRuntimeLogs.ts.
In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts`:
- Around line 43-49: The current parsing of logLevelParam produces values
without validating them against LOG_LEVELS; update the logic in
useUrlFiltersForRuntimeLogs (the logLevel variable construction using
logLevelParam) to filter the parsed levels so only entries present in LOG_LEVELS
are kept (e.g., after .map(level => level.trim()) add .filter(level =>
LOG_LEVELS.includes(level)) ), while preserving the fallback when logLevelParam
is null to return [...LOG_LEVELS]. This ensures parsed URL values match the same
validation used during serialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8770909c-6caa-4c01-ba6f-9abef3a8d187
📒 Files selected for processing (10)
plugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsxplugins/openchoreo-observability/src/hooks/useFilters.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useUrlFilters.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts
plugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsx
Outdated
Show resolved
Hide resolved
plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts`:
- Around line 43-49: When parsing logLevelParam in
useUrlFiltersForRuntimeLogs.ts, ensure the resulting logLevel is deduplicated
and normalized to the canonical values/order from LOG_LEVELS: split and trim the
param, map each token to the canonical string from LOG_LEVELS (e.g., find the
matching item in LOG_LEVELS to normalize casing/value), remove duplicates (e.g.,
via Set or filter by first index), and finally sort the array according to the
order of LOG_LEVELS so the result is a canonical, deduplicated list assigned to
logLevel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35d930cf-dbd4-4b5c-952a-2d2c5fe4c819
📒 Files selected for processing (10)
plugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsxplugins/openchoreo-observability/src/hooks/useFilters.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useUrlFilters.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts
✅ Files skipped from review due to trivial changes (2)
- plugins/openchoreo-observability/src/hooks/useUrlFilters.ts
- plugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/openchoreo-observability/src/hooks/useFilters.ts
- plugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.tsx
- plugins/openchoreo-observability/src/hooks/useRuntimeLogs.ts
- plugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.ts
- plugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.ts
- plugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsx
plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Akila-I <akila.99g@gmail.com>
Signed-off-by: Akila-I <akila.99g@gmail.com>
Signed-off-by: Akila-I <akila.99g@gmail.com>
Added to the PR description |
Purpose
Related: openchoreo/openchoreo#2970
This pull request introduces several improvements to the runtime logs filtering experience in the OpenChoreo Observability plugin. The main focus is on enhancing the log level filter UI and logic: users can now easily select or deselect all log levels, and the filter state is more accurately reflected in both the UI and URL. Additionally, the default time range for filters is standardized to '10m' across the codebase. The changes also include optimizations to log fetching and clearing logic to ensure consistent behavior when filters change.
Log Level Filter Improvements
LogsFilter, allowing users to quickly select or deselect all log levels. The UI now clearly reflects when all, some, or none are selected, and prevents redundant state updates when "All" is toggled. [1] [2] [3]Filter State Management and URL Handling
logLevelparameter is omitted from the URL (treated as default); if none are selected, it's explicitly set as empty. The initial state for log levels defaults to all selected if not specified in the URL. [1] [2] [3] [4]Default Time Range Standardization
'1h'to'10m'across all relevant hooks and utilities, ensuring consistency throughout the application. [1] [2] [3] [4] [5] [6]Log Fetching and Clearing Enhancements
clearLogsmethods to log hooks, and updated components to clear logs when filters result in no log levels selected, preventing unnecessary API calls and stale data. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]These changes collectively improve the usability, clarity, and reliability of the runtime logs filtering experience.
Goals
Approach
Screen.Recording.2026-04-02.at.13.39.22.mov
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Bug Fixes