refactor: update observability filters to use component names instead of IDs#494
refactor: update observability filters to use component names instead of IDs#494LakshanSS merged 5 commits intoopenchoreo:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR renames filter fields (environmentId → environment, componentIds → components), switches environment URL handling to use environment.name, removes componentId/projectId/environmentId from getRuntimeLogs signature and callers, adds Environment.displayName, and updates all affected components, hooks, and tests accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
plugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsx (1)
136-156:⚠️ Potential issue | 🟡 MinorRender selected component labels with display names too.
The menu items now use
component.displayName || component.name, but the closed multiselect still joins the raw stored names. After selection, the field flips back to internal names instead of the clearer labels this PR is introducing.💡 Suggested fix
renderValue={selected => { const selectedArray = selected as string[]; if (selectedArray.length === 0) return 'All'; - return selectedArray.join(', '); + return selectedArray + .map( + name => + components.find(component => component.name === name) + ?.displayName || name, + ) + .join(', '); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsx` around lines 136 - 156, The closed multiselect currently shows raw internal names because renderValue in the Select uses selected strings directly; update renderValue to map each selected name to its user-friendly label by looking it up in the components array (use component.displayName || component.name) before joining, preserving the "All" when selected length is 0; reference the Select's renderValue, the components array, filters.components and MenuItem/component.displayName to locate and implement the change (ensure proper typing/casting of selected as string[] and fallback to name when no displayName).plugins/openchoreo-observability/src/hooks/useTraces.ts (1)
54-68:⚠️ Potential issue | 🟡 MinorDeduplicate component filters before firing parallel trace requests.
A duplicated
componentsentry in URL/state will now enqueue the samegetTraces()call more than once. The later merge bytraceIdhides it, but it still adds avoidable backend load.💡 Suggested fix
- const selectedComponents = filters.components ?? []; + const selectedComponents = Array.from( + new Set(filters.components ?? []), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/hooks/useTraces.ts` around lines 54 - 68, Deduplicate the component list before firing parallel requests: derive selectedComponents from filters.components, remove duplicates (e.g., via Set or Array.filter) and use that deduped array in the Promise.all call to observabilityApi.getTraces so each component name is requested only once; update references to selectedComponents (the map that calls observabilityApi.getTraces(namespace, projectName, filters.environment.name, name, ...)) to use the deduped array.plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts (1)
97-100:⚠️ Potential issue | 🟠 MajorStop sending the legacy environment field to
getRuntimeLogs.Line 100 still passes
selectedEnv.resourceName, but the rest of this refactor moves observability lookups to the canonical environment name. That keeps the overview card pinned to the old shape and will break onceclient.getEnvironments(entity)returns{ name, displayName }like the other observability paths. Please migrate this hook to the new environment model and pass the environment name here before merging.Proposed fix
- selectedEnv.resourceName, + selectedEnv.name,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts` around lines 97 - 100, The hook useLogsSummary is still passing the legacy environment field selectedEnv.resourceName into observabilityApi.getRuntimeLogs; update the hook to use the canonical environment model and pass the environment name (e.g., selectedEnv.name or the new environment property returned by client.getEnvironments) instead of resourceName when calling observabilityApi.getRuntimeLogs, and adjust any intermediate variables or types within useLogsSummary that build the environment argument so the overview card uses the new environment shape across this hook.
🧹 Nitpick comments (3)
plugins/openchoreo-observability/src/hooks/useRuntimeLogs.ts (1)
162-169: Avoid starting live polling when environment is unset.When
isLiveis true butoptions.environmentis empty, the interval still runs every 5s and only no-ops insidefetchLogs. Consider gating interval setup here too.♻️ Suggested change
- if (!options.isLive) { + if (!options.isLive || !options.environment) { return undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/hooks/useRuntimeLogs.ts` around lines 162 - 169, The live-polling interval is started whenever options.isLive is true even if options.environment is empty, causing a pointless interval that only no-ops in fetchLogs; update the polling setup in useRuntimeLogs to only create the interval when both options.isLive and options.environment are truthy (or early-return when options.environment is falsy), use the same pollingIntervalRef.current reference to set/clear the timer, and ensure you clear any existing interval when environment becomes empty so fetchLogs is only polled while options.environment is set.plugins/openchoreo-observability/src/hooks/useUrlFilters.ts (1)
18-18: Minor: Update JSDoc to reflect the rename from IDs to names.The comment says "Comma-separated component IDs" but the PR refactors to use component names instead of IDs.
📝 Suggested documentation fix
- * - `components`: Comma-separated component IDs + * - `components`: Comma-separated component names🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/hooks/useUrlFilters.ts` at line 18, Update the JSDoc in useUrlFilters.ts to reflect that the query param now uses component names rather than IDs: change the `components` description from "Comma-separated component IDs" to "Comma-separated component names" in the JSDoc for the useUrlFilters hook (the `components` query param description) so documentation matches the refactor.plugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.ts (1)
13-15: Remove the duplicatecomponentsdefinition.
RuntimeLogsFiltersalready includescomponents?: string[], so redefining it here adds a second place to keep in sync without changing behavior.♻️ Suggested simplification
-export interface ProjectRuntimeLogsFilters extends RuntimeLogsFilters { - components?: string[]; -} +export type ProjectRuntimeLogsFilters = RuntimeLogsFilters;🤖 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 13 - 15, ProjectRuntimeLogsFilters redundantly redeclares components?: string[] even though RuntimeLogsFilters already defines it; remove the components property from the ProjectRuntimeLogsFilters interface so it simply extends RuntimeLogsFilters, leaving any other custom properties (if any) intact and ensuring code that references ProjectRuntimeLogsFilters continues to rely on the inherited components field from RuntimeLogsFilters.
🤖 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 60-63: The components parsing keeps whitespace so values like
"foo, bar" yield " bar" and won't match downstream; update the logic in
useUrlFiltersForRuntimeLogs (where componentsParam and components are computed)
to trim each entry before filtering/using it (e.g., split on ',' then map(s =>
s.trim()) then filter(Boolean)) so entries are normalized the same way as
logLevel.
---
Outside diff comments:
In `@plugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsx`:
- Around line 136-156: The closed multiselect currently shows raw internal names
because renderValue in the Select uses selected strings directly; update
renderValue to map each selected name to its user-friendly label by looking it
up in the components array (use component.displayName || component.name) before
joining, preserving the "All" when selected length is 0; reference the Select's
renderValue, the components array, filters.components and
MenuItem/component.displayName to locate and implement the change (ensure proper
typing/casting of selected as string[] and fallback to name when no
displayName).
In `@plugins/openchoreo-observability/src/hooks/useTraces.ts`:
- Around line 54-68: Deduplicate the component list before firing parallel
requests: derive selectedComponents from filters.components, remove duplicates
(e.g., via Set or Array.filter) and use that deduped array in the Promise.all
call to observabilityApi.getTraces so each component name is requested only
once; update references to selectedComponents (the map that calls
observabilityApi.getTraces(namespace, projectName, filters.environment.name,
name, ...)) to use the deduped array.
In
`@plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts`:
- Around line 97-100: The hook useLogsSummary is still passing the legacy
environment field selectedEnv.resourceName into observabilityApi.getRuntimeLogs;
update the hook to use the canonical environment model and pass the environment
name (e.g., selectedEnv.name or the new environment property returned by
client.getEnvironments) instead of resourceName when calling
observabilityApi.getRuntimeLogs, and adjust any intermediate variables or types
within useLogsSummary that build the environment argument so the overview card
uses the new environment shape across this hook.
---
Nitpick comments:
In `@plugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.ts`:
- Around line 13-15: ProjectRuntimeLogsFilters redundantly redeclares
components?: string[] even though RuntimeLogsFilters already defines it; remove
the components property from the ProjectRuntimeLogsFilters interface so it
simply extends RuntimeLogsFilters, leaving any other custom properties (if any)
intact and ensuring code that references ProjectRuntimeLogsFilters continues to
rely on the inherited components field from RuntimeLogsFilters.
In `@plugins/openchoreo-observability/src/hooks/useRuntimeLogs.ts`:
- Around line 162-169: The live-polling interval is started whenever
options.isLive is true even if options.environment is empty, causing a pointless
interval that only no-ops in fetchLogs; update the polling setup in
useRuntimeLogs to only create the interval when both options.isLive and
options.environment are truthy (or early-return when options.environment is
falsy), use the same pollingIntervalRef.current reference to set/clear the
timer, and ensure you clear any existing interval when environment becomes empty
so fetchLogs is only polled while options.environment is set.
In `@plugins/openchoreo-observability/src/hooks/useUrlFilters.ts`:
- Line 18: Update the JSDoc in useUrlFilters.ts to reflect that the query param
now uses component names rather than IDs: change the `components` description
from "Comma-separated component IDs" to "Comma-separated component names" in the
JSDoc for the useUrlFilters hook (the `components` query param description) so
documentation matches the refactor.
🪄 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: 314ff97a-8968-4f5a-a030-6a417c9e0a90
📒 Files selected for processing (39)
plugins/openchoreo-observability-backend/src/services/ObservabilityService.tsplugins/openchoreo-observability/src/api/ObservabilityApi.tsplugins/openchoreo-observability/src/components/Alerts/AlertsActions.test.tsxplugins/openchoreo-observability/src/components/Alerts/AlertsFilter.test.tsxplugins/openchoreo-observability/src/components/Alerts/AlertsFilter.tsxplugins/openchoreo-observability/src/components/Alerts/ObservabilityAlertsPage.test.tsxplugins/openchoreo-observability/src/components/Alerts/ObservabilityAlertsPage.tsxplugins/openchoreo-observability/src/components/Alerts/types.tsplugins/openchoreo-observability/src/components/Incidents/IncidentsActions.test.tsxplugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.test.tsxplugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.tsxplugins/openchoreo-observability/src/components/Incidents/ObservabilityProjectIncidentsPage.test.tsxplugins/openchoreo-observability/src/components/Incidents/ObservabilityProjectIncidentsPage.tsxplugins/openchoreo-observability/src/components/Incidents/types.tsplugins/openchoreo-observability/src/components/Metrics/ObservabilityMetricsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsActions.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/types.tsplugins/openchoreo-observability/src/components/Traces/ObservabilityTracesPage.tsxplugins/openchoreo-observability/src/components/Traces/TracesFilters.tsxplugins/openchoreo-observability/src/hooks/useComponentAlerts.tsplugins/openchoreo-observability/src/hooks/useFilters.tsplugins/openchoreo-observability/src/hooks/useProjectIncidents.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.test.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useTraces.test.tsplugins/openchoreo-observability/src/hooks/useTraces.tsplugins/openchoreo-observability/src/hooks/useUrlFilters.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.tsplugins/openchoreo-observability/src/types.tsplugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts
💤 Files with no reviewable changes (1)
- plugins/openchoreo-observability/src/api/ObservabilityApi.ts
4062f55 to
2b0ac3a
Compare
Signed-off-by: Akila-I <akila.99g@gmail.com>
…nd hooks - Changed environmentId to environment in incidents filter and related components. - Updated tests to reflect the new environment property. - Adjusted hooks and API calls to use the new environment structure. - Modified environment mapping in runtime logs and incidents to use displayName. - Ensured consistency across filters and state management for environment selection. 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>
…components Signed-off-by: Akila-I <akila.99g@gmail.com>
2b0ac3a to
71c6625
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts (1)
97-108:⚠️ Potential issue | 🔴 CriticalFix
selectedEnv.resourceName—Environmenttype no longer has this property.The
Environmentinterface was updated to removeresourceName. Line 100 attempts to accessselectedEnv.resourceNamewhen callinggetRuntimeLogs(), but the field no longer exists. UseselectedEnv.nameinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts` around lines 97 - 108, The call to observabilityApi.getRuntimeLogs in useLogsSummary.ts uses selectedEnv.resourceName which no longer exists on Environment; update the argument to use selectedEnv.name instead (i.e., replace selectedEnv.resourceName with selectedEnv.name) so getRuntimeLogs receives the environment name; ensure any related references in the useLogsSummary function and its parameters reflect the Environment type change and compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts`:
- Around line 97-108: The call to observabilityApi.getRuntimeLogs in
useLogsSummary.ts uses selectedEnv.resourceName which no longer exists on
Environment; update the argument to use selectedEnv.name instead (i.e., replace
selectedEnv.resourceName with selectedEnv.name) so getRuntimeLogs receives the
environment name; ensure any related references in the useLogsSummary function
and its parameters reflect the Environment type change and compile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d336e578-f4e3-4c57-abea-ecd3983f5be7
📒 Files selected for processing (39)
plugins/openchoreo-observability-backend/src/services/ObservabilityService.tsplugins/openchoreo-observability/src/api/ObservabilityApi.tsplugins/openchoreo-observability/src/components/Alerts/AlertsActions.test.tsxplugins/openchoreo-observability/src/components/Alerts/AlertsFilter.test.tsxplugins/openchoreo-observability/src/components/Alerts/AlertsFilter.tsxplugins/openchoreo-observability/src/components/Alerts/ObservabilityAlertsPage.test.tsxplugins/openchoreo-observability/src/components/Alerts/ObservabilityAlertsPage.tsxplugins/openchoreo-observability/src/components/Alerts/types.tsplugins/openchoreo-observability/src/components/Incidents/IncidentsActions.test.tsxplugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.test.tsxplugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.tsxplugins/openchoreo-observability/src/components/Incidents/ObservabilityProjectIncidentsPage.test.tsxplugins/openchoreo-observability/src/components/Incidents/ObservabilityProjectIncidentsPage.tsxplugins/openchoreo-observability/src/components/Incidents/types.tsplugins/openchoreo-observability/src/components/Metrics/ObservabilityMetricsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsActions.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.test.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/types.tsplugins/openchoreo-observability/src/components/Traces/ObservabilityTracesPage.tsxplugins/openchoreo-observability/src/components/Traces/TracesFilters.tsxplugins/openchoreo-observability/src/hooks/useComponentAlerts.tsplugins/openchoreo-observability/src/hooks/useFilters.tsplugins/openchoreo-observability/src/hooks/useProjectIncidents.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.test.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useTraces.test.tsplugins/openchoreo-observability/src/hooks/useTraces.tsplugins/openchoreo-observability/src/hooks/useUrlFilters.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.tsplugins/openchoreo-observability/src/types.tsplugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts
💤 Files with no reviewable changes (1)
- plugins/openchoreo-observability/src/api/ObservabilityApi.ts
✅ Files skipped from review due to trivial changes (11)
- plugins/openchoreo-observability/src/components/RuntimeLogs/LogsActions.test.tsx
- plugins/openchoreo-observability/src/components/Alerts/ObservabilityAlertsPage.test.tsx
- plugins/openchoreo-observability/src/components/Incidents/IncidentsActions.test.tsx
- plugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.test.tsx
- plugins/openchoreo-observability/src/components/Alerts/AlertsFilter.test.tsx
- plugins/openchoreo-observability/src/hooks/useTraces.test.ts
- plugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityRuntimeLogsPage.test.tsx
- plugins/openchoreo-observability/src/types.ts
- plugins/openchoreo-observability/src/hooks/useTraces.ts
- plugins/openchoreo-observability/src/components/Incidents/types.ts
- plugins/openchoreo-observability/src/components/Incidents/ObservabilityProjectIncidentsPage.test.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- plugins/openchoreo-observability/src/components/Alerts/AlertsFilter.tsx
- plugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.test.tsx
- plugins/openchoreo-observability/src/components/Traces/TracesFilters.tsx
- plugins/openchoreo-observability/src/components/Alerts/AlertsActions.test.tsx
- plugins/openchoreo-observability/src/components/Alerts/types.ts
- plugins/openchoreo-observability/src/hooks/useComponentAlerts.ts
- plugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.tsx
- plugins/openchoreo-observability-backend/src/services/ObservabilityService.ts
- plugins/openchoreo-observability/src/components/RuntimeLogs/ObservabilityProjectRuntimeLogsPage.tsx
- plugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.ts
- plugins/openchoreo-observability/src/components/Alerts/ObservabilityAlertsPage.tsx
Purpose
This pull request refactors the way environments and components are represented and filtered in the Observability plugin, aiming for greater consistency and clarity across the codebase. The most significant changes include standardizing environment and component identifiers, updating filter interfaces and usage, and improving environment display names throughout the UI and tests.
Filter and Environment Refactoring:
AlertsFiltersandIncidentsFiltersinterfaces now useenvironment(string) instead ofenvironmentId, andcomponents(string array) instead ofcomponentIds, for clarity and consistency across the codebase. All relevant usages in components, handlers, and tests have been updated accordingly. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19]nameanddisplayNamefields, removing the previousidandresourceNamefields, and this is reflected in both the UI components and the test data. [1] [2] [3] [4]UI and Display Improvements:
displayNameif available, falling back tonameotherwise, providing a more user-friendly label throughout the UI. [1] [2]ObservabilityNotConfiguredErrornow usescomponentNameinstead ofcomponentIdfor clearer error messages.Test Data and Mocks Update:
Type and Import Clean-up:
Environmenttype have been updated to use a centralized definition, reducing duplication and potential for drift. [1] [2] [3] [4]API Parameter Simplification:
getRuntimeLogsAPI and client now accept environment and project names directly, removing the need for separate IDs, which simplifies the API surface. [1] [2]These changes collectively make the codebase more maintainable and ensure consistency in how environments and components are referenced and displayed throughout the Observability plugin.
Goals
Approach
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
Refactor