build(deps): bump @elastic/elasticsearch from 7.7.1 to 8.2.1

[Fdawgs]

closes #147

[Fdawgs]

Almost there.
The only part now throwing errors is:

pino-elasticsearch/lib.js

Line 62 in 6deb633

Connection: opts.Connection || Connection

Which is throwing this.Connection is not a constructor.
Can't find any reference to changes to this in the v8 release notes: https://www.elastic.co/guide/en/elasticsearch/client/javascript-api/8.0/changelog-client.html

[mcollina]

cc @trentm @delvedor could you help?

[trentm]

Hi. I'm not a maintainer of @elastic/elasticsearch, so take this with a grain of salt. :)

In @elastic/elasticsearch@8 there isn't a Connection export anymore. There are now BaseConnection, UndiciConnection and HttpConnection. The default used (if no Connection Client option is given) is UndiciConnection. Unfortunately (in both v7 and v8) if new Client({ Connection: undefined }) is passed in, the client attempts to use that undefined value. One way you could do this is to only pass a Connection option to new Client if one was given to pinoElasticSearch. See patch below.

I also needed a tweak to docker-compose-v8.yml to get Elasticsearch to start using HTTP (as oppsed to HTTPS and auth).

After those changes the tests passed for me:

% npm test

> pino-elasticsearch@6.2.0 test
> standard && tap test/*.test.js -j 1 --no-coverage --timeout 120

 PASS  test/acceptance.test.js 47 OK 3s
 PASS  test/unit.test.js 26 OK 188.134ms



  🌈 SUMMARY RESULTS 🌈


Suites:   2 passed, 2 of 2 completed
Asserts:  73 passed, of 73
Time:   32s

Note that that took 32 seconds. Either there is some hang in the tests or it is intentional that there is a ~30s delay in there. I didn't look into which.

diff --git a/docker-compose-v8.yml b/docker-compose-v8.yml
index 04e854a..f9d2be8 100644
--- a/docker-compose-v8.yml
+++ b/docker-compose-v8.yml
@@ -3,8 +3,11 @@ version: '3.8'
 services:
   elasticsearch:
     image: docker.elastic.co/elasticsearch/elasticsearch:8.3.3
-    environment:
+    environment:
       - discovery.type=single-node
+      # Elasticsearch 8.x has HTTPS and auth on by default. This option is
+      # needed to use HTTP and no auth (as used in the tests).
+      - xpack.security.enabled=false
     container_name: elasticsearch
     ports: ['9200:9200']

diff --git a/lib.js b/lib.js
index 79a6efa..87d9864 100644
--- a/lib.js
+++ b/lib.js
@@ -3,7 +3,7 @@
 /* eslint no-prototype-builtins: 0 */

 const split = require('split2')
-const { Client, Connection } = require('@elastic/elasticsearch')
+const { Client } = require('@elastic/elasticsearch')

 function pinoElasticSearch (opts) {
   if (opts['bulk-size']) {
@@ -54,13 +54,16 @@ function pinoElasticSearch (opts) {
     return value
   }, { autoDestroy: true })

-  const client = new Client({
+  const clientOpts = {
     node: opts.node,
     auth: opts.auth,
     cloud: opts.cloud,
-    ssl: { rejectUnauthorized: opts.rejectUnauthorized },
-    Connection: opts.Connection || Connection
-  })
+    ssl: { rejectUnauthorized: opts.rejectUnauthorized }
+  }
+  if (opts.Connection) {
+    clientOpts.Connection = opts.Connection
+  }
+  const client = new Client(clientOpts)

   const esVersion = Number(opts['es-version']) || 7
   const index = opts.index || 'pino'

Cheers.

[trentm]

Oh, another consideration here. @elastic/elasticsearch@8 dropped support for node v10, and then node v12:

% npm info @elastic/elasticsearch@8.0.0 engines
{ node: '>=12' }

% npm info @elastic/elasticsearch@8.2.0 engines
{ node: '>=14' }

and I notice this project still claims node v10 support:

% npm info pino-elasticsearch engines
{ node: '>=10' }

So there might be something to consider there.

[Fdawgs]

and I notice this project still claims node v10 support:

% npm info pino-elasticsearch engines

{ node: '>=10' }

So there might be something to consider there.

Thanks @trentm, dropped that in #159 😊