-
Notifications
You must be signed in to change notification settings - Fork 46
feat: add Delivery Insights (DORA metrics) page to the sidebar #729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
LakshanSS
wants to merge
12
commits into
openchoreo:main
Choose a base branch
from
LakshanSS:laki-dora-2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
1632bb1
feat: add Delivery Insights (DORA metrics) UI at namespace, project, …
LakshanSS a0a350c
fix: keep KPI tile footer text clear of the corner sparkline
LakshanSS fe6b016
Merge remote-tracking branch 'upstream/main' into laki-dora-1
LakshanSS 826de56
chore: fix prettier formatting
LakshanSS 475639f
test: fix resolve-urls test to expect namespaceName-only validation
LakshanSS 983dc6c
fix: partition the namespace observability-URL cache by caller token
LakshanSS c947160
fix: propagate the environment filter into breakdown table children
LakshanSS d8ad0c2
fix: round Deployment Frequency perDay to two decimal places
LakshanSS 11da8a3
Merge upstream/main into laki-dora-1
LakshanSS fee9ce7
feat: add Delivery Insights (DORA metrics) page to the sidebar
LakshanSS 7672c49
fix: address review feedback on Delivery Insights
LakshanSS e8bf913
fix: request breakdown metrics at the page's granularity
LakshanSS File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| --- | ||
| '@openchoreo/backstage-plugin-openchoreo-observability': minor | ||
| '@openchoreo/backstage-plugin-openchoreo-observability-backend': minor | ||
| '@openchoreo/openchoreo-client-node': minor | ||
| '@openchoreo/backstage-portal-app': minor | ||
| --- | ||
|
|
||
| Add a **Delivery Insights** sidebar page showing the four DORA metrics, scoped | ||
| by breadcrumb (Namespace → Project → Component). It sits alongside Cost | ||
| Insights in the sidebar rather than on entity pages, since the audience is | ||
| delivery leadership looking across an organisation rather than a developer | ||
| working on one component. | ||
|
|
||
| - **Metrics**: Deployment Frequency, Lead Time for Changes, Change Failure Rate | ||
| and MTTR as KPI tiles with DORA classification, delta vs the previous equal | ||
| window, and sparklines; a trend chart per metric at daily/weekly/monthly | ||
| granularity (lead time shows p50/p75/p95). | ||
| - **Drill-down**: a one-level-down breakdown table (namespace → projects, | ||
| project → components, component → environments) sorted by deployment | ||
| frequency, where each row carries its own metrics and an overall DORA rating | ||
| (the scope's weakest tier). Project/component rows narrow the page scope; | ||
| environment rows apply the environment filter. | ||
| - **Per-environment cards** for the current scope, plus an environment filter | ||
| and a "how these metrics are calculated" footnote. | ||
| - **Bookmarkable views**: scope, range, granularity and environment all live in | ||
| the URL, so a particular view can be shared or saved. | ||
| - **Data layer**: `ObservabilityClient` gains `getDoraMetrics` / | ||
| `getDoraDeployments` against the observer's | ||
| `POST /api/v1alpha1/insights/dora/query` and | ||
| `.../insights/dora/deployments/query`, called directly like the other | ||
| observability APIs. | ||
| - **URL resolution** gains namespace-level support: `/resolve-urls` now works | ||
| without an `environmentName` by resolving through the namespace's | ||
| environments (new `resolveForNamespace` in the client-node observability URL | ||
| resolver), which is what the org-wide scope needs. | ||
| - The namespace/project/component breadcrumb is now a shared `ScopeBreadcrumb` | ||
| component used by both Delivery Insights and Cost Insights. |
85 changes: 85 additions & 0 deletions
85
packages/openchoreo-client-node/src/observability-url-resolver.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { ObservabilityUrlResolver } from './observability-url-resolver'; | ||
| import { createOpenChoreoApiClient } from './factory'; | ||
|
|
||
| jest.mock('./factory', () => ({ | ||
| createOpenChoreoApiClient: jest.fn(), | ||
| })); | ||
|
|
||
| const mockedCreateClient = createOpenChoreoApiClient as jest.MockedFunction< | ||
| typeof createOpenChoreoApiClient | ||
| >; | ||
|
|
||
| function ok(data: unknown) { | ||
| return { data, error: undefined, response: { ok: true, status: 200 } }; | ||
| } | ||
|
|
||
| describe('ObservabilityUrlResolver.resolveForNamespace', () => { | ||
| beforeEach(() => { | ||
| mockedCreateClient.mockReset(); | ||
| }); | ||
|
|
||
| it('resolves through the namespace environments and caches the result', async () => { | ||
| const get = jest | ||
| .fn() | ||
| .mockResolvedValueOnce(ok({ items: [{ metadata: { name: 'dev' } }] })) | ||
| .mockResolvedValueOnce(ok({ spec: { dataPlaneRef: undefined } })) | ||
| .mockResolvedValueOnce(ok({ spec: { observabilityPlaneRef: undefined } })) | ||
| .mockResolvedValueOnce( | ||
| ok({ spec: { observerURL: 'https://observer.example.com' } }), | ||
| ); | ||
| mockedCreateClient.mockReturnValue({ GET: get } as any); | ||
|
|
||
| const resolver = new ObservabilityUrlResolver({ | ||
| baseUrl: 'https://api.example.com', | ||
| }); | ||
|
|
||
| const first = await resolver.resolveForNamespace('org-1', 'user-a-token'); | ||
| expect(first.observerUrl).toBe('https://observer.example.com'); | ||
| expect(get).toHaveBeenCalledTimes(4); | ||
|
|
||
| // Second call for the *same* token should hit the cache: no new HTTP calls. | ||
| const second = await resolver.resolveForNamespace('org-1', 'user-a-token'); | ||
| expect(second).toEqual(first); | ||
| expect(get).toHaveBeenCalledTimes(4); | ||
| }); | ||
|
|
||
| it('does not leak a cached result across callers with different tokens', async () => { | ||
| // resolveForNamespace creates its own client for listing environments, | ||
| // then resolveForEnvironment creates another one internally — route each | ||
| // by token rather than assuming a fixed call count/order. | ||
| const getA = jest | ||
| .fn() | ||
| .mockResolvedValueOnce(ok({ items: [{ metadata: { name: 'dev' } }] })) | ||
| .mockResolvedValueOnce(ok({ spec: { dataPlaneRef: undefined } })) | ||
| .mockResolvedValueOnce(ok({ spec: { observabilityPlaneRef: undefined } })) | ||
| .mockResolvedValueOnce( | ||
| ok({ spec: { observerURL: 'https://observer.example.com' } }), | ||
| ); | ||
|
|
||
| // User B has no visible environments in the same namespace (e.g. RBAC | ||
| // scopes them out) and must not receive user A's cached URL. | ||
| const getB = jest.fn().mockResolvedValue(ok({ items: [] })); | ||
|
|
||
| mockedCreateClient.mockImplementation( | ||
| config => ({ GET: config.token === 'user-a-token' ? getA : getB } as any), | ||
| ); | ||
|
|
||
| const resolver = new ObservabilityUrlResolver({ | ||
| baseUrl: 'https://api.example.com', | ||
| }); | ||
|
|
||
| const forUserA = await resolver.resolveForNamespace( | ||
| 'org-1', | ||
| 'user-a-token', | ||
| ); | ||
| expect(forUserA.observerUrl).toBe('https://observer.example.com'); | ||
|
|
||
| await expect( | ||
| resolver.resolveForNamespace('org-1', 'user-b-token'), | ||
| ).rejects.toThrow(/No environments found in namespace 'org-1'/); | ||
|
|
||
| // User B's request must have gone through its own client, not reused | ||
| // user A's cached result. | ||
| expect(getB).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: openchoreo/backstage-plugins
Length of output: 50385
🏁 Script executed:
Repository: openchoreo/backstage-plugins
Length of output: 7507
🏁 Script executed:
Repository: openchoreo/backstage-plugins
Length of output: 11199
Enforce one observer URL per namespace, or aggregate cross-environment calls.
resolveForNamespacereturns the first environment’s observer URL it can resolve. If environments in the same namespace can point to different observers, namespace-scoped requests may call an observer that does not have the requested environment’s metrics. Add invariant validation and a test for this case, or aggregate calls across all namespace observers.🤖 Prompt for AI Agents