Przeglądaj źródła

[misc] fix collection of open sockets

```
> require('http').globalAgent.sockets
{ 'HOST:PORT:': [{ ... }, { ... }] }
```

- `new URL('http://HOST:PORT:')` throws
  -> test suite `open_sockets`
- gauges were not cleaned up after no agents were present for a given
   key. This lead to continuous emission of the previous connection
   count for a given hostname, following the completion of all requests.
  -> test suite `when all requests complete`
Jakob Ackermann 5 lat temu
rodzic
commit
2d1c84ea2e

+ 23 - 25
libraries/metrics/open_sockets.js

@@ -6,7 +6,6 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 let OpenSocketsMonitor
-const { URL } = require('url')
 const seconds = 1000
 
 // In Node 0.10 the default is 5, which means only 5 open connections at one.
@@ -15,6 +14,27 @@ const seconds = 1000
 require('http').globalAgent.maxSockets = Infinity
 require('https').globalAgent.maxSockets = Infinity
 
+const SOCKETS_HTTP = require('http').globalAgent.sockets
+const SOCKETS_HTTPS = require('https').globalAgent.sockets
+
+// keep track of set gauges and reset them in the next collection cycle
+const SEEN_HOSTS_HTTP = new Set()
+const SEEN_HOSTS_HTTPS = new Set()
+
+function collectOpenConnections(sockets, seenHosts, prefix) {
+  const Metrics = require('./index')
+  Object.keys(sockets).forEach(host => seenHosts.add(host))
+  seenHosts.forEach(host => {
+    // host: 'HOST:PORT:'
+    const hostname = host.split(':')[0]
+    const openConnections = (sockets[host] || []).length
+    if (!openConnections) {
+      seenHosts.delete(host)
+    }
+    Metrics.gauge(`open_connections.${prefix}.${hostname}`, openConnections)
+  })
+}
+
 module.exports = OpenSocketsMonitor = {
   monitor(logger) {
     const interval = setInterval(
@@ -26,29 +46,7 @@ module.exports = OpenSocketsMonitor = {
   },
 
   gaugeOpenSockets() {
-    let agents, hostname, url
-    const Metrics = require('./index')
-    const object = require('http').globalAgent.sockets
-    for (url in object) {
-      agents = object[url]
-      url = new URL(`http://${url}`)
-      hostname =
-        url.hostname != null ? url.hostname.replace(/\./g, '_') : undefined
-      Metrics.gauge(`open_connections.http.${hostname}`, agents.length)
-    }
-    return (() => {
-      const result = []
-      const object1 = require('https').globalAgent.sockets
-      for (url in object1) {
-        agents = object1[url]
-        url = new URL(`https://${url}`)
-        hostname =
-          url.hostname != null ? url.hostname.replace(/\./g, '_') : undefined
-        result.push(
-          Metrics.gauge(`open_connections.https.${hostname}`, agents.length)
-        )
-      }
-      return result
-    })()
+    collectOpenConnections(SOCKETS_HTTP, SEEN_HOSTS_HTTP, 'http')
+    collectOpenConnections(SOCKETS_HTTPS, SEEN_HOSTS_HTTPS, 'https')
   }
 }

+ 130 - 0
libraries/metrics/test/acceptance/metrics_tests.js

@@ -1,4 +1,5 @@
 const os = require('os')
+const http = require('http')
 const { expect } = require('chai')
 const Metrics = require('../..')
 
@@ -84,6 +85,129 @@ describe('Metrics module', function() {
       expect(labels.app).to.equal(APP_NAME)
     })
   })
+
+  describe('open_sockets', function() {
+    const keyServer1 = 'open_connections_http_127_42_42_1'
+    const keyServer2 = 'open_connections_http_127_42_42_2'
+
+    let finish1, finish2, emitResponse1, emitResponse2
+    function resetEmitResponse1() {
+      emitResponse1 = new Promise(resolve => (finish1 = resolve))
+    }
+    resetEmitResponse1()
+    function resetEmitResponse2() {
+      emitResponse2 = new Promise(resolve => (finish2 = resolve))
+    }
+    resetEmitResponse2()
+
+    let server1, server2
+    before(function setupServer1(done) {
+      server1 = http.createServer((req, res) => {
+        res.write('...')
+        emitResponse1.then(() => res.end())
+      })
+      server1.listen(0, '127.42.42.1', done)
+    })
+    before(function setupServer2(done) {
+      server2 = http.createServer((req, res) => {
+        res.write('...')
+        emitResponse2.then(() => res.end())
+      })
+      server2.listen(0, '127.42.42.2', done)
+    })
+    after(function cleanupPendingRequests() {
+      finish1()
+      finish2()
+    })
+    after(function shutdownServer1(done) {
+      if (server1) server1.close(done)
+    })
+    after(function shutdownServer2(done) {
+      if (server2) server2.close(done)
+    })
+
+    let urlServer1, urlServer2
+    before(function setUrls() {
+      urlServer1 = `http://127.42.42.1:${server1.address().port}/`
+      urlServer2 = `http://127.42.42.2:${server2.address().port}/`
+    })
+    describe('gaugeOpenSockets()', function() {
+      beforeEach(function runGaugeOpenSockets() {
+        Metrics.open_sockets.gaugeOpenSockets()
+      })
+
+      describe('without pending connections', function() {
+        it('emits no open_connections', async function() {
+          await expectNoMetricValue(keyServer1)
+          await expectNoMetricValue(keyServer2)
+        })
+      })
+
+      describe('with pending connections for server1', function() {
+        before(function(done) {
+          http.get(urlServer1)
+          http.get(urlServer1)
+          setTimeout(done, 10)
+        })
+
+        it('emits 2 open_connections for server1', async function() {
+          await expectMetricValue(keyServer1, 2)
+        })
+
+        it('emits no open_connections for server2', async function() {
+          await expectNoMetricValue(keyServer2)
+        })
+      })
+
+      describe('with pending connections for server1 and server2', function() {
+        before(function(done) {
+          http.get(urlServer2)
+          http.get(urlServer2)
+          setTimeout(done, 10)
+        })
+
+        it('emits 2 open_connections for server1', async function() {
+          await expectMetricValue(keyServer1, 2)
+        })
+
+        it('emits 2 open_connections for server1', async function() {
+          await expectMetricValue(keyServer1, 2)
+        })
+      })
+
+      describe('when requests finish for server1', function() {
+        before(function(done) {
+          finish1()
+          resetEmitResponse1()
+          http.get(urlServer1)
+
+          setTimeout(done, 10)
+        })
+
+        it('emits 1 open_connections for server1', async function() {
+          await expectMetricValue(keyServer1, 1)
+        })
+
+        it('emits 2 open_connections for server2', async function() {
+          await expectMetricValue(keyServer2, 2)
+        })
+      })
+
+      describe('when all requests complete', function() {
+        before(function(done) {
+          finish1()
+          finish2()
+
+          setTimeout(done, 10)
+        })
+
+        it('emits no open_connections', async function() {
+          await expectNoMetricValue(keyServer1)
+          await expectNoMetricValue(keyServer2)
+        })
+      })
+    })
+  })
 })
 
 function getMetric(key) {
@@ -113,3 +237,9 @@ async function expectMetricValue(key, expectedValue) {
   expect(value.labels.host).to.equal(HOSTNAME)
   expect(value.labels.app).to.equal(APP_NAME)
 }
+
+async function expectNoMetricValue(key) {
+  const metric = getMetric(key)
+  if (!metric) return
+  await expectMetricValue(key, 0)
+}