feat: Allow for custom logger to be configured and default to console (#186)

Provide the option to configure a custom logger to `RESTDataSource` to replace
the default console logging.

Author: js-lowes

.changeset/great-windows-warn.md

@@ -0,0 +1,18 @@
+---
+'@apollo/datasource-rest': minor
+---
+
+Customize the logger used by `RESTDataSource`.
+By default the `RESTDataSource` will use `console`.
+Common use cases would be to override the default logger with `pino` or `winston`.
+
+E.g.
+```typescript
+const pino = require('pino');
+const loggerPino = pino({});
+const dataSource = new (class extends RESTDataSource {})({
+  logger: loggerPino,
+});
+```
+
+In the example above, all logging calls made by the `RESTDataSource` will now use the `pino` logger instead of the `console` logger.

README.md

@@ -53,6 +53,25 @@ class MoviesAPI extends RESTDataSource {
 
 This README lists all the protected methods. In practice, if you're looking to customize behavior by overriding methods, reading the [source code](https://github.com/apollographql/datasource-rest/tree/main/src/RESTDataSource.ts) is the best option.
 
+#### Constructor
+
+The `RESTDataSource` takes in a `DataSourceConfig` which allows for overriding some default behavior.
+
+Configuration Overrides
+  - `cache` - Custom [`KeyValueCache`](https://www.npmjs.com/package/@apollo/utils.keyvaluecache) implementation
+  - `fetch` - Custom [`Fetcher`](https://www.npmjs.com/package/@apollo/utils.fetcher) implementation
+  - `logger` - Custom [`Logger`](https://www.npmjs.com/package/@apollo/utils.logger) implementation that will replace all logging activity within `RESTDataSource`
+
+To override the RESTDataSource, see the following example code:
+
+```typescript
+const dataSource = new (class extends RESTDataSource {})({
+  cache: customCache,
+  fetch: customFetcher,
+  logger: customLogger,
+});
+```
+
 #### Properties
 
 ##### `baseURL`

package-lock.json

@@ -54,6 +54,7 @@
   "dependencies": {
     "@apollo/utils.fetcher": "^2.0.0",
     "@apollo/utils.keyvaluecache": "^2.0.0",
+    "@apollo/utils.logger": "^2.0.1",
     "@apollo/utils.withrequired": "^2.0.0",
     "@types/http-cache-semantics": "^4.0.1",
     "http-cache-semantics": "^4.1.1",

package.json

@@ -8,6 +8,7 @@ import type { WithRequired } from '@apollo/utils.withrequired';
 import { GraphQLError } from 'graphql';
 import isPlainObject from 'lodash.isplainobject';
 import { HTTPCache } from './HTTPCache';
+import type { Logger } from '@apollo/utils.logger';
 import type { Options as HttpCacheSemanticsOptions } from 'http-cache-semantics';
 
 export type ValueOrPromise<T> = T | Promise<T>;
@@ -124,6 +125,7 @@ const NODE_ENV = process.env.NODE_ENV;
 export interface DataSourceConfig {
   cache?: KeyValueCache;
   fetch?: Fetcher;
+  logger?: Logger;
 }
 
 export interface RequestDeduplicationResult {
@@ -175,9 +177,11 @@ export abstract class RESTDataSource {
   protected httpCache: HTTPCache;
   protected deduplicationPromises = new Map<string, Promise<any>>();
   baseURL?: string;
+  logger: Logger;
 
   constructor(config?: DataSourceConfig) {
     this.httpCache = new HTTPCache(config?.cache, config?.fetch);
+    this.logger = config?.logger ?? console;
   }
 
   // By default, we use `cacheKey` from the request if provided, or the full
@@ -605,7 +609,7 @@ export abstract class RESTDataSource {
   // Override this method to handle these errors in a different way.
   protected catchCacheWritePromiseErrors(cacheWritePromise: Promise<void>) {
     cacheWritePromise.catch((e) => {
-      console.error(`Error writing from RESTDataSource to cache: ${e}`);
+      this.logger.error(`Error writing from RESTDataSource to cache: ${e}`);
     });
   }
 
@@ -622,7 +626,7 @@ export abstract class RESTDataSource {
       } finally {
         const duration = Date.now() - startTime;
         const label = `${request.method || 'GET'} ${url}`;
-        console.log(`${label} (${duration}ms)`);
+        this.logger.info(`${label} (${duration}ms)`);
       }
     } else {
       return fn();

src/RESTDataSource.ts

@@ -14,13 +14,35 @@ import nock from 'nock';
 import { nockAfterEach, nockBeforeEach } from './nockAssertions';
 import type { WithRequired } from '@apollo/utils.withrequired';
 import { Headers as NodeFetchHeaders } from 'node-fetch';
+import type { Logger } from '@apollo/utils.logger';
 
 const apiUrl = 'https://api.example.com';
 
 describe('RESTDataSource', () => {
   beforeEach(nockBeforeEach);
   afterEach(nockAfterEach);
 
+  it('not overriding logger will use "console" as logger', async () => {
+    const dataSource = new (class extends RESTDataSource {})();
+
+    expect(dataSource.logger).toEqual(console);
+  });
+
+  it('providing override to logger will use "console" as logger', async () => {
+    const testLogger: Logger = {
+      debug: jest.fn(),
+      info: jest.fn(),
+      error: jest.fn(),
+      warn: jest.fn(),
+    };
+    const dataSource = new (class extends RESTDataSource {})({
+      logger: testLogger,
+    });
+
+    expect(dataSource.logger).not.toEqual(console);
+    expect(dataSource.logger).toEqual(testLogger);
+  });
+
   describe('constructing requests', () => {
     it('interprets paths relative to the base URL', async () => {
       const dataSource = new (class extends RESTDataSource {