Skip to content

Commit

Permalink
Instrument all of our middleware (github#17527)
Browse files Browse the repository at this point in the history
* Add instrument-middleware.js

* Make it do some fancy require-ing

* Use it

* Prefix names

* dot prefix

* Improve async detection

* Add some comments

* Can't instrument error handler
  • Loading branch information
JasonEtco authored Jan 27, 2021
1 parent a168b96 commit 58319a2
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 30 deletions.
19 changes: 19 additions & 0 deletions lib/instrument-middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const path = require('path')
const statsd = require('./statsd')

module.exports = function instrumentMiddleware (relativePath) {
// Requires the file as if it were being required from '../middleware/index.js'.
// This is a little wonky, but let's us write `app.use(instrument(path))` and
// maintain the name of the file, instead of hard-coding it for each middleware.
const middleware = require(path.resolve(__dirname, '../middleware', relativePath))

// Check if the middleware is an async function, to use the appropriate timer
const isAsyncFunction = middleware.constructor.name === 'AsyncFunction'

// Name it `middleware.<filename>`
const name = `middleware.${path.basename(relativePath)}`

return isAsyncFunction
? statsd.asyncTimer(middleware, name)
: statsd.timer(middleware, name)
}
61 changes: 31 additions & 30 deletions middleware/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const express = require('express')
const instrument = require('../lib/instrument-middleware')

const isDevelopment = process.env.NODE_ENV === 'development'

Expand All @@ -24,7 +25,7 @@ module.exports = function (app) {
// See https://expressjs.com/en/guide/behind-proxies.html
app.set('trust proxy', 1)
app.use(require('./rate-limit'))
app.use(require('./handle-invalid-paths'))
app.use(instrument('./handle-invalid-paths'))

// *** Security ***
app.use(require('./cors'))
Expand All @@ -43,50 +44,50 @@ module.exports = function (app) {
// *** Config and context for redirects ***
app.use(require('./req-utils')) // Must come before record-redirect and events
app.use(require('./record-redirect'))
app.use(require('./detect-language')) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
app.use(asyncMiddleware(require('./context'))) // Must come before early-access-*, handle-redirects
app.use(instrument('./detect-language')) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
app.use(asyncMiddleware(instrument('./context'))) // Must come before early-access-*, handle-redirects

// *** Redirects, 3xx responses ***
// I ordered these by use frequency
app.use(require('connect-slashes')(false))
app.use(require('./redirects/external'))
app.use(require('./redirects/help-to-docs'))
app.use(require('./redirects/language-code-redirects')) // Must come before contextualizers
app.use(require('./redirects/handle-redirects')) // Must come before contextualizers
app.use(instrument('./redirects/external'))
app.use(instrument('./redirects/help-to-docs'))
app.use(instrument('./redirects/language-code-redirects')) // Must come before contextualizers
app.use(instrument('./redirects/handle-redirects')) // Must come before contextualizers

// *** Config and context for rendering ***
app.use(require('./find-page')) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
app.use(require('./block-robots'))
app.use(instrument('./find-page')) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
app.use(instrument('./block-robots'))

// *** Rendering, 2xx responses ***
// I largely ordered these by use frequency
app.use(require('./archived-enterprise-versions-assets')) // Must come before static/assets
app.use(instrument('./archived-enterprise-versions-assets')) // Must come before static/assets
app.use('/dist', express.static('dist'))
app.use('/assets', express.static('assets'))
app.use('/public', express.static('data/graphql'))
app.use('/events', require('./events'))
app.use('/csrf', require('./csrf-route'))
app.use('/search', require('./search'))
app.use(require('./archived-enterprise-versions'))
app.use(require('./robots'))
app.use(/(\/.*)?\/early-access$/, require('./contextualizers/early-access-links'))
app.use(require('./categories-for-support-team'))
app.use(require('./loaderio-verification'))
app.get('/_500', asyncMiddleware(require('./trigger-error')))
app.use('/events', instrument('./events'))
app.use('/csrf', instrument('./csrf-route'))
app.use('/search', instrument('./search'))
app.use(instrument('./archived-enterprise-versions'))
app.use(instrument('./robots'))
app.use(/(\/.*)?\/early-access$/, instrument('./contextualizers/early-access-links'))
app.use(instrument('./categories-for-support-team'))
app.use(instrument('./loaderio-verification'))
app.get('/_500', asyncMiddleware(instrument('./trigger-error')))

// *** Preparation for render-page ***
app.use(asyncMiddleware(require('./contextualizers/enterprise-release-notes')))
app.use(require('./contextualizers/graphql'))
app.use(require('./contextualizers/rest'))
app.use(require('./contextualizers/webhooks'))
app.use(require('./breadcrumbs'))
app.use(require('./early-access-breadcrumbs'))
app.use(require('./enterprise-server-releases'))
app.use(require('./dev-toc'))
app.use(require('./featured-links'))
app.use(require('./learning-track'))
app.use(asyncMiddleware(instrument('./contextualizers/enterprise-release-notes')))
app.use(instrument('./contextualizers/graphql'))
app.use(instrument('./contextualizers/rest'))
app.use(instrument('./contextualizers/webhooks'))
app.use(instrument('./breadcrumbs'))
app.use(instrument('./early-access-breadcrumbs'))
app.use(instrument('./enterprise-server-releases'))
app.use(instrument('./dev-toc'))
app.use(instrument('./featured-links'))
app.use(instrument('./learning-track'))

// *** Rendering, must go last ***
app.get('/*', asyncMiddleware(require('./render-page')))
app.get('/*', asyncMiddleware(instrument('./render-page')))
app.use(require('./handle-errors'))
}

0 comments on commit 58319a2

Please sign in to comment.