Improve observability (#834)

* API generation * Added correlation id support * Updated docs * Updated test * Updated code generation * API generation * Updated code generation * Added support for client name and custom context object * Updated docs * Updated test * Fix docs * Updated docs * Added id support also for sniffing * Updated test * Update docs/observability.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Update docs/observability.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Apply suggestions * Update docs/configuration.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Update docs/configuration.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Update docs/observability.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Update docs/observability.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Update docs/observability.asciidoc Co-Authored-By: delvedor <delvedor@users.noreply.github.com> * Apply suggestions * Updated README.md * Fixed test * Addressed suggestions
2019-05-03 17:23:40 +02:00
parent 7d810f8110
commit 14fc908d4b
279 changed files with 2314 additions and 142 deletions
--- a/test/behavior/observability.test.js
+++ b/test/behavior/observability.test.js
@ -0,0 +1,334 @@
+'use strict'
+
+const { test } = require('tap')
+const lolex = require('lolex')
+const { Client, Transport } = require('../../index')
+const {
+  connection: { MockConnection, MockConnectionSniff }
+} = require('../utils')
+const noop = () => {}
+
+test('Request id', t => {
+  t.test('Default generateRequestId', t => {
+    const { generateRequestId } = Transport.internals
+    t.type(generateRequestId, 'function')
+
+    const genReqId = generateRequestId()
+    t.type(genReqId, 'function')
+
+    for (var i = 1; i <= 10; i++) {
+      t.strictEqual(genReqId(), i)
+    }
+
+    t.end()
+  })
+
+  t.test('Custom generateRequestId', t => {
+    t.plan(7)
+
+    const options = { context: { winter: 'is coming' } }
+
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection,
+      generateRequestId: function (requestParams, requestOptions) {
+        t.match(requestParams, { method: 'GET', path: '/' })
+        t.match(requestOptions, options)
+        return 'custom-id'
+      }
+    })
+
+    client.on('request', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.request.id, 'custom-id')
+    })
+
+    client.on('response', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.request.id, 'custom-id')
+    })
+
+    client.info({}, options, t.error)
+  })
+
+  t.test('Custom request id in method options', t => {
+    t.plan(5)
+
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection
+    })
+
+    client.on('request', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.request.id, 'custom-id')
+    })
+
+    client.on('response', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.request.id, 'custom-id')
+    })
+
+    client.info({}, { id: 'custom-id' }, t.error)
+  })
+
+  t.test('Sniff and correlation id', t => {
+    t.test('sniffOnStart - should autogenerate the id', t => {
+      t.plan(2)
+
+      const client = new Client({
+        node: 'http://localhost:9200',
+        Connection: MockConnectionSniff,
+        sniffOnStart: true
+      })
+
+      client.on('sniff', (err, { meta }) => {
+        t.error(err)
+        t.strictEqual(meta.request.id, 1)
+      })
+    })
+
+    t.test('sniffOnConnectionFault - should use the request id', t => {
+      t.plan(5)
+
+      const client = new Client({
+        nodes: ['http://localhost:9200', 'http://localhost:9201'],
+        Connection: MockConnectionSniff,
+        sniffOnConnectionFault: true,
+        maxRetries: 0
+      })
+
+      client.on('request', (e, { meta }) => {
+        t.strictEqual(meta.request.id, 'custom')
+      })
+
+      client.on('response', (e, { meta }) => {
+        t.strictEqual(meta.request.id, 'custom')
+      })
+
+      client.on('sniff', (e, { meta }) => {
+        t.strictEqual(meta.request.id, 'custom')
+      })
+
+      client.transport.request({
+        path: '/500',
+        method: 'GET'
+      }, {
+        id: 'custom',
+        headers: { timeout: 'true' }
+      }, noop)
+    })
+
+    t.end()
+  })
+
+  t.test('Resurrect should use the same request id of the request that starts it', t => {
+    t.plan(2)
+
+    const clock = lolex.install({ toFake: ['Date'] })
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection,
+      sniffOnConnectionFault: true,
+      maxRetries: 0
+    })
+
+    const conn = client.connectionPool.getConnection()
+    client.connectionPool.markDead(conn)
+    clock.tick(1000 * 61)
+
+    client.on('resurrect', (err, meta) => {
+      t.error(err)
+      t.strictEqual(meta.request.id, 'custom')
+      clock.uninstall()
+    })
+
+    client.info({}, { id: 'custom' }, noop)
+  })
+
+  t.end()
+})
+
+test('Request context', t => {
+  t.test('no value', t => {
+    t.plan(5)
+
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection
+    })
+
+    client.on('request', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.context, null)
+    })
+
+    client.on('response', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.context, null)
+    })
+
+    client.info(t.error)
+  })
+
+  t.test('custom value', t => {
+    t.plan(5)
+
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection
+    })
+
+    client.on('request', (err, { meta }) => {
+      t.error(err)
+      t.deepEqual(meta.context, { winter: 'is coming' })
+    })
+
+    client.on('response', (err, { meta }) => {
+      t.error(err)
+      t.deepEqual(meta.context, { winter: 'is coming' })
+    })
+
+    client.info({}, { context: { winter: 'is coming' } }, t.error)
+  })
+
+  t.end()
+})
+
+test('Client name', t => {
+  t.test('Property of the client instance', t => {
+    const client = new Client({
+      node: 'http://localhost:9200',
+      name: 'cluster'
+    })
+    t.strictEqual(client.name, 'cluster')
+    t.end()
+  })
+
+  t.test('Is present in the event metadata', t => {
+    t.plan(6)
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection,
+      name: 'cluster'
+    })
+
+    client.on('request', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.name, 'cluster')
+    })
+
+    client.on('response', (err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.name, 'cluster')
+    })
+
+    client.info((err, { meta }) => {
+      t.error(err)
+      t.strictEqual(meta.name, 'cluster')
+    })
+  })
+
+  t.test('Sniff and client name', t => {
+    t.test('sniffOnStart', t => {
+      t.plan(2)
+
+      const client = new Client({
+        node: 'http://localhost:9200',
+        Connection: MockConnectionSniff,
+        sniffOnStart: true
+      })
+
+      client.on('sniff', (err, { meta }) => {
+        t.error(err)
+        t.strictEqual(meta.name, 'elasticsearch-js')
+      })
+    })
+
+    t.test('sniffOnConnectionFault', t => {
+      t.plan(5)
+
+      const client = new Client({
+        nodes: ['http://localhost:9200', 'http://localhost:9201'],
+        Connection: MockConnectionSniff,
+        sniffOnConnectionFault: true,
+        maxRetries: 0
+      })
+
+      client.on('request', (e, { meta }) => {
+        t.strictEqual(meta.name, 'elasticsearch-js')
+      })
+
+      client.on('response', (e, { meta }) => {
+        t.strictEqual(meta.name, 'elasticsearch-js')
+      })
+
+      client.on('sniff', (e, { meta }) => {
+        t.strictEqual(meta.name, 'elasticsearch-js')
+      })
+
+      client.transport.request({
+        path: '/500',
+        method: 'GET'
+      }, {
+        headers: { timeout: 'true' }
+      }, noop)
+    })
+
+    t.end()
+  })
+
+  t.test('Resurrect should have the client name configured', t => {
+    t.plan(2)
+
+    const clock = lolex.install({ toFake: ['Date'] })
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection,
+      sniffOnConnectionFault: true,
+      maxRetries: 0
+    })
+
+    const conn = client.connectionPool.getConnection()
+    client.connectionPool.markDead(conn)
+    clock.tick(1000 * 61)
+
+    client.on('resurrect', (err, meta) => {
+      t.error(err)
+      t.strictEqual(meta.name, 'elasticsearch-js')
+      clock.uninstall()
+    })
+
+    client.info({}, { id: 'custom' }, noop)
+  })
+
+  t.test('Resurrect should have the client name configured (child client)', t => {
+    t.plan(2)
+
+    const clock = lolex.install({ toFake: ['Date'] })
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection,
+      sniffOnConnectionFault: true,
+      maxRetries: 0
+    })
+
+    const child = client.child({
+      name: 'child-client'
+    })
+
+    const conn = client.connectionPool.getConnection()
+    client.connectionPool.markDead(conn)
+    clock.tick(1000 * 61)
+
+    client.on('resurrect', (err, meta) => {
+      t.error(err)
+      t.strictEqual(meta.name, 'child-client')
+      clock.uninstall()
+    })
+
+    child.info({}, { id: 'custom' }, noop)
+  })
+
+  t.end()
+})
--- a/test/behavior/resurrect.test.js
+++ b/test/behavior/resurrect.test.js
@ -37,7 +37,7 @@ const { Client, events } = require('../../index')
 */

 test('Should execute the recurrect API with the ping strategy', t => {
-  t.plan(6)
+  t.plan(8)

  const clock = lolex.install({ toFake: ['Date'] })
  const q = workq()
@ -59,6 +59,8 @@ test('Should execute the recurrect API with the ping strategy', t => {
      t.strictEqual(meta.strategy, 'ping')
      t.false(meta.isAlive)
      t.strictEqual(meta.connection.id, 'node0')
+      t.strictEqual(meta.name, 'elasticsearch-js')
+      t.deepEqual(meta.request, { id: 2 })
    })

    q.add((q, done) => {
@ -89,7 +91,7 @@ test('Should execute the recurrect API with the ping strategy', t => {
 })

 test('Resurrect a node and handle 502/3/4 status code', t => {
-  t.plan(11)
+  t.plan(15)

  const clock = lolex.install({ toFake: ['Date'] })
  const q = workq()
@ -114,10 +116,13 @@ test('Resurrect a node and handle 502/3/4 status code', t => {
      maxRetries: 0
    })

+    var idCount = 2
    client.on(events.RESURRECT, (err, meta) => {
      t.error(err)
      t.strictEqual(meta.strategy, 'ping')
      t.strictEqual(meta.connection.id, 'node0')
+      t.strictEqual(meta.name, 'elasticsearch-js')
+      t.deepEqual(meta.request, { id: idCount++ })
      if (count < 3) {
        t.false(meta.isAlive)
      } else {
@ -156,7 +161,7 @@ test('Resurrect a node and handle 502/3/4 status code', t => {
 })

 test('Should execute the recurrect API with the optimistic strategy', t => {
-  t.plan(6)
+  t.plan(8)

  const clock = lolex.install({ toFake: ['Date'] })
  const q = workq()
@ -179,6 +184,8 @@ test('Should execute the recurrect API with the optimistic strategy', t => {
      t.strictEqual(meta.strategy, 'optimistic')
      t.true(meta.isAlive)
      t.strictEqual(meta.connection.id, 'node0')
+      t.strictEqual(meta.name, 'elasticsearch-js')
+      t.deepEqual(meta.request, { id: 2 })
    })

    q.add((q, done) => {
--- a/test/unit/child.test.js
+++ b/test/unit/child.test.js
@ -211,3 +211,72 @@ test('Should share the event emitter', t => {

  t.end()
 })
+
+test('Should create a child client (generateRequestId check)', t => {
+  t.plan(6)
+
+  function generateRequestId1 () {
+    var id = 0
+    return () => `trace-1-${id++}`
+  }
+
+  function generateRequestId2 () {
+    var id = 0
+    return () => `trace-2-${id++}`
+  }
+
+  const client = new Client({
+    node: 'http://localhost:9200',
+    Connection: MockConnection,
+    generateRequestId: generateRequestId1()
+  })
+  const child = client.child({
+    Connection: MockConnection,
+    generateRequestId: generateRequestId2()
+  })
+
+  var count = 0
+  client.on('request', (err, { meta }) => {
+    t.error(err)
+    t.strictEqual(
+      meta.request.id,
+      count++ === 0 ? 'trace-1-0' : 'trace-2-0'
+    )
+  })
+
+  client.info(err => {
+    t.error(err)
+    child.info(t.error)
+  })
+})
+
+test('Should create a child client (name check)', t => {
+  t.plan(8)
+
+  const client = new Client({
+    node: 'http://localhost:9200',
+    Connection: MockConnection,
+    name: 'parent'
+  })
+  const child = client.child({
+    Connection: MockConnection,
+    name: 'child'
+  })
+
+  t.strictEqual(client.name, 'parent')
+  t.strictEqual(child.name, 'child')
+
+  var count = 0
+  client.on('request', (err, { meta }) => {
+    t.error(err)
+    t.strictEqual(
+      meta.name,
+      count++ === 0 ? 'parent' : 'child'
+    )
+  })
+
+  client.info(err => {
+    t.error(err)
+    child.info(t.error)
+  })
+})
--- a/test/unit/connection-pool.test.js
+++ b/test/unit/connection-pool.test.js
@ -118,7 +118,12 @@ test('API', t => {
        const href = 'http://localhost:9200/'
        var connection = pool.addConnection(href)
        pool.markDead(connection)
-        pool.resurrect(Date.now() + 1000 * 60 * 3, (isAlive, connection) => {
+        const opts = {
+          now: Date.now() + 1000 * 60 * 3,
+          requestId: 1,
+          name: 'elasticsearch-js'
+        }
+        pool.resurrect(opts, (isAlive, connection) => {
          t.true(isAlive)
          connection = pool.connections.get(connection.id)
          t.strictEqual(connection.deadCount, 0)
@ -139,7 +144,12 @@ test('API', t => {
        const href = 'http://localhost:9200/'
        var connection = pool.addConnection(href)
        pool.markDead(connection)
-        pool.resurrect(Date.now() + 1000 * 60 * 3, (isAlive, connection) => {
+        const opts = {
+          now: Date.now() + 1000 * 60 * 3,
+          requestId: 1,
+          name: 'elasticsearch-js'
+        }
+        pool.resurrect(opts, (isAlive, connection) => {
          t.false(isAlive)
          connection = pool.connections.get(connection.id)
          t.strictEqual(connection.deadCount, 2)
@ -162,7 +172,12 @@ test('API', t => {
      const href = 'http://localhost:9200/'
      var connection = pool.addConnection(href)
      pool.markDead(connection)
-      pool.resurrect(Date.now() + 1000 * 60 * 3, (isAlive, connection) => {
+      const opts = {
+        now: Date.now() + 1000 * 60 * 3,
+        requestId: 1,
+        name: 'elasticsearch-js'
+      }
+      pool.resurrect(opts, (isAlive, connection) => {
        t.true(isAlive)
        connection = pool.connections.get(connection.id)
        t.strictEqual(connection.deadCount, 1)
@ -182,7 +197,12 @@ test('API', t => {
      const href = 'http://localhost:9200/'
      var connection = pool.addConnection(href)
      pool.markDead(connection)
-      pool.resurrect(Date.now() + 1000 * 60 * 3, (isAlive, connection) => {
+      const opts = {
+        now: Date.now() + 1000 * 60 * 3,
+        requestId: 1,
+        name: 'elasticsearch-js'
+      }
+      pool.resurrect(opts, (isAlive, connection) => {
        t.ok(isAlive === null)
        t.ok(connection === null)
        connection = pool.connections.get(href)
--- a/test/unit/events.test.js
+++ b/test/unit/events.test.js
@ -40,6 +40,8 @@ test('Should emit a request event when a request is performed', t => {
      headers: null,
      warnings: null,
      meta: {
+        context: null,
+        name: 'elasticsearch-js',
        request: {
          params: {
            method: 'GET',
@ -59,7 +61,8 @@ test('Should emit a request event when a request is performed', t => {
            headers: null,
            compression: false,
            warnings: null
-          }
+          },
+          id: 1
        },
        connection: {
          id: 'http://localhost:9200'
@ -98,6 +101,8 @@ test('Should emit a response event in case of a successful response', t => {
      },
      warnings: null,
      meta: {
+        context: null,
+        name: 'elasticsearch-js',
        request: {
          params: {
            method: 'GET',
@ -117,7 +122,8 @@ test('Should emit a response event in case of a successful response', t => {
            headers: null,
            compression: false,
            warnings: null
-          }
+          },
+          id: 1
        },
        connection: {
          id: 'http://localhost:9200'
@ -154,6 +160,8 @@ test('Should emit a response event with the error set', t => {
      headers: null,
      warnings: null,
      meta: {
+        context: null,
+        name: 'elasticsearch-js',
        request: {
          params: {
            method: 'GET',
@ -173,7 +181,8 @@ test('Should emit a response event with the error set', t => {
            headers: null,
            compression: false,
            warnings: null
-          }
+          },
+          id: 1
        },
        connection: {
          id: 'http://localhost:9200'
--- a/test/unit/transport.test.js
+++ b/test/unit/transport.test.js
@ -704,11 +704,13 @@ test('Should call markAlive with a successful response', t => {
 })

 test('Should call resurrect on every request', t => {
-  t.plan(3)
+  t.plan(5)

  class CustomConnectionPool extends ConnectionPool {
-    resurrect (now) {
+    resurrect ({ now, requestId, name }) {
      t.type(now, 'number')
+      t.type(requestId, 'number')
+      t.type(name, 'string')
    }
  }

@ -725,7 +727,8 @@ test('Should call resurrect on every request', t => {
    maxRetries: 3,
    requestTimeout: 30000,
    sniffInterval: false,
-    sniffOnStart: false
+    sniffOnStart: false,
+    name: 'elasticsearch-js'
  })

  transport.request({
@ -2107,3 +2110,30 @@ test('Should accept custom querystring in the optons object', t => {

  t.end()
 })
+
+test('Should pass request params and options to generateRequestId', t => {
+  t.plan(3)
+
+  const pool = new ConnectionPool({ Connection: MockConnection })
+  pool.addConnection('http://localhost:9200')
+
+  const params = { method: 'GET', path: '/hello' }
+  const options = { context: { winter: 'is coming' } }
+
+  const transport = new Transport({
+    emit: () => {},
+    connectionPool: pool,
+    serializer: new Serializer(),
+    maxRetries: 3,
+    requestTimeout: 30000,
+    sniffInterval: false,
+    sniffOnStart: false,
+    generateRequestId: function (requestParams, requestOptions) {
+      t.deepEqual(requestParams, params)
+      t.deepEqual(requestOptions, options)
+      return 'id'
+    }
+  })
+
+  transport.request(params, options, t.error)
+})
--- a/test/utils/MockConnection.js
+++ b/test/utils/MockConnection.js
@ -73,6 +73,47 @@ class MockConnectionError extends Connection {
  }
 }

+class MockConnectionSniff extends Connection {
+  request (params, callback) {
+    var aborted = false
+    const sniffResult = {
+      nodes: {
+        'node-1': {
+          http: {
+            publish_address: 'localhost:9200'
+          },
+          roles: ['master', 'data', 'ingest']
+        },
+        'node-2': {
+          http: {
+            publish_address: 'localhost:9201'
+          },
+          roles: ['master', 'data', 'ingest']
+        }
+      }
+    }
+    const stream = intoStream(JSON.stringify(sniffResult))
+    stream.statusCode = setStatusCode(params.path)
+    stream.headers = {
+      'content-type': 'application/json;utf=8',
+      'date': new Date().toISOString(),
+      'connection': 'keep-alive',
+      'content-length': '205'
+    }
+    process.nextTick(() => {
+      if (!aborted) {
+        if (params.headers.timeout) {
+          callback(new TimeoutError('Request timed out', params), null)
+        } else {
+          callback(null, stream)
+        }
+      }
+    })
+    return {
+      abort: () => { aborted = true }
+    }
+  }
+}
 function setStatusCode (path) {
  const statusCode = Number(path.slice(1))
  if (Number.isInteger(statusCode)) {
@ -81,4 +122,9 @@ function setStatusCode (path) {
  return 200
 }

-module.exports = { MockConnection, MockConnectionTimeout, MockConnectionError }
+module.exports = {
+  MockConnection,
+  MockConnectionTimeout,
+  MockConnectionError,
+  MockConnectionSniff
+}