diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f5b1ccff61..3440a9be9f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ The version headers in this history reflect the versions of Apollo Server itself ## v2.15.1 - The default branch of the repository has been changed to `main`. As this changed a number of references in the repository's `package.json` and `README.md` files (e.g., for badges, links, etc.), this necessitates a release to publish those changes to npm. [PR #4302](https://github.com/apollographql/apollo-server/pull/4302) +- `apollo-datasource-rest`: changed `trace` method in `RESTDataSource` to `protected` to allow implementation of custom metrics. ## v2.15.0 diff --git a/packages/apollo-datasource-rest/README.md b/packages/apollo-datasource-rest/README.md index aed614df182..e7ba9842ea2 100644 --- a/packages/apollo-datasource-rest/README.md +++ b/packages/apollo-datasource-rest/README.md @@ -190,3 +190,9 @@ From our resolvers, we can access the data source and return the result: }, }, ``` + +### Implementing custom metrics + +By overriding `trace` method, it's possible to implement custom metrics for request timing. + +See the original method [implementation](https://github.com/apollographql/apollo-server/tree/main/packages/apollo-datasource-rest/src/RESTDataSource.ts) or the reference. diff --git a/packages/apollo-datasource-rest/src/RESTDataSource.ts b/packages/apollo-datasource-rest/src/RESTDataSource.ts index ce737ee7184..e37879ecd51 100644 --- a/packages/apollo-datasource-rest/src/RESTDataSource.ts +++ b/packages/apollo-datasource-rest/src/RESTDataSource.ts @@ -249,7 +249,7 @@ export abstract class RESTDataSource extends DataSource { const cacheKey = this.cacheKeyFor(request); const performRequest = async () => { - return this.trace(`${options.method || 'GET'} ${url}`, async () => { + return this.trace(request, async () => { const cacheOptions = options.cacheOptions ? options.cacheOptions : this.cacheOptionsFor && this.cacheOptionsFor.bind(this); @@ -278,10 +278,11 @@ export abstract class RESTDataSource extends DataSource { } } - private async trace( - label: string, + protected async trace( + request: Request, fn: () => Promise, ): Promise { + if (process && process.env && process.env.NODE_ENV === 'development') { // We're not using console.time because that isn't supported on Cloudflare const startTime = Date.now(); @@ -289,6 +290,7 @@ export abstract class RESTDataSource extends DataSource { return await fn(); } finally { const duration = Date.now() - startTime; + const label = `${request.method || 'GET'} ${request.url}`; console.log(`${label} (${duration}ms)`); } } else { diff --git a/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts b/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts index e001fcb42dc..343cd9b43fe 100644 --- a/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts +++ b/packages/apollo-datasource-rest/src/__tests__/RESTDataSource.test.ts @@ -45,7 +45,7 @@ describe('RESTDataSource', () => { await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/foo'); }); @@ -64,7 +64,7 @@ describe('RESTDataSource', () => { await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual( 'https://api.example.com/bar/foo', ); @@ -85,7 +85,7 @@ describe('RESTDataSource', () => { await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://example.com/api/foo'); }); @@ -110,7 +110,7 @@ describe('RESTDataSource', () => { fetch.mockJSONResponseOnce(); await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual( 'https://api-dev.example.com/foo', ); @@ -135,7 +135,7 @@ describe('RESTDataSource', () => { fetch.mockJSONResponseOnce(); await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/foo'); }); @@ -161,7 +161,7 @@ describe('RESTDataSource', () => { offset: 20, }); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual( 'https://api.example.com/posts?username=beyonc%C3%A9&filter=jalape%C3%B1o&limit=10&offset=20', ); @@ -187,7 +187,7 @@ describe('RESTDataSource', () => { await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual( 'https://api.example.com/foo?a=1&api_key=secret', ); @@ -212,7 +212,7 @@ describe('RESTDataSource', () => { await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); // FIXME: request.credentials is not supported by node-fetch // expect(fetch.mock.calls[0][0].credentials).toEqual('include'); }); @@ -237,7 +237,7 @@ describe('RESTDataSource', () => { await dataSource.getFoo(); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].headers.get('Authorization')).toEqual( 'secret', ); @@ -258,7 +258,7 @@ describe('RESTDataSource', () => { await dataSource.postFoo({ foo: 'bar' }); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/foo'); expect(fetch.mock.calls[0][0].body).toEqual( JSON.stringify({ foo: 'bar' }), @@ -283,7 +283,7 @@ describe('RESTDataSource', () => { await dataSource.postFoo(['foo', 'bar']); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/foo'); expect(fetch.mock.calls[0][0].body).toEqual( JSON.stringify(['foo', 'bar']), @@ -319,7 +319,7 @@ describe('RESTDataSource', () => { await dataSource.postFoo(model); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/foo'); expect(fetch.mock.calls[0][0].body).toEqual( JSON.stringify({ foo: 'bar' }), @@ -347,7 +347,7 @@ describe('RESTDataSource', () => { await dataSource.postFoo(form); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual('https://api.example.com/foo'); expect(fetch.mock.calls[0][0].body).not.toEqual('{}'); expect(fetch.mock.calls[0][0].headers.get('Content-Type')).not.toEqual( @@ -389,7 +389,7 @@ describe('RESTDataSource', () => { expect(data).toEqual({ foo: 'bar' }); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].method).toEqual(method); }); } @@ -530,7 +530,7 @@ describe('RESTDataSource', () => { await Promise.all([dataSource.getFoo(1), dataSource.getFoo(1)]); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual( 'https://api.example.com/foo/1', ); @@ -638,7 +638,7 @@ describe('RESTDataSource', () => { dataSource.getFoo(1, 'anotherSecret'), ]); - expect(fetch.mock.calls.length).toEqual(1); + expect(fetch).toBeCalledTimes(1); expect(fetch.mock.calls[0][0].url).toEqual( 'https://api.example.com/foo/1?api_key=secret', ); @@ -764,4 +764,28 @@ describe('RESTDataSource', () => { }); }); }); + + describe('trace', () => { + it('is called once per request', async () => { + const traceMock = jest.fn() + const dataSource = new (class extends RESTDataSource { + baseURL = 'https://api.example.com'; + + getFoo() { + return this.get('foo'); + } + + trace = traceMock; + })(); + + dataSource.httpCache = httpCache; + + fetch.mockJSONResponseOnce(); + + await dataSource.getFoo(); + + expect(traceMock).toBeCalledTimes(1); + expect(traceMock).toBeCalledWith(expect.any(Object),expect.any(Function)); + }); + }) });