Merge pull request #14 from apilytics/request-response-sizes

Send request and response body size information with metrics

Author: ruohola

.github/workflows/cd.yml

@@ -32,39 +32,43 @@ jobs:
       - name: "Install dependencies"
         run: yarn install --frozen-lockfile
 
-      - name: "Build the sources"
-        run: yarn build
-
-      - name: "Publish to npm"
-        id: publish
+      - name: "Bump package versions"
+        id: bump
         run: |
-          echo '//registry.npmjs.org/:_authToken=${{ secrets.NPM_TOKEN }}' > .npmrc
-          yarn release ${{ github.event.inputs.release-type }}
+          yarn bump ${{ github.event.inputs.release-type }}
           echo "::set-output name=version::$(node --print 'require("./lerna.json").version')"
 
       - name: "Update the changelog"
         # Find the first line that starts with `###` or `## [<number>` from the CHANGELOG and insert the new version header before it.
-        run: |
-          current_date="$(date -u '+%Y-%m-%d')"
-          sed -i "0,/^\(###\|## *\[[0-9]\).*/{s//## [${{ steps.publish.outputs.version }}] - ${current_date}\n\n&/}" CHANGELOG.md
+        run: sed -i "0,/^\(###\|## *\[[0-9]\).*/{s//## [${{ steps.bump.outputs.version }}] - $(date -u '+%Y-%m-%d')\n\n&/}" CHANGELOG.md
 
       - name: "Extract version's changelog for release notes"
         # 1. Find the lines between the first `## [<number>` and the second `## [<number>`.
         # 2. Remove all leading and trailing newlines from the output.
         run: sed '1,/^## *\[[0-9]/d;/^## *\[[0-9]/Q' CHANGELOG.md | sed -e :a -e '/./,$!d;/^\n*$/{$d;N;};/\n$/ba' > release_notes.txt
 
       - name: "Commit and tag the changes"
-        uses: EndBug/add-and-commit@8c12ff729a98cfbcd3fe38b49f55eceb98a5ec02  # v7.5.0
-        with:
-          add: '["lerna.json", "*package.json", "CHANGELOG.md"]'
-          message: 'Release ${{ steps.publish.outputs.version }}'
-          tag: 'v${{ steps.publish.outputs.version }} --annotate --file /dev/null'
-          default_author: github_actions
-          pathspec_error_handling: exitImmediately
+        run: |
+          git config user.name 'github-actions'
+          git config user.email '41898282+github-actions[bot]@users.noreply.github.com'
+          git add lerna.json \*package.json CHANGELOG.md
+          git commit --message='Release ${{ steps.bump.outputs.version }}'
+          git tag --annotate --message='' v${{ steps.bump.outputs.version }}
+
+      - name: "Build the sources"
+        run: yarn build
+
+      - name: "Publish to npm"
+        run: |
+          echo '//registry.npmjs.org/:_authToken=${{ secrets.NPM_TOKEN }}' > .npmrc
+          yarn release
+
+      - name: "Push the changes"
+        run: git push --follow-tags
 
       - name: "Create a GitHub release"
         uses: softprops/action-gh-release@1e07f4398721186383de40550babbdf2b84acfc5  # v1
         with:
-          tag_name: v${{ steps.publish.outputs.version }}
-          name: v${{ steps.publish.outputs.version }}
+          tag_name: v${{ steps.bump.outputs.version }}
+          name: v${{ steps.bump.outputs.version }}
           body_path: release_notes.txt

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Send request and response body size information with metrics.
 - Thoroughly document all exported symbols with JSDoc.
 - Enable `inlineSources` for source maps.
 

package.json

@@ -6,11 +6,11 @@
   "scripts": {
     "build": "lerna run build",
     "clean": "lerna run clean",
-    "clean:git-head": "lerna exec node \\$LERNA_ROOT_PATH/scripts/clean-git-head.js",
-    "release": "lerna publish --force-publish='*' --exact --no-git-tag-version --no-git-reset --no-verify-access --yes && yarn clean:git-head",
+    "bump": "lerna version --force-publish='*' --exact --no-git-tag-version --yes",
+    "release": "lerna publish --no-verify-access --yes from-package",
     "format": "prettier --write '**/*.{ts,js,json}' && eslint --fix --max-warnings=0 --ext=.ts,.js .",
     "lint": "prettier --check '**/*.{ts,js,json}' && eslint --max-warnings=0 --ext=.ts,.js .",
-    "type-check": "yarn tsc --noEmit",
+    "type-check": "tsc --noEmit",
     "test": "jest --verbose",
     "test:cov": "yarn test --coverage",
     "postinstall": "yarn --cwd=packages/core build"

packages/core/README.md

@@ -50,6 +50,8 @@ const myApilyticsMiddleware = async (req, handler) => {
     query: req.queryString,
     method: req.method,
     statusCode: res.statusCode,
+    requestSize: req.bodyBytes.length,
+    responseSize: res.bodyBytes.length,
     timeMillis: timer(),
   });
   return res;

packages/core/__tests__/index.test.ts

@@ -142,6 +142,30 @@ describe('sendApilyticsMetrics()', () => {
     expect(data).not.toHaveProperty('query');
   });
 
+  it('should handle empty values correctly', async () => {
+    sendApilyticsMetrics({
+      apiKey,
+      path: '',
+      method: '',
+      timeMillis: 0,
+      query: '',
+      statusCode: null,
+      requestSize: undefined,
+      responseSize: undefined,
+      apilyticsIntegration: undefined,
+      integratedLibrary: undefined,
+    });
+
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+
+    const data = JSON.parse(clientRequestMock.write.mock.calls[0]);
+    expect(data).toStrictEqual({
+      path: '',
+      method: '',
+      timeMillis: 0,
+    });
+  });
+
   it('should hide HTTP errors in production', async () => {
     // @ts-ignore: Assigning to a read-only property.
     process.env.NODE_ENV = 'production';

packages/core/src/index.ts

@@ -9,6 +9,8 @@ interface Params {
   timeMillis: number;
   query?: string;
   statusCode?: number | null;
+  requestSize?: number;
+  responseSize?: number;
   apilyticsIntegration?: string;
   integratedLibrary?: string;
 }
@@ -29,6 +31,8 @@ interface Params {
  * @param params.statusCode - Status code for the sent HTTP response.
  *     Can be omitted (or null) if the middleware could not get the status code
  *     for the response. E.g. if the inner request handling threw an exception.
+ * @param params.requestSize - Size of the user's HTTP request's body in bytes.
+ * @param params.responseSize - Size of the sent HTTP response's body in bytes.
  * @param params.apilyticsIntegration - Name of the Apilytics integration that's
  *     calling this, e.g. 'apilytics-node-express'.
  *     No need to pass this when calling from user code.
@@ -46,6 +50,8 @@ interface Params {
  *      query: req.queryString,
  *      method: req.method,
  *      statusCode: res.statusCode,
+ *      requestSize: req.bodyBytes.length,
+ *      responseSize: res.bodyBytes.length,
  *      timeMillis: timer(),
  *    });
  */
@@ -56,6 +62,8 @@ export const sendApilyticsMetrics = ({
   timeMillis,
   query,
   statusCode,
+  requestSize,
+  responseSize,
   apilyticsIntegration,
   integratedLibrary,
 }: Params): void => {
@@ -64,6 +72,8 @@ export const sendApilyticsMetrics = ({
     query: query || undefined,
     method,
     statusCode: statusCode ?? undefined,
+    requestSize,
+    responseSize,
     timeMillis,
   });
   let apilyticsVersion = `${

packages/express/__tests__/index.test.ts

@@ -39,12 +39,17 @@ describe('apilyticsMiddleware()', () => {
       throw new Error();
     }
 
+    if (req.url?.includes('empty')) {
+      res.status(200).end();
+      return;
+    }
+
     if (req.method === 'POST') {
-      res.status(201).end();
+      res.status(201).send('created');
       return;
     }
 
-    res.status(200).end();
+    res.status(200).send('ok');
   };
 
   const createAgent = ({
@@ -94,6 +99,7 @@ describe('apilyticsMiddleware()', () => {
       path: '/',
       method: 'GET',
       statusCode: 200,
+      responseSize: 2,
       timeMillis: expect.any(Number),
     });
     expect(data['timeMillis']).toEqual(Math.trunc(data['timeMillis']));
@@ -129,10 +135,34 @@ describe('apilyticsMiddleware()', () => {
       query: '?param=foo&param2=bar',
       method: 'POST',
       statusCode: 201,
+      requestSize: 0,
+      responseSize: 7,
       timeMillis: expect.any(Number),
     });
   });
 
+  it('should handle zero request and response sizes', async () => {
+    const agent = createAgent({ apiKey });
+    const response = await agent.post('/empty');
+    expect(response.status).toEqual(200);
+
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+    const data = JSON.parse(clientRequestMock.write.mock.calls[0]);
+    expect(data.requestSize).toEqual(0);
+    expect(data.responseSize).toEqual(0);
+  });
+
+  it('should handle non zero request and response sizes', async () => {
+    const agent = createAgent({ apiKey });
+    const response = await agent.post('/dummy').send({ hello: 'world' });
+    expect(response.status).toEqual(201);
+
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+    const data = JSON.parse(clientRequestMock.write.mock.calls[0]);
+    expect(data.requestSize).toEqual(17);
+    expect(data.responseSize).toEqual(7);
+  });
+
   it('should be disabled if API key is unset', async () => {
     const agent = createAgent({ apiKey: undefined });
     const response = await agent.get('/');
@@ -154,7 +184,19 @@ describe('apilyticsMiddleware()', () => {
       path: '/error',
       method: 'GET',
       statusCode: 500,
+      responseSize: expect.any(Number),
       timeMillis: expect.any(Number),
     });
   });
+
+  it('should handle undefined content lengths', async () => {
+    const agent = createAgent({ apiKey });
+    const numberSpy = jest
+      .spyOn(global, 'Number')
+      .mockImplementation(() => NaN);
+    const response = await agent.get('/empty');
+    numberSpy.mockRestore();
+    expect(response.status).toEqual(200);
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+  });
 });

packages/express/src/index.ts

@@ -36,12 +36,25 @@ export const apilyticsMiddleware = (
         req.originalUrl,
         'http://_', // Cannot parse a relative URL, so make it absolute.
       );
+
+      const _requestSize = Number(req.headers['content-length']);
+      const requestSize = isNaN(_requestSize) ? undefined : _requestSize;
+
+      const _responseSize = Number(
+        // @ts-ignore: `_contentLength` is not typed, but it does exist sometimes
+        // when the header doesn't. Even if it doesn't this won't fail at runtime.
+        res.getHeader('content-length') ?? res._contentLength,
+      );
+      const responseSize = isNaN(_responseSize) ? undefined : _responseSize;
+
       sendApilyticsMetrics({
         apiKey,
         path,
         query,
         method: req.method,
         statusCode: res.statusCode,
+        requestSize,
+        responseSize,
         timeMillis: timer(),
         apilyticsIntegration: 'apilytics-node-express',
         integratedLibrary: `express/${EXPRESS_VERSION}`,

packages/next/__tests__/index.test.ts

@@ -40,18 +40,23 @@ describe('withApilytics()', () => {
     }
 
     if (req.url?.includes('empty')) {
+      res.status(200).end();
+      return;
+    }
+
+    if (req.url?.includes('no-url-or-method')) {
       req.url = undefined;
       req.method = undefined;
-      res.end();
+      res.status(200).end();
       return;
     }
 
     if (req.method === 'POST') {
-      res.status(201).end();
+      res.status(201).send('created');
       return;
     }
 
-    res.status(200).end();
+    res.status(200).send('ok');
   };
 
   const createAgent = ({
@@ -114,6 +119,7 @@ describe('withApilytics()', () => {
       path: '/',
       method: 'GET',
       statusCode: 200,
+      responseSize: 2,
       timeMillis: expect.any(Number),
     });
     expect(data['timeMillis']).toEqual(Math.trunc(data['timeMillis']));
@@ -149,10 +155,34 @@ describe('withApilytics()', () => {
       query: '?param=foo&param2=bar',
       method: 'POST',
       statusCode: 201,
+      requestSize: 0,
+      responseSize: 7,
       timeMillis: expect.any(Number),
     });
   });
 
+  it('should handle zero request and response sizes', async () => {
+    const agent = createAgent({ apiKey });
+    const response = await agent.post('/empty');
+    expect(response.status).toEqual(200);
+
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+    const data = JSON.parse(clientRequestMock.write.mock.calls[0]);
+    expect(data.requestSize).toEqual(0);
+    expect(data.responseSize).toEqual(0);
+  });
+
+  it('should handle non zero request and response sizes', async () => {
+    const agent = createAgent({ apiKey });
+    const response = await agent.post('/dummy').send({ hello: 'world' });
+    expect(response.status).toEqual(201);
+
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+    const data = JSON.parse(clientRequestMock.write.mock.calls[0]);
+    expect(data.requestSize).toEqual(17);
+    expect(data.responseSize).toEqual(7);
+  });
+
   it('should be disabled if API key is unset', async () => {
     const agent = createAgent({ apiKey: undefined });
     const response = await agent.get('/');
@@ -174,21 +204,34 @@ describe('withApilytics()', () => {
     expect(data).toStrictEqual({
       path: '/error',
       method: 'GET',
+      responseSize: 0,
       timeMillis: expect.any(Number),
     });
   });
 
   it('should use correct default values', async () => {
     const agent = createAgent({ apiKey });
-    const response = await agent.get('/empty');
+    const response = await agent.get('/no-url-or-method');
     expect(response.status).toEqual(200);
 
     const data = JSON.parse(clientRequestMock.write.mock.calls[0]);
     expect(data).toStrictEqual({
       path: '',
       method: '',
       statusCode: 200,
+      responseSize: 0,
       timeMillis: expect.any(Number),
     });
   });
+
+  it('should handle undefined content lengths', async () => {
+    const agent = createAgent({ apiKey });
+    const numberSpy = jest
+      .spyOn(global, 'Number')
+      .mockImplementation(() => NaN);
+    const response = await agent.get('/empty');
+    numberSpy.mockRestore();
+    expect(response.status).toEqual(200);
+    expect(requestSpy).toHaveBeenCalledTimes(1);
+  });
 });