Metrics with Prometheus

[platan]

Idea behind this change is described in #2068

This change contains:

• /metrics resource with access limit with default metrics (disabled by default)
• METRICS_PROMETHEUS_ENABLED environment variable to enable metrics
• METRICS_PROMETHEUS_ALLOWED_IPS env variable for controlling allowed IP adresses

[shields-ci]

	Warnings
⚠️	This PR modified the server but none of the service tests. That's okay so long as it's refactoring existing code. Otherwise, please consider adding tests to the service: How-to

	Messages
📖	✨ Thanks for your contribution to Shields, @platan!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

[platan]

I would like to have tests for this. But I do not know is it possible to run service tests with config.metrics.prometheus.enabled set to false and true.
My first attempt:
services/metrics/metrics.tester.js

'use strict'

const ServiceTester = require('../service-tester')

const t = new ServiceTester({ id: 'metrics', title: 'Prometheus metrics', pathPrefix: '' })
module.exports = t

t.create('metric are disabled by default')
  .get('/metrics')
  .expectStatus(404)

// below test will fail, we have to set config.metrics.prometheus.enabled = true
// const config = require('../../lib/server-config')
// config.metrics.prometheus.enabled = true

t.create('metric can be enabled by feature flag')
  .get('/metrics')
  .expectStatus(200)
  .expectBodyContains('nodejs_version_info')

Both tests requires change in services/service-tester.js (we have response which is not a JSON)

diff --git a/services/service-tester.js b/services/service-tester.js
index 91223be..17009af 100644
--- a/services/service-tester.js
+++ b/services/service-tester.js
@@ -62,12 +62,18 @@ class ServiceTester {
       })
       // eslint-disable-next-line mocha/prefer-arrow-callback
       .finally(function() {
+        let content;
+        try {
+          content = JSON.parse(this._response.body)
+        } catch(e) {
+          content = this._response.body;
+        }
         // `this` is the IcedFrisby instance.
         trace.logTrace(
           'outbound',
           emojic.shield,
           'Response',
-          JSON.parse(this._response.body)
+          content
         )
       })

I've tried with separate test files for both cases, but server is started only once for all tests.
@paulmelnikow maybe you have an idea how to test both cases?

[paulmelnikow]

The approach I'd probably take to testing this is to test it with a lower-level unit test. I just did something similar for the GitHub token acceptor in https://github.com/badges/shields/pull/2021/files#diff-e852a48eb7b4b8dcdf24b0bb30321a60.

The module exposes a function setRoutes, which gets a camp instance and adds routes to it. The unit test creates a new server, invokes setRoutes, and then fires some requests at it using got or fetch.

We have to trust that the wiring in server.js works, and don't get coverage of the config option, though that code is simple enough that it seems like a decent tradeoff.

There's some similar code for the github admin route and its test:

https://github.com/badges/shields/blob/master/services/github/auth/admin.js
https://github.com/badges/shields/blob/master/services/github/auth/admin.spec.js

[platan]

I've refactored the code and I've added an access limit. @paulmelnikow thanks for the hints about testing! :-)

[paulmelnikow]

👍

[platan]

Can we merge it?

[paulmelnikow]

Let's merge it! I'll make sure @espadrine is aware of it before we turn it on in production.

[shields-deployment]

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch: