refactor(observability): Invoke observer related APIs directly from frontend#300
Merged
akila-i merged 3 commits intoopenchoreo:mainfrom Mar 1, 2026
Merged
refactor(observability): Invoke observer related APIs directly from frontend#300akila-i merged 3 commits intoopenchoreo:mainfrom
akila-i merged 3 commits intoopenchoreo:mainfrom
Conversation
…rontend Signed-off-by: Stefinie Fernando <minolispencer@gmail.com>
…ectly invoke obs API Signed-off-by: Stefinie Fernando <minolispencer@gmail.com>
…ary, and ObservabilityApi Signed-off-by: Stefinie Fernando <minolispencer@gmail.com>
stefinie123
requested review from
jhivandb,
kaviththiranga and
sameerajayasoma
as code owners
📝 WalkthroughWalkthroughThis PR refactors the observability request flow to enable direct calls from the frontend to observer and RCA agent APIs. It introduces a backend Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Frontend)
participant Backend as Backstage Backend
participant Resolver as Resolver Service
participant Observer as Observer API
participant RCA as RCA Agent
rect rgba(100, 150, 200, 0.5)
Note over Browser,RCA: Old Flow: All requests routed through backend
Browser->>Backend: GET /api/runtime-logs?componentId=...
Backend->>Resolver: Resolve observerUrl for env
Resolver-->>Backend: observerUrl
Backend->>Observer: GET /logs?componentId=...
Observer-->>Backend: logs data
Backend-->>Browser: logs data
end
rect rgba(150, 200, 100, 0.5)
Note over Browser,RCA: New Flow: Direct calls with URL caching
Browser->>Backend: GET /api/resolve-urls?namespace=...,env=...
Backend->>Resolver: Resolve observerUrl & rcaAgentUrl
Resolver-->>Backend: { observerUrl, rcaAgentUrl }
Backend-->>Browser: { observerUrl, rcaAgentUrl }
Note over Browser: Cache URLs (5min TTL)
Browser->>Observer: GET /logs (x-openchoreo-direct header)
Observer-->>Browser: logs data
Browser->>RCA: POST /chat (x-openchoreo-direct header)
RCA-->>Browser: chat response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts (1)
94-109:⚠️ Potential issue | 🟠 MajorRuntime logs are unnecessarily blocked by project UID lookup failures.
getRuntimeLogscurrently ignoresprojectIdinplugins/openchoreo-observability/src/api/ObservabilityApi.ts, but this hook still throws when/projectlookup fails. That creates avoidable runtime failures.💡 Suggested fix (best-effort UID lookup)
- const projectResponse = await fetchApi.fetch(projectUrl.toString()); - if (!projectResponse.ok) { - throw new Error('Failed to fetch project details'); - } - const projectData = await projectResponse.json(); - const projectId = projectData.uid ?? ''; + let projectId = ''; + try { + const projectResponse = await fetchApi.fetch(projectUrl.toString()); + if (projectResponse.ok) { + const projectData = await projectResponse.json(); + projectId = projectData.uid ?? ''; + } + } catch { + // Best effort: runtime logs call can proceed without projectId for now. + }Also applies to: 140-143
🤖 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 94 - 109, The current project UID lookup in useLogsSummary (the block that builds projectUrl, calls fetchApi.fetch, and sets projectId) throws an Error when the /project fetch fails, causing runtime logs to be blocked; change this to a best-effort lookup: catch fetch/network errors and non-ok responses, do not throw—log or warn using the existing logger and set projectId = '' (or undefined) so getRuntimeLogs in plugins/openchoreo-observability/src/api/ObservabilityApi.ts can continue to be called without a projectId; apply the same non-throwing fallback to the second occurrence at lines 140-143 so both lookups gracefully degrade.plugins/openchoreo-observability/src/api/RCAAgentApi.ts (1)
77-80:⚠️ Potential issue | 🟡 MinorUnguarded JSON parsing on error responses hides HTTP status context.
At lines 77-80,
response.json()is called without try-catch protection. If the server returns an error as plain text or HTML (common in proxies, gateways, or server errors), the JSON parsing throws aSyntaxError, masking the actual HTTP status and response body. This inconsistency exists in the same plugin—ObserverUrlCache.tsandObservabilityApi.tsalready wrap similar calls in try-catch blocks.Suggested fix
if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || `RCA chat failed: ${response.statusText}`); + let message = `RCA chat failed: ${response.status} ${response.statusText}`; + try { + const error = await response.json(); + message = error?.error || error?.message || message; + } catch { + // Keep status-based fallback + } + throw new Error(message); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/api/RCAAgentApi.ts` around lines 77 - 80, In RCAAgentApi (the fetch/error-handling block that currently calls response.json()), guard the JSON parse with a try/catch: attempt to parse response.json() and if it fails fall back to await response.text(); then throw an Error that includes the HTTP status (response.status and response.statusText) plus either the parsed error.error message (if JSON and present) or the raw text body, so non-JSON error responses (HTML/plain text) don’t cause a SyntaxError that hides the HTTP status.
🧹 Nitpick comments (2)
packages/app/src/apis/OpenChoreoFetchApi.ts (1)
11-11: Consider importingOPENCHOREO_TOKEN_HEADERfrom the shared package.This constant is already defined and exported from
packages/openchoreo-auth/src/types.ts. Duplicating it here risks inconsistency if the value changes in one place but not the other.♻️ Suggested refactor
import { ConfigApi, FetchApi, IdentityApi, OAuthApi, } from '@backstage/core-plugin-api'; +import { OPENCHOREO_TOKEN_HEADER } from '@internal/openchoreo-auth'; -/** - * Header name used to pass the user's IDP token to backend services. - */ -const OPENCHOREO_TOKEN_HEADER = 'x-openchoreo-token';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/apis/OpenChoreoFetchApi.ts` at line 11, Replace the locally defined OPENCHOREO_TOKEN_HEADER constant in OpenChoreoFetchApi.ts with an import of the shared export to avoid duplication; remove the const declaration and import OPENCHOREO_TOKEN_HEADER from the module that exports it (the shared types export), then update any usages in OpenChoreoFetchApi to reference the imported symbol. Ensure imports compile by updating the import list at the top of the file and run type checks to confirm no other references remain to the removed local constant.plugins/openchoreo-observability/src/api/ObserverUrlCache.ts (1)
14-14: TTL cache currently has no eviction path for expired keys.Expired entries are never removed unless the same key is requested again. Over time, many unique namespace/environment pairs can accumulate in memory.
♻️ Suggested refinement
const cached = this.cache.get(cacheKey); - if (cached && Date.now() < cached.expiresAt) { - return { - observerUrl: cached.observerUrl, - rcaAgentUrl: cached.rcaAgentUrl, - }; + if (cached) { + if (Date.now() < cached.expiresAt) { + return { + observerUrl: cached.observerUrl, + rcaAgentUrl: cached.rcaAgentUrl, + }; + } + this.cache.delete(cacheKey); }Also applies to: 26-33, 64-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/api/ObserverUrlCache.ts` at line 14, ObserverUrlCache's private readonly cache (Map<string, CachedUrls>) currently never evicts expired entries unless re-requested; add an automatic eviction path by either scheduling per-entry timeouts when inserting CachedUrls or by adding a periodic cleanup task in the ObserverUrlCache constructor that iterates over cache entries and deletes those whose expiresAt/ttl indicate they are expired. Update the code paths that insert into the cache (where CachedUrls are created) to set the timeout or to ensure the periodic sweeper will remove them, and ensure the cleanup task is cleared on shutdown/destruction to avoid leaks.
🤖 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-backend/src/router.ts`:
- Around line 35-46: The query params namespaceName and environmentName must be
validated as single strings (not arrays) before calling
observabilityService.resolveUrls; update the handler to check typeof
namespaceName === 'string' and typeof environmentName === 'string' (or
explicitly reject Array.isArray values) and return a 400 JSON error if they are
not valid single strings, then cast and pass those validated string values to
resolveUrls; reference getUserTokenFromRequest and
observabilityService.resolveUrls when making the change.
In `@plugins/openchoreo-observability/src/api/ObservabilityApi.ts`:
- Around line 307-309: The forEach callback currently uses an implicit-return
arrow which causes the callback to return a value; change the callback to a
block-bodied arrow so it does not return anything (e.g., update
componentUids.forEach(uid => url.searchParams.append('componentUids', uid)) to
use a block: componentUids.forEach(uid => {
url.searchParams.append('componentUids', uid); }); this keeps the invocation in
the ObservabilityApi code (the componentUids.forEach and url.searchParams.append
calls) and satisfies the useIterableCallbackReturn lint rule.
In `@plugins/openchoreo-observability/src/api/ObserverUrlCache.ts`:
- Line 24: The resolver in ObserverUrlCache currently hard-throws if observerUrl
is missing even though RCA flows may supply only rcaAgentUrl; update the method
(in class ObserverUrlCache) so it does not require observerUrl alone: change the
validation to only throw when both observerUrl and rcaAgentUrl are absent,
return the available rcaAgentUrl when observerUrl is undefined, and
remove/adjust any unconditional throws that reference observerUrl (also apply
the same fix at the other occurrence around the code handling lines 60–62).
Ensure the returned shape still matches Promise<{ observerUrl?: string;
rcaAgentUrl?: string }> or keep the existing return type but allow observerUrl
to be undefined when rcaAgentUrl exists.
---
Outside diff comments:
In `@plugins/openchoreo-observability/src/api/RCAAgentApi.ts`:
- Around line 77-80: In RCAAgentApi (the fetch/error-handling block that
currently calls response.json()), guard the JSON parse with a try/catch: attempt
to parse response.json() and if it fails fall back to await response.text();
then throw an Error that includes the HTTP status (response.status and
response.statusText) plus either the parsed error.error message (if JSON and
present) or the raw text body, so non-JSON error responses (HTML/plain text)
don’t cause a SyntaxError that hides the HTTP status.
In
`@plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts`:
- Around line 94-109: The current project UID lookup in useLogsSummary (the
block that builds projectUrl, calls fetchApi.fetch, and sets projectId) throws
an Error when the /project fetch fails, causing runtime logs to be blocked;
change this to a best-effort lookup: catch fetch/network errors and non-ok
responses, do not throw—log or warn using the existing logger and set projectId
= '' (or undefined) so getRuntimeLogs in
plugins/openchoreo-observability/src/api/ObservabilityApi.ts can continue to be
called without a projectId; apply the same non-throwing fallback to the second
occurrence at lines 140-143 so both lookups gracefully degrade.
---
Nitpick comments:
In `@packages/app/src/apis/OpenChoreoFetchApi.ts`:
- Line 11: Replace the locally defined OPENCHOREO_TOKEN_HEADER constant in
OpenChoreoFetchApi.ts with an import of the shared export to avoid duplication;
remove the const declaration and import OPENCHOREO_TOKEN_HEADER from the module
that exports it (the shared types export), then update any usages in
OpenChoreoFetchApi to reference the imported symbol. Ensure imports compile by
updating the import list at the top of the file and run type checks to confirm
no other references remain to the removed local constant.
In `@plugins/openchoreo-observability/src/api/ObserverUrlCache.ts`:
- Line 14: ObserverUrlCache's private readonly cache (Map<string, CachedUrls>)
currently never evicts expired entries unless re-requested; add an automatic
eviction path by either scheduling per-entry timeouts when inserting CachedUrls
or by adding a periodic cleanup task in the ObserverUrlCache constructor that
iterates over cache entries and deletes those whose expiresAt/ttl indicate they
are expired. Update the code paths that insert into the cache (where CachedUrls
are created) to set the timeout or to ensure the periodic sweeper will remove
them, and ensure the cleanup task is cleared on shutdown/destruction to avoid
leaks.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/app/src/apis/OpenChoreoFetchApi.tsplugins/openchoreo-observability-backend/src/plugin.test.tsplugins/openchoreo-observability-backend/src/plugin.tsplugins/openchoreo-observability-backend/src/router.test.tsplugins/openchoreo-observability-backend/src/router.tsplugins/openchoreo-observability-backend/src/services/ObservabilityService.tsplugins/openchoreo-observability/src/api/ObservabilityApi.tsplugins/openchoreo-observability/src/api/ObserverUrlCache.tsplugins/openchoreo-observability/src/api/RCAAgentApi.tsplugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts
💤 Files with no reviewable changes (1)
- plugins/openchoreo-observability-backend/src/plugin.ts
akila-i
approved these changes
Mar 1, 2026
This was referenced Mar 1, 2026
This was referenced Mar 9, 2026
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
2 participants
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.
Purpose
Resolves: openchoreo/openchoreo#2223
Goals
Refactor code to invoke observer related APIs directly from frontend.
Approach
This pull request refactors the OpenChoreo Observability backend to streamline service dependencies and update the test and router logic to match new API endpoints. It removes references to the
rcaAgentServicethroughout the backend plugin and test files, and replaces the previous metrics-focused endpoint tests with new ones for resolving observer URLs. Additionally, it introduces and tests a new/resolve-urlsendpoint and updates the custom fetch API to support a "direct mode" for external API calls.Observability Backend Refactor
rcaAgentServicefrom the backend plugin, router, and test files, simplifying service dependencies and initialization logic (plugin.ts,router.ts,router.test.ts) [1] [2] [3] [4] [5] [6] [7] [8].API Endpoint and Test Updates
/resolve-urlsendpoint, including success, error, and missing parameter scenarios (plugin.test.ts,router.test.ts) [1] [2] [3] [4].resolveUrlsmethod from the observability service, and removed all mock metrics data and related tests (plugin.test.ts,router.test.ts) [1] [2] [3].Custom Fetch API Enhancement
OpenChoreoFetchApi, allowing requests to bypass Backstage backend authentication and send the user's IDP token directly in the Authorization header for external API calls (OpenChoreoFetchApi.ts) [1] [2].OpenChoreoFetchApi.ts).Error Handling Improvements
/resolve-urlsendpoint, ensuring correct status codes and error messages for various failure scenarios (plugin.test.ts,router.test.ts) [1] [2] [3].Environment Endpoint Test
/environmentsendpoint, including validation for missing parameters (router.test.ts).Summary by CodeRabbit
Release Notes
Improvements
Backend Changes