diff --git a/.changeset/khaki-camels-rush.md b/.changeset/khaki-camels-rush.md new file mode 100644 index 0000000000..a38600abf3 --- /dev/null +++ b/.changeset/khaki-camels-rush.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-newrelic': patch +--- + +The newrelic plugin now supports pagination when retrieving results from newrelic. It will no longer truncate results. To see all applications, the link header will need to be allowed through the proxy (see the newrelic plugin readme). diff --git a/app-config.yaml b/app-config.yaml index 999bb38005..37cb61f26e 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -73,6 +73,8 @@ proxy: target: https://api.newrelic.com/v2 headers: X-Api-Key: ${NEW_RELIC_REST_API_KEY} + allowedHeaders: + - link '/newrelic/api': target: https://api.newrelic.com diff --git a/plugins/newrelic/README.md b/plugins/newrelic/README.md index 8ac7ed2c72..14049ba39c 100644 --- a/plugins/newrelic/README.md +++ b/plugins/newrelic/README.md @@ -18,6 +18,8 @@ APIs. target: https://api.newrelic.com/v2 headers: X-Api-Key: ${NEW_RELIC_REST_API_KEY} + allowedHeaders: + - link ``` There is some types of api key on new relic, to this use must be `User` type of key, In your production deployment of Backstage, you would also need to ensure that @@ -33,6 +35,8 @@ APIs. '/newrelic/apm/api': headers: X-Api-Key: NRRA-YourActualApiKey + allowedHeaders: + - link ``` Read more about how to find or generate this key in diff --git a/plugins/newrelic/package.json b/plugins/newrelic/package.json index b09050c599..c70a06dee8 100644 --- a/plugins/newrelic/package.json +++ b/plugins/newrelic/package.json @@ -39,6 +39,7 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.61", + "parse-link-header": "^2.0.0", "react-use": "^17.2.4" }, "peerDependencies": { @@ -47,6 +48,7 @@ "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { + "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@backstage/core-app-api": "workspace:^", "@backstage/dev-utils": "workspace:^", @@ -56,9 +58,10 @@ "@testing-library/react": "^12.1.3", "@testing-library/user-event": "^14.0.0", "@types/node": "^16.11.26", + "@types/parse-link-header": "^2.0.1", "@types/react": "^16.13.1 || ^17.0.0", "cross-fetch": "^3.1.5", - "msw": "^1.0.0" + "msw": "^1.2.3" }, "files": [ "dist" diff --git a/plugins/newrelic/src/api/index.test.ts b/plugins/newrelic/src/api/index.test.ts new file mode 100644 index 0000000000..d4ed4a1691 --- /dev/null +++ b/plugins/newrelic/src/api/index.test.ts @@ -0,0 +1,384 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NewRelicApplication, NewRelicClient } from '.'; +import { DiscoveryApi } from '@backstage/core-plugin-api'; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; +import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; + +const mockedDiscoveryApi: DiscoveryApi = { + getBaseUrl: async () => 'https://test.test', +}; + +beforeEach(() => { + jest.resetAllMocks(); +}); + +describe('NewRelicClient', () => { + const server = setupServer(); + setupRequestMockHandlers(server); + + beforeEach(() => { + server.resetHandlers(); + }); + + test.each([ + ['https://test.test/BASEPATH/apm/api/applications.json', '/BASEPATH'], + ['https://test.test/BASEPATH2/apm/api/applications.json', '/BASEPATH2'], + ['https://test.testBASEPATH3/apm/api/applications.json', 'BASEPATH3'], + ['https://test.test/newrelic/apm/api/applications.json', undefined], + ])( + 'It correctly forms the request url (%p) when proxyPathBase is %p', + async (expectedUrl, basePathOverride) => { + server.use( + rest.get(expectedUrl, (_, res, ctx) => + res(ctx.status(200), ctx.json({ applications: [] })), + ), + ); + + const mockedFetchApi = new MockFetchApi(); + const fetchSpy = jest.spyOn(mockedFetchApi, 'fetch'); + + const client = new NewRelicClient({ + discoveryApi: mockedDiscoveryApi, + fetchApi: mockedFetchApi, + proxyPathBase: basePathOverride, + }); + await client.getApplications(); + + expect(fetchSpy).toHaveBeenCalledWith(expectedUrl); + }, + ); + + it('Correctly reads all pages of results and returns the expected results', async () => { + const mockedApplicationOne: NewRelicApplication = { + id: 1, + application_summary: { + apdex_score: 0, + error_rate: 100, + host_count: 500, + instance_count: 5000, + response_time: 20, + throughput: 500000, + }, + name: 'Testing Application #1', + language: 'en-us', + health_status: 'Failing', + reporting: true, + settings: { + app_apdex_threshold: 0, + end_user_apdex_threshold: 0, + enable_real_user_monitoring: true, + use_server_side_config: true, + }, + }; + + const mockedApplicationTwo: NewRelicApplication = { + id: 2, + application_summary: { + apdex_score: -900, + error_rate: 0, + host_count: 0, + instance_count: 0, + response_time: 0, + throughput: 0, + }, + name: 'Testing Application #2', + language: 'en-us', + health_status: 'Working', + reporting: true, + settings: { + app_apdex_threshold: 0, + end_user_apdex_threshold: 0, + enable_real_user_monitoring: true, + use_server_side_config: true, + }, + }; + + const mockedApplicationThree: NewRelicApplication = { + id: 3, + application_summary: { + apdex_score: -900, + error_rate: 0, + host_count: 0, + instance_count: 0, + response_time: 0, + throughput: 0, + }, + name: 'Testing Application #3', + language: 'en-us', + health_status: 'Waiting', + reporting: false, + settings: { + app_apdex_threshold: 1000, + end_user_apdex_threshold: 500, + enable_real_user_monitoring: false, + use_server_side_config: false, + }, + }; + + const queryToRequestData = new Map< + string | null, + { link?: string | string[]; apps: NewRelicApplication[] } + >([ + [ + null, + { + link: [ + '; rel="next"', + '; rel="bad"', + ], + apps: [mockedApplicationOne], + }, + ], + [ + '2', + { + link: '; rel="next",', + apps: [], + }, + ], + [ + '3', + { + apps: [mockedApplicationTwo, mockedApplicationThree], + }, + ], + ]); + + const mockedFetchApi = new MockFetchApi(); + const fetchSpy = jest.spyOn(mockedFetchApi, 'fetch'); + + server.use( + rest.get( + 'https://test.test/newrelic/apm/api/applications.json', + (req, res, ctx) => { + const nextPageNumber = req.url.searchParams.get('page'); + const requestData = queryToRequestData.get(nextPageNumber) ?? { + apps: [], + }; + + const { link, apps: applications } = requestData; + const statusTransform = ctx.status(200); + const responseBody = ctx.json({ applications }); + + if (!!link) { + return res(statusTransform, ctx.set({ link }), responseBody); + } + + return res(statusTransform, responseBody); + }, + ), + ); + + const client = new NewRelicClient({ + discoveryApi: mockedDiscoveryApi, + fetchApi: mockedFetchApi, + }); + + const actual = await client.getApplications(); + const expected = { + applications: [ + mockedApplicationOne, + mockedApplicationTwo, + mockedApplicationThree, + ], + }; + + expect(fetchSpy).toHaveBeenCalledTimes(3); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://test.test/newrelic/apm/api/applications.json', + ); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://test.test/newrelic/apm/api/applications.json?page=2', + ); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://test.test/newrelic/apm/api/applications.json?page=3', + ); + expect(actual).toStrictEqual(expected); + }); + + test.each([['Link'], ['LINK'], ['lINK']])( + 'It does not attempt pagination when the link header name is invalid (%p)', + async linkHeaderName => { + const mockedFetchApi = new MockFetchApi(); + const fetchSpy = jest.spyOn(mockedFetchApi, 'fetch'); + + server.use( + rest.get( + 'https://test.test/newrelic/apm/api/applications.json', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set( + linkHeaderName, + '; rel="next"', + ), + ctx.json({ applications: [] }), + ), + ), + rest.get('https://test.test/badroute', () => { + throw new Error( + 'NewRelicClient attempted to paginate when it should not have', + ); + }), + ); + + const client = new NewRelicClient({ + discoveryApi: mockedDiscoveryApi, + fetchApi: mockedFetchApi, + }); + await client.getApplications(); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://test.test/newrelic/apm/api/applications.json', + ); + }, + ); + + test.each([ + [''], + ['<> rel=""'], + ['<>; rel=""'], + ['; rel=""'], + ['<>; rel:"value"'], + ['; rel: "next"'], + ['ABCDE'], + ['; rel="next",'], + ])( + 'It does not attempt pagination when the link header value is invalid (%p)', + async linkHeaderValue => { + const mockedFetchApi = new MockFetchApi(); + const fetchSpy = jest.spyOn(mockedFetchApi, 'fetch'); + + server.use( + rest.get( + 'https://test.test/newrelic/apm/api/applications.json', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('link', linkHeaderValue), + ctx.json({ applications: [] }), + ), + ), + rest.get('https://test.test/badroute', () => { + throw new Error( + 'NewRelicClient attempted to paginate when it should not have', + ); + }), + ); + + const client = new NewRelicClient({ + discoveryApi: mockedDiscoveryApi, + fetchApi: mockedFetchApi, + }); + await client.getApplications(); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://test.test/newrelic/apm/api/applications.json', + ); + }, + ); + + test.each([ + ['Error communicating with New Relic: Not Found', 404, JSON.stringify({})], + [ + 'Error communicating with New Relic: ERROR TITLE', + 404, + JSON.stringify({ + error: { + title: 'ERROR TITLE', + }, + }), + ], + [ + 'Error communicating with New Relic: Internal Server Error', + 500, + JSON.stringify(undefined), + ], + [ + 'Error communicating with New Relic: Internal Server Error', + 500, + JSON.stringify(null), + ], + [ + 'Error communicating with New Relic: Internal Server Error', + 500, + ' { + server.use( + rest.get( + 'https://test.test/newrelic/apm/api/applications.json', + (_, res, ctx) => res(ctx.status(statusCode), ctx.body(body)), + ), + ); + + const client = new NewRelicClient({ + discoveryApi: mockedDiscoveryApi, + fetchApi: new MockFetchApi(), + }); + + await expect(client.getApplications()).rejects.toThrow( + expectedErrorMessage, + ); + }, + ); + + it('Throws an error when the body is invalid json but the status code is 200', async () => { + server.use( + rest.get( + 'https://test.test/newrelic/apm/api/applications.json', + (_, res, ctx) => res(ctx.status(200), ctx.body(' { + const getBaseUrlSpy = jest.spyOn(mockedDiscoveryApi, 'getBaseUrl'); + + server.use( + rest.get( + 'https://test.test/newrelic/apm/api/applications.json', + (_, res, ctx) => res(ctx.status(200), ctx.json({ applications: [] })), + ), + ); + + const client = new NewRelicClient({ + discoveryApi: mockedDiscoveryApi, + fetchApi: new MockFetchApi(), + }); + + await client.getApplications(); + await client.getApplications(); + await client.getApplications(); + await client.getApplications(); + + expect(getBaseUrlSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/plugins/newrelic/src/api/index.ts b/plugins/newrelic/src/api/index.ts index 2734dd2b4f..b37575b163 100644 --- a/plugins/newrelic/src/api/index.ts +++ b/plugins/newrelic/src/api/index.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -import { createApiRef, DiscoveryApi } from '@backstage/core-plugin-api'; +import { + createApiRef, + DiscoveryApi, + FetchApi, +} from '@backstage/core-plugin-api'; + +import parseLinkHeader from 'parse-link-header'; export type NewRelicApplication = { id: number; @@ -61,6 +67,7 @@ const DEFAULT_PROXY_PATH_BASE = '/newrelic'; type Options = { discoveryApi: DiscoveryApi; + fetchApi: FetchApi; /** * Path to use for requests via the proxy, defaults to /newrelic */ @@ -71,39 +78,72 @@ export interface NewRelicApi { getApplications(): Promise; } +interface NewRelicPageReadResult { + nextPageUrl: string | undefined; + applicationsFromReadPage: NewRelicApplication[]; +} + export class NewRelicClient implements NewRelicApi { private readonly discoveryApi: DiscoveryApi; + private readonly fetchApi: FetchApi; private readonly proxyPathBase: string; + private baseUrl: string; constructor(options: Options) { this.discoveryApi = options.discoveryApi; + this.fetchApi = options.fetchApi; this.proxyPathBase = options.proxyPathBase ?? DEFAULT_PROXY_PATH_BASE; + this.baseUrl = ''; } async getApplications(): Promise { - const url = await this.getApiUrl('apm', 'applications.json'); - const response = await fetch(url); - let responseJson; - - try { - responseJson = await response.json(); - } catch (e) { - responseJson = { applications: [] }; + if (!this.baseUrl) { + const proxyUrl = await this.discoveryApi.getBaseUrl('proxy'); + this.baseUrl = `${proxyUrl}${this.proxyPathBase}/apm/api/applications.json`; } - if (response.status !== 200) { + const applications: NewRelicApplication[] = []; + let targetUrl = this.baseUrl; + + do { + const { nextPageUrl, applicationsFromReadPage } = + await this.fetchNewRelic(targetUrl); + + targetUrl = nextPageUrl ?? ''; + applications.push(...applicationsFromReadPage); + } while (!!targetUrl); + + return { applications }; + } + + private async fetchNewRelic( + targetUrl: string, + ): Promise { + const response = await this.fetchApi.fetch(targetUrl); + + if (!response.ok) { + let specificErrorTitle = undefined; + try { + specificErrorTitle = (await response.json())?.error?.title; + } catch (e) { + /* empty */ + } + throw new Error( `Error communicating with New Relic: ${ - responseJson?.error?.title || response.statusText + specificErrorTitle || response.statusText }`, ); } - return responseJson; - } + const readResponse = (await response.json()) as NewRelicApplications; + const linkHeader = response.headers.get('link'); + const parseResult = parseLinkHeader(linkHeader); + const nextPageNumber = parseResult?.next?.page; - private async getApiUrl(product: string, path: string) { - const proxyUrl = await this.discoveryApi.getBaseUrl('proxy'); - return `${proxyUrl}${this.proxyPathBase}/${product}/api/${path}`; + return { + nextPageUrl: nextPageNumber && `${this.baseUrl}?page=${nextPageNumber}`, + applicationsFromReadPage: readResponse.applications, + }; } } diff --git a/plugins/newrelic/src/plugin.ts b/plugins/newrelic/src/plugin.ts index 0387c50aa9..da7cae9b93 100644 --- a/plugins/newrelic/src/plugin.ts +++ b/plugins/newrelic/src/plugin.ts @@ -20,6 +20,7 @@ import { createPlugin, createRouteRef, discoveryApiRef, + fetchApiRef, createRoutableExtension, } from '@backstage/core-plugin-api'; @@ -33,8 +34,12 @@ export const newRelicPlugin = createPlugin({ apis: [ createApiFactory({ api: newRelicApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => new NewRelicClient({ discoveryApi }), + deps: { + discoveryApi: discoveryApiRef, + fetchApi: fetchApiRef, + }, + factory: ({ discoveryApi, fetchApi }) => + new NewRelicClient({ discoveryApi, fetchApi }), }), ], routes: { diff --git a/yarn.lock b/yarn.lock index 2c7e8d6012..6a0c6279fc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7530,6 +7530,7 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-newrelic@workspace:plugins/newrelic" dependencies: + "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/core-app-api": "workspace:^" "@backstage/core-components": "workspace:^" @@ -7545,9 +7546,11 @@ __metadata: "@testing-library/react": ^12.1.3 "@testing-library/user-event": ^14.0.0 "@types/node": ^16.11.26 + "@types/parse-link-header": ^2.0.1 "@types/react": ^16.13.1 || ^17.0.0 cross-fetch: ^3.1.5 - msw: ^1.0.0 + msw: ^1.2.3 + parse-link-header: ^2.0.0 react-use: ^17.2.4 peerDependencies: react: ^16.13.1 || ^17.0.0 @@ -17602,6 +17605,13 @@ __metadata: languageName: node linkType: hard +"@types/parse-link-header@npm:^2.0.1": + version: 2.0.1 + resolution: "@types/parse-link-header@npm:2.0.1" + checksum: f76678612511365aefc23704f00f3262fcb1d6bd9c4dd83f783a38e50e3ccf26615f9a62c757952cab5af6f2bd8b3b1763ce391376458afce5fe47c6fe2b80e1 + languageName: node + linkType: hard + "@types/passport-auth0@npm:^1.0.5": version: 1.0.5 resolution: "@types/passport-auth0@npm:1.0.5" @@ -32871,7 +32881,7 @@ __metadata: languageName: node linkType: hard -"msw@npm:^1.0.0, msw@npm:^1.0.1, msw@npm:^1.2.1": +"msw@npm:^1.0.0, msw@npm:^1.0.1, msw@npm:^1.2.1, msw@npm:^1.2.3": version: 1.2.3 resolution: "msw@npm:1.2.3" dependencies: @@ -34372,6 +34382,15 @@ __metadata: languageName: node linkType: hard +"parse-link-header@npm:^2.0.0": + version: 2.0.0 + resolution: "parse-link-header@npm:2.0.0" + dependencies: + xtend: ~4.0.1 + checksum: 0e96c6af9910e8f92084b49b8dc6a10dd58db470847d1499f562576180c1ac5e49d18007697f0d538e5f3efdc8ce1d8777641f3ae225302b74af0dd0578b628e + languageName: node + linkType: hard + "parse-path@npm:^7.0.0": version: 7.0.0 resolution: "parse-path@npm:7.0.0" @@ -42758,7 +42777,7 @@ __metadata: languageName: node linkType: hard -"xtend@npm:^4.0.0": +"xtend@npm:^4.0.0, xtend@npm:~4.0.1": version: 4.0.2 resolution: "xtend@npm:4.0.2" checksum: ac5dfa738b21f6e7f0dd6e65e1b3155036d68104e67e5d5d1bde74892e327d7e5636a076f625599dc394330a731861e87343ff184b0047fef1360a7ec0a5a36a