Skip to content

Catalog: collect support diagnostics from admin Settings - #5168

Open
sir-sigurd wants to merge 4 commits into
masterfrom
support-diagnostics-ui
Open

Catalog: collect support diagnostics from admin Settings#5168
sir-sigurd wants to merge 4 commits into
masterfrom
support-diagnostics-ui

Conversation

@sir-sigurd

@sir-sigurd sir-sigurd commented Aug 4, 2026

Copy link
Copy Markdown
Member

Description

Adds a Support Diagnostics section to the admin Settings page. One button runs the stack's diagnostics collector and downloads the bundle it produces — search-cluster and infrastructure state for debugging a deployment — so an admin with no AWS access can attach diagnostics to a support request instead of working through a multi-step cloud procedure.

The button POSTs to an admin-only registry endpoint, which invokes the collector and streams the archive straight back; the catalog turns the response into a blob and saves it. Nothing is presigned, and the bundle goes to the admin's browser rather than anywhere else — the manifest.json inside it states what was collected, so the admin reviews before sending anything to support.

Three decisions worth surfacing:

  • The section renders unconditionally, because no capability probe exists. It reports whatever the endpoint says rather than hiding itself; the alternative is a second endpoint whose only job is to report whether the first one works.
  • Alert severity follows the status. A stack whose CloudFormation template predates the collector answers 503, and a collection already in flight answers 409. Neither is a malfunction, so neither gets the red alert that would send an admin off to file a bug.
  • The filename comes from Content-Disposition, which carries the collection's run id — what support asks for when several bundles are in flight. There is a generic fallback for any deployment whose registry does not expose that header to a cross-origin fetch.

The endpoint and the collector behind it ship in paired changes outside this repo. On a stack without them the section is inert and says so.

End-user documentation is deliberately not here: it publishes from master, so it lands with the release that ships the whole flow rather than describing a feature nobody has yet.

TODO

  • Unit tests
  • Security: Confirm that this change meets security best practices and does not violate the security model
  • Open: Confirm that this change doesn't break the Open variant
  • Documentation
    • JavaScript: basic explanation — the Description above; no screenshot, since the section is standard Material-UI built from the two Settings sections beside it, and the customer-facing screenshot (docs/imgs/admin-settings.png) is refreshed with the deferred markdown rather than twice
    • Markdown somewhere in docs/**/*.md that explains the feature to end users — deferred to the release that ships the flow, per above
  • Changelog entry

Greptile Summary

Adds an admin Settings control that requests a support-diagnostics archive from the registry and downloads it in the browser.

  • Adds the Support Diagnostics section to the Settings page.
  • Handles registry-provided filenames, fallback naming, and status-specific alerts.
  • Adds component tests for successful downloads and registry error responses.
  • Documents the feature in the catalog changelog.

Confidence Score: 4/5

The PR is not yet safe to merge because an in-flight diagnostics request can still trigger an archive download after the administrator leaves Settings.

The collection promise remains active after SupportDiagnostics unmounts, and its successful completion unconditionally invokes the browser download side effect.

Files Needing Attention: catalog/app/containers/Admin/Settings/SupportDiagnostics.tsx

Important Files Changed

Filename Overview
catalog/app/containers/Admin/Settings/SupportDiagnostics.tsx Implements diagnostics collection, archive download, filename extraction, and status-specific error presentation.
catalog/app/containers/Admin/Settings/SupportDiagnostics.spec.tsx Tests response filename handling, fallback naming, unavailable-stack messaging, and unexpected failures.
catalog/app/containers/Admin/Settings/Settings.tsx Adds the Support Diagnostics section to the existing admin Settings layout.
catalog/CHANGELOG.md Announces the new browser-based support diagnostics workflow.

Sequence Diagram

sequenceDiagram
  actor Admin
  participant Settings as Admin Settings
  participant Registry as Registry API
  participant Browser
  Admin->>Settings: Click Collect diagnostics
  Settings->>Registry: POST /admin/support-diagnostics
  Registry-->>Settings: ZIP response
  Settings->>Browser: Create blob URL and download archive
Loading

Reviews (2): Last reviewed commit: "Say what navigating away actually does" | Re-trigger Greptile

Context used:

sir-sigurd and others added 2 commits August 4, 2026 21:41
Adds a Support Diagnostics section to the admin Settings page: one button
that runs the stack's diagnostics collector through the registry and
downloads the bundle it returns, so an admin with no AWS access can attach
diagnostics to a support request.

No capability probe exists, so the section renders unconditionally and
reports whatever the endpoint says. Severity follows the status rather
than always being red: a stack whose template predates the collector (503)
and a collection already in flight (409) are not malfunctions.

The filename comes from Content-Disposition, which carries the collection's
run id, with a fallback for deployments whose registry does not expose that
header to a cross-origin fetch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sir-sigurd
sir-sigurd marked this pull request as ready for review August 4, 2026 16:44
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.28%. Comparing base (c2596cb) to head (43e2811).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...p/containers/Admin/Settings/SupportDiagnostics.tsx 93.33% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5168      +/-   ##
==========================================
+ Coverage   48.22%   48.28%   +0.05%     
==========================================
  Files         827      828       +1     
  Lines       34120    34165      +45     
  Branches     5837     5848      +11     
==========================================
+ Hits        16455    16497      +42     
- Misses      15781    15784       +3     
  Partials     1884     1884              
Flag Coverage Δ
api-python 93.25% <ø> (ø)
catalog 25.32% <93.33%> (+0.13%) ⬆️
lambda 85.38% <ø> (ø)
py-shared 98.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +74 to +78
endpoint: '/admin/support-diagnostics',
method: 'POST',
// The response is an archive, and there is no request body to encode.
json: false,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Navigation does not cancel collection

When an administrator leaves Settings before the request completes, the application-level API saga continues the POST and the success path still calls saveAs, causing the archive to download on another page despite the UI stating that navigation cancels collection.

Knowledge Base Used: Catalog Frontend (catalog/app)

The API request runs in an app-root redux saga with no AbortController, so
leaving Settings does not cancel it -- the archive still downloads. Suppressing
that would discard a finished bundle with no way to fetch it again, so the copy
changes instead: closing the tab is what loses the download.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sir-sigurd

Copy link
Copy Markdown
Member Author

Confirmed and fixed in d234547 — thanks, this was a real defect.

Checked the mechanism rather than the symptom: APIConnector.Provider is composed at app.tsx:105, so apiSaga runs for the app's lifetime rather than the route's, and there is no AbortController anywhere in utils/APIConnector.jsx. Nothing cancels an in-flight request, so the archive does download on whatever page the admin has navigated to.

I fixed the copy rather than the behaviour. Making navigation actually cancel would mean discarding a bundle the collection already produced, and there is deliberately no fetch-by-run endpoint to get it back — a lost download costs a fresh collection against the cluster being diagnosed, which is the thing worth avoiding. So navigating away is fine and the download still arrives; the message now says closing the tab is what loses it.

@sir-sigurd

Copy link
Copy Markdown
Member Author

@greptileai please re-review — the copy fix in d234547 landed after your pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an admin-facing Support Diagnostics section to Catalog’s Admin Settings, enabling admins to trigger a registry diagnostics collection and download the resulting archive directly in the browser for support/debug workflows.

Changes:

  • Adds a new Support Diagnostics UI on the Admin Settings page that POSTs to /admin/support-diagnostics and downloads the returned ZIP.
  • Extracts the download filename from Content-Disposition with a fallback name, and surfaces status-specific alert severities.
  • Adds unit tests for download + error cases and documents the feature in the catalog changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
catalog/app/containers/Admin/Settings/SupportDiagnostics.tsx New UI component that triggers diagnostics collection and downloads the archive.
catalog/app/containers/Admin/Settings/SupportDiagnostics.spec.tsx Unit tests for successful download, fallback naming, and error severity handling.
catalog/app/containers/Admin/Settings/Settings.tsx Wires the new Support Diagnostics section into the Admin Settings page layout.
catalog/CHANGELOG.md Changelog entry announcing the new Support Diagnostics workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +110 to +121
it('surfaces an unexpected failure as an error', async () => {
mocks.req.mockRejectedValue(
new mocks.HTTPError(502, 'The diagnostics collector failed.'),
)
const { container, getByText } = renderComponent()

fireEvent.click(getByText('Collect diagnostics'))

await waitFor(() => expect(getByText(/collector failed/)).toBeTruthy())
expect(container.querySelector('.MuiAlert-standardError')).not.toBeNull()
})
})
Comment on lines +69 to +93
const collect = React.useCallback(async () => {
setCollecting(true)
setFailure(null)
try {
const response: Response = await req({
endpoint: '/admin/support-diagnostics',
method: 'POST',
// The response is an archive, and there is no request body to encode.
json: false,
})
saveAs(await response.blob(), getFilename(response))
} catch (e) {
if (e instanceof APIConnector.HTTPError) {
setFailure({
severity: SEVERITIES[e.status] || 'error',
message: e.json?.message || e.message,
})
} else {
Sentry.captureException(e)
setFailure({ severity: 'error', message: `Could not collect diagnostics: ${e}` })
}
} finally {
setCollecting(false)
}
}, [req])
The registry answers 503 when a stack has no collector. An ALB, or the
nginx sidecar, answers 503 when the registry is merely cycling -- with an
HTML body. Keying severity on the status alone reported the second as
calmly as the first, which is the reading that sends nobody to look; and
the body was rendered verbatim, so an error page landed in the alert box.

Severity now comes from the response's error_code. Anything without one --
every error raised before the endpoint is reached, including the 404 from
a registry that predates it -- gets the red alert and a generic message,
and the raw body never reaches the DOM.

Also stop revoking the blob URL on the line after click(): clicking only
queues the download, and revoking before the browser has read the blob
cancels it in some of them, which the UI would have reported as success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants