-
Notifications
You must be signed in to change notification settings - Fork 90
Catalog: collect support diagnostics from admin Settings #5168
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
Open
sir-sigurd
wants to merge
4
commits into
master
Choose a base branch
from
support-diagnostics-ui
base: master
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.
+316
−0
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
bbc438f
Catalog: collect support diagnostics from admin Settings
sir-sigurd de774e5
Changelog entry for the support diagnostics section
sir-sigurd d234547
Say what navigating away actually does
sir-sigurd 43e2811
Catalog: key diagnostics failures on the error code, not the status
sir-sigurd 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
Some comments aren't visible on the classic Files Changed page.
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
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
161 changes: 161 additions & 0 deletions
161
catalog/app/containers/Admin/Settings/SupportDiagnostics.spec.tsx
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,161 @@ | ||
| import * as React from 'react' | ||
| import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest' | ||
| import { ThemeProvider, createMuiTheme } from '@material-ui/core/styles' | ||
| import { cleanup, fireEvent, render, waitFor } from '@testing-library/react' | ||
|
|
||
| const mocks = vi.hoisted(() => { | ||
| class HTTPError extends Error { | ||
| status: number | ||
|
|
||
| json: { message: string; error_code?: string } | ||
|
|
||
| // `json` is whatever the body parsed to; APIConnector falls back to | ||
| // `{ message: <the raw text> }` for a body that is not JSON at all, which is | ||
| // what an ALB or nginx error page arrives as. | ||
| constructor(status: number, message: string, errorCode?: string) { | ||
| super(message) | ||
| this.status = status | ||
| this.json = errorCode ? { message, error_code: errorCode } : { message } | ||
| } | ||
| } | ||
| return { HTTPError, req: vi.fn() } | ||
| }) | ||
|
|
||
| vi.mock('utils/APIConnector', () => ({ | ||
| use: () => mocks.req, | ||
| HTTPError: mocks.HTTPError, | ||
| })) | ||
|
|
||
| vi.mock('@sentry/react', () => ({ captureException: vi.fn() })) | ||
|
|
||
| import SupportDiagnostics from './SupportDiagnostics' | ||
|
|
||
| const theme = createMuiTheme() | ||
|
|
||
| function renderComponent() { | ||
| return render( | ||
| <ThemeProvider theme={theme}> | ||
| <SupportDiagnostics /> | ||
| </ThemeProvider>, | ||
| ) | ||
| } | ||
|
|
||
| function archive(headers: Record<string, string>) { | ||
| return new Response(new Blob(['PK\x03\x04'], { type: 'application/zip' }), { headers }) | ||
| } | ||
|
|
||
| describe('containers/Admin/Settings/SupportDiagnostics', () => { | ||
| let downloaded: { name: string } | null = null | ||
|
|
||
| beforeEach(() => { | ||
| downloaded = null | ||
| vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:bundle') | ||
| vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}) | ||
| vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation( | ||
| function click(this: HTMLAnchorElement) { | ||
| downloaded = { name: this.download } | ||
| }, | ||
| ) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| cleanup() | ||
| vi.restoreAllMocks() | ||
| mocks.req.mockReset() | ||
| }) | ||
|
|
||
| it('downloads the bundle under the name the registry gave it', async () => { | ||
| mocks.req.mockResolvedValue( | ||
| archive({ | ||
| 'Content-Disposition': | ||
| 'attachment; filename="quilt-support-diagnostics-run-1.zip"', | ||
| }), | ||
| ) | ||
| const { getByText } = renderComponent() | ||
|
|
||
| fireEvent.click(getByText('Collect diagnostics')) | ||
|
|
||
| await waitFor(() => expect(downloaded).not.toBeNull()) | ||
| expect(downloaded!.name).toBe('quilt-support-diagnostics-run-1.zip') | ||
| expect(mocks.req).toHaveBeenCalledWith({ | ||
| endpoint: '/admin/support-diagnostics', | ||
| method: 'POST', | ||
| json: false, | ||
| }) | ||
| }) | ||
|
|
||
| it('falls back to a generic name when the filename header is not exposed', async () => { | ||
| mocks.req.mockResolvedValue(archive({})) | ||
| const { getByText } = renderComponent() | ||
|
|
||
| fireEvent.click(getByText('Collect diagnostics')) | ||
|
|
||
| await waitFor(() => expect(downloaded).not.toBeNull()) | ||
| expect(downloaded!.name).toBe('quilt-support-diagnostics.zip') | ||
| }) | ||
|
|
||
| it('reports a stack without the collector as information, not as a failure', async () => { | ||
| mocks.req.mockRejectedValue( | ||
| new mocks.HTTPError( | ||
| 503, | ||
| 'Support diagnostics collection is not available on this stack.', | ||
| 'NotAvailable', | ||
| ), | ||
| ) | ||
| const { container, getByText } = renderComponent() | ||
|
|
||
| fireEvent.click(getByText('Collect diagnostics')) | ||
|
|
||
| await waitFor(() => expect(getByText(/not available on this stack/)).toBeTruthy()) | ||
| expect(container.querySelector('.MuiAlert-standardInfo')).not.toBeNull() | ||
| expect(container.querySelector('.MuiAlert-standardError')).toBeNull() | ||
| }) | ||
|
|
||
| it('surfaces an unexpected failure as an error', async () => { | ||
| mocks.req.mockRejectedValue( | ||
| new mocks.HTTPError(502, 'The diagnostics collector failed.', 'CollectorFailed'), | ||
| ) | ||
| const { container, getByText } = renderComponent() | ||
|
|
||
| fireEvent.click(getByText('Collect diagnostics')) | ||
|
|
||
| await waitFor(() => expect(getByText(/collector failed/)).toBeTruthy()) | ||
| expect(container.querySelector('.MuiAlert-standardError')).not.toBeNull() | ||
| }) | ||
|
|
||
| it('does not dress an infrastructure failure up as an expected one', async () => { | ||
| // The registry cycling, or an ALB with no healthy target, answers 503 too -- | ||
| // with an HTML body and no error_code. Keying severity on the status alone | ||
| // would paint that the same calm blue as "this stack has no collector". | ||
| mocks.req.mockRejectedValue( | ||
| new mocks.HTTPError( | ||
| 503, | ||
| '<html><body><h1>503 Service Unavailable</h1></body></html>', | ||
| ), | ||
| ) | ||
| const { container, getByText } = renderComponent() | ||
|
|
||
| fireEvent.click(getByText('Collect diagnostics')) | ||
|
|
||
| await waitFor(() => | ||
| expect(container.querySelector('.MuiAlert-standardError')).not.toBeNull(), | ||
| ) | ||
| expect(container.querySelector('.MuiAlert-standardInfo')).toBeNull() | ||
| // And the page itself never reaches the admin. | ||
| expect(getByText(/registry may be restarting/)).toBeTruthy() | ||
| expect(container.textContent).not.toContain('<html>') | ||
| }) | ||
|
|
||
| it('reports a registry that predates the endpoint as information', async () => { | ||
| // The catalog and the registry are separate containers in one template, so a | ||
| // stack mid-update can serve this button from a registry with no such route. | ||
| // Flask answers 404 with an HTML body, which is the same shape as above. | ||
| mocks.req.mockRejectedValue(new mocks.HTTPError(404, '<html>404 Not Found</html>')) | ||
| const { container, getByText } = renderComponent() | ||
|
|
||
| fireEvent.click(getByText('Collect diagnostics')) | ||
|
|
||
| await waitFor(() => expect(getByText(/Could not collect diagnostics/)).toBeTruthy()) | ||
| expect(container.textContent).not.toContain('<html>') | ||
| }) | ||
| }) |
146 changes: 146 additions & 0 deletions
146
catalog/app/containers/Admin/Settings/SupportDiagnostics.tsx
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,146 @@ | ||
| import * as React from 'react' | ||
| import * as M from '@material-ui/core' | ||
| import * as Lab from '@material-ui/lab' | ||
| import * as Sentry from '@sentry/react' | ||
|
|
||
| import * as APIConnector from 'utils/APIConnector' | ||
|
|
||
| // The registry names the archive after the collection run, and support asks for | ||
| // that name when several bundles are in flight. Readable cross-origin only | ||
| // because the endpoint's CORS config exposes Content-Disposition. | ||
| const FILENAME_RE = /filename="([^"]+)"/ | ||
|
|
||
| const FALLBACK_FILENAME = 'quilt-support-diagnostics.zip' | ||
|
|
||
| function getFilename(response: Response): string { | ||
| const match = FILENAME_RE.exec(response.headers.get('Content-Disposition') || '') | ||
| return match?.[1] || FALLBACK_FILENAME | ||
| } | ||
|
|
||
| function saveAs(blob: Blob, filename: string) { | ||
| const url = URL.createObjectURL(blob) | ||
| const link = document.createElement('a') | ||
| link.href = url | ||
| link.download = filename | ||
| document.body.appendChild(link) | ||
| link.click() | ||
| document.body.removeChild(link) | ||
| // Not synchronously: clicking only queues the download, and revoking the URL | ||
| // before the browser reads the blob cancels it in some of them. | ||
| setTimeout(() => URL.revokeObjectURL(url), 60000) | ||
| } | ||
|
|
||
| interface Failure { | ||
| severity: 'error' | 'warning' | 'info' | ||
| message: string | ||
| } | ||
|
|
||
| // Keyed on the registry's error_code, not on the HTTP status: this stack having | ||
| // no collector answers 503 and so does a registry that is merely cycling, and | ||
| // only the code tells them apart. Anything unrecognised -- including every error | ||
| // raised before the endpoint is reached, by the ALB or the nginx sidecar -- keeps | ||
| // the red alert, which is the right reading for an infrastructure failure. | ||
| const SEVERITIES: Record<string, Failure['severity']> = { | ||
| NotAvailable: 'info', | ||
| AlreadyRunning: 'warning', | ||
| } | ||
|
|
||
| // An error from anything other than the endpoint carries whatever body that thing | ||
| // serves, usually an HTML page. APIConnector puts the raw text in `message` when | ||
| // it will not parse as JSON, so showing it would paste a document into the alert. | ||
| const GENERIC_FAILURE = | ||
| 'Could not collect diagnostics. The registry may be restarting or unreachable; try again in a few minutes.' | ||
|
|
||
| function describe(e: APIConnector.HTTPError): Failure { | ||
| const code = e.json?.error_code | ||
| if (!code) return { severity: 'error', message: GENERIC_FAILURE } | ||
| return { | ||
| severity: SEVERITIES[code] || 'error', | ||
| message: e.json?.message || GENERIC_FAILURE, | ||
| } | ||
| } | ||
|
|
||
| const useStyles = M.makeStyles((t) => ({ | ||
| actions: { | ||
| alignItems: 'center', | ||
| display: 'flex', | ||
| marginTop: t.spacing(2), | ||
| }, | ||
| progress: { | ||
| marginLeft: t.spacing(2), | ||
| }, | ||
| progressText: { | ||
| color: t.palette.text.secondary, | ||
| marginLeft: t.spacing(1), | ||
| }, | ||
| failure: { | ||
| marginTop: t.spacing(2), | ||
| }, | ||
| })) | ||
|
|
||
| export default function SupportDiagnostics() { | ||
| const classes = useStyles() | ||
| const req = APIConnector.use() | ||
|
|
||
| const [collecting, setCollecting] = React.useState(false) | ||
| const [failure, setFailure] = React.useState<Failure | null>(null) | ||
|
|
||
| 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(describe(e)) | ||
| } else { | ||
| Sentry.captureException(e) | ||
| setFailure({ severity: 'error', message: GENERIC_FAILURE }) | ||
| } | ||
| } finally { | ||
| setCollecting(false) | ||
| } | ||
| }, [req]) | ||
|
Comment on lines
+88
to
+109
|
||
|
|
||
| return ( | ||
| <> | ||
| <M.Typography variant="body2"> | ||
| Collect a diagnostics bundle describing this stack's search cluster and | ||
| infrastructure, for Quilt support to debug against. Nothing is sent anywhere: the | ||
| bundle downloads to your computer, and the <code>manifest.json</code> inside it | ||
| lists exactly what was collected, so you can review it before attaching it to a | ||
| support request. | ||
| </M.Typography> | ||
| <div className={classes.actions}> | ||
| <M.Button | ||
| variant="contained" | ||
| color="primary" | ||
| onClick={collect} | ||
| disabled={collecting} | ||
| > | ||
| Collect diagnostics | ||
| </M.Button> | ||
| {collecting && ( | ||
| <> | ||
| <M.CircularProgress size={20} className={classes.progress} /> | ||
| <M.Typography variant="body2" className={classes.progressText}> | ||
| Collecting… A large or unhealthy cluster takes longer; closing this | ||
| tab loses the download. | ||
| </M.Typography> | ||
| </> | ||
| )} | ||
| </div> | ||
| {failure && ( | ||
| <Lab.Alert severity={failure.severity} className={classes.failure}> | ||
| {failure.message} | ||
| </Lab.Alert> | ||
| )} | ||
| </> | ||
| ) | ||
| } | ||
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.
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)