Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v8.0 Roadmap #1542

Closed
15 of 18 tasks
delvedor opened this issue Aug 26, 2021 · 4 comments
Closed
15 of 18 tasks

v8.0 Roadmap #1542

delvedor opened this issue Aug 26, 2021 · 4 comments
Labels

Comments

@delvedor
Copy link
Member

delvedor commented Aug 26, 2021

Elasticsearch JavaScript Client v8.0 Roadmap

This work is currently happening in the v8 branch and in the elastic/elastic-transport-js repository.

  • Drop old typescript definitions
  • Drop callback-style API
  • Remove the current abort API and use the new AbortController standard
  • Remove the body key from the request
  • Migrate to new separate transport
  • The returned value of API calls is the body and not the HTTP related keys
  • Use a weighted connection pool
  • Migrate to the "undici" http client
  • Drop support for old camelCased keys
  • Migrate from CJS to ESM (on hold)
  • Deprecate the old https://www.npmjs.com/package/elasticsearch package
  • Rename ssl option to tls
  • Remove prototype poisoning protection
  • Remove client extensions API
  • Move to TypeScript
  • Move from emitter-like interface to a diagnostic method
  • Remove username & password properties from Cloud configuration
  • Calling client.close will reject new requests

Drop old typescript definitions

Breaking: Yes | Migration effort: Medium

The current TypeScript definitions will be removed from the client, and the new definitions, which contain request and response definitions as well will be shipped by default. You can already migrate to the new definitions by following this guide.

Drop callback-style API

Breaking: Yes | Migration effort: Large

Maintaining both API styles is not a problem per se, but it makes error handling more convoluted due to async stack traces.
Moving to a full-promise API will solve this issue.

// callback-style api
client.search({ params }, { options }, (err, result) => {
 console.log(err || result)
})

// promise-style api
client.search({ params }, { options })
  .then(console.log)
  .catch(console.log)
  
// async-style (sugar syntax on top of promises)
const response = await client.search({ params }, { options })
console.log(response)

If you are already using the promise-style API, this won't be a breaking change for you.

Remove the current abort API and use the new AbortController standard

Breaking: Yes | Migration effort: Small

The old abort API makes sense for callbacks but it's annoying to use with promises

// callback-style api
const request = client.search({ params }, { options }, (err, result) => {
 console.log(err) // RequestAbortedError
})

request.abort()

// promise-style api
const promise = client.search({ params }, { options })

promise
  .then(console.log)
  .catch(console.log) // RequestAbortedError

promise.abort()

Node v12 has added the standard AbortController API which is designed to work well with both callbacks and promises.

const ac = new AbortController()
client.search({ params }, { signal: ac.signal })
  .then(console.log)
  .catch(console.log) // RequestAbortedError

ac.abort()

Remove the body key from the request

Breaking: Yes | Migration effort: Small

Thanks to the new types we are developing now we know exactly where a parameter should go.
The client API leaks HTTP-related notions in many places, and removing them would definitely improve the DX.

This could be a rather big breaking change, so a double solution could be used during the 8.x lifecycle. (accepting body keys without them being wrapped in the body as well as the current solution).

// from
const response = await client.search({
  index: 'test',
  body: {
    query: {
      match_all: {}
    }
  }
})

// to
const response = await client.search({
  index: 'test',
  query: {
    match_all: {}
  }
})

Migrate to new separate transport

Breaking: Yes | Migration effort: Small to none

The separated transport has been rewritten in TypeScript and has already dropped the callback style API.
Given that now is separated, most of the Elasticsearch specific concepts have been removed, and the client will likely need to extend parts of it for reintroducing them.
If you weren't extending the internals of the client, this won't be a breaking change for you.

The returned value of API calls is the body and not the HTTP related keys

Breaking: Yes | Migration effort: Small

The client API leaks HTTP-related notions in many places, and removing them would definitely improve the DX.
The client will expose a new request-specific option to still get the full response details.

// from
const response = await client.search({
  index: 'test',
  body: {
    query: {
      match_all: {}
    }
  }
})
console.log(response) // { body: SearchResponse, statusCode: number, headers: object, warnings: array }

// to
const response = await client.search({
  index: 'test',
  query: {
    match_all: {}
  }
})
console.log(response) // SearchResponse

// with a bit of TypeScript and JavaScript magic...
const response = await client.search({
  index: 'test',
  query: {
    match_all: {}
  }
}, {
  meta: true
})
console.log(response) // { body: SearchResponse, statusCode: number, headers: object, warnings: array }

Use a weighted connection pool

Breaking: Yes | Migration effort: Small to none

Move from the current cluster connection pool to a weight-based implementation.
This new implementation offers better performances and runs less code in the background, the old connection pool can still be used.
If you weren't extending the internals of the client, this won't be a breaking change for you.

Migrate to the "undici" http client

Breaking: Yes | Migration effort: Small to none

By default, the HTTP client will no longer be the default Node.js HTTP client, but undici instead.
Undici is a brand new HTTP client written from scratch, it offers vastly improved performances and has better support for promises.
Furthermore, it offers comprehensive and predictable error handling. The old HTTP client can still be used.
If you weren't extending the internals of the client, this won't be a breaking change for you.

Drop support for old camelCased keys

Breaking: Yes | Migration effort: Medium

Currently, every path or query parameter could be expressed in both snake_case and camelCase. Internally the client will convert everything to snake_case.
This was done in an effort to reduce the friction of migrating from the legacy to the new client, but now it no longer makes sense.
If you are already using snake_case keys, this won't be a breaking change for you.

Migrate from CJS to ESM (won't happen in v8)

Breaking: Yes | Migration effort: Medium

Currently, Node.js has two module systems, CommonJS and ES Modules.
CommonJS is the native Node.js module system since 2009, while ES Modules has been introduced in 2015 in the JavaScript spec, but landed in Node.js stable this year. The two-module systems are not compatible at all, and while you can use a CJS module in ESM you can't do it the other way around. Furthermore, the instrumentation of ESM modules is not possible yet.

This change won't happen in v8, as ES module loaders are not stable yet and the ecosystem is not ready.

Deprecate the old https://www.npmjs.com/package/elasticsearch package

Breaking: Yes | Migration effort: Large

The legacy client will be officially deprecated and no longer supported.
You can find here a guide to migrate to the new client.

Rename ssl option to tls

Breaking: Yes | Migration effort: Small

People usually refers to this as tls, furthermore, internally we use the tls API and Node.js refers to it as tls everywhere.

// before
const client = new Client({
  node: 'https://localhost:9200',
  ssl: {
    rejectUnauthorized: false
  }
})

// after
const client = new Client({
  node: 'https://localhost:9200',
  tls: {
    rejectUnauthorized: false
  }
})

Remove prototype poisoning protection

Breaking: Yes | Migration effort: Small

Prototype poisoning protection is very useful, but it can cause performances issues with big payloads.
In v8 it will be removed, and the documentation will show how to add it back with a custom serializer.

Remove client extensions API

Breaking: Yes | Migration effort: Large

Nowadays the client support the entire Elasticsearch API, and the transport.request method can be used if necessary. The client extensions API have no reason to exist.

client.extend('utility.index', ({ makeRequest }) => {
  return function _index (params, options) {
    // your code
  }
})

client.utility.index(...)

If you weren't using client extensions, this won't be a breaking change for you.

Move to TypeScript

Breaking: No | Migration effort: None

The new separated transport is already written in TypeScript, and it makes sense that the client v8 will be fully written in TypeScript as well.

Move from emitter-like interface to a diagnostic method

Breaking: Yes | Migration effort: Small

Currently, the client offers a subset of methods of the EventEmitter class, v8 will ship with a diagnostic property which will be a proper event emitter.

// from
client.on('request', console.log)

// to
client.diagnostic.on('request', console.log)

Remove username & password properties from Cloud configuration

Breaking: Yes | Migration effort: Small

The Cloud configuration does not support ApiKey and Bearer auth, while the auth options does.
There is no need to keep the legacy basic auth support in the cloud configuration.

// before
const client = new Client({
  cloud: {
    id: '<cloud-id>',
    username: 'elastic',
    password: 'changeme'
  }
})

// after
const client = new Client({
  cloud: {
    id: '<cloud-id>'
  },
  auth: {
    username: 'elastic',
    password: 'changeme'
  }
})

If you are already passing the basic auth options in the auth configuration, this won't be a breaking change for you.

Calling client.close will reject new requests

Once you call client.close every new request after that will be rejected with a NoLivingConnectionsError. In-flight requests will be executed normally unless an in-flight request requires a retry, in which case it will be rejected.

@ronag
Copy link

ronag commented Oct 13, 2021

@delvedor the abort controller api should probably be more like:

const ac = new AbortController()
client.search({ params, signal: ac.signal })
  .then(console.log)
  .catch(console.log) // RequestAbortedError

ac.abort()

@delvedor
Copy link
Member Author

Thanks for the suggestion @ronag!
I did the change in elastic/elastic-transport-js#21.

trentm added a commit to elastic/apm-agent-nodejs that referenced this issue Oct 25, 2021
feat: support @elastic/elasticsearch@8

Version 8 of the Elasticsearch client has a number of significant changes
and is still in pre-release (elastic/elasticsearch-js#1542).
This commit tests version 8 releases via the `@elastic/elasticsearch-canary`
package.

- Bump the ES used for testing to >7.14 to support the header used by
  ES client v8's product check.
- Add examples/... for tracing ES using v7 and v8 of the client.
- Guard against including a non-string body (e.g. a Buffer) in
  span.context.db.statement. ES client types say 'body' can be:
     body?: string | Buffer | ReadableStream | null

Fixes: #2355
@stale
Copy link

stale bot commented Jan 9, 2022

We understand that this might be important for you, but this issue has been automatically marked as stale because it has not had recent activity either from our end or yours.
It will be closed if no further activity occurs, please write a comment if you would like to keep this going.

Note: in the past months we have built a new client, that has just landed in master. If you want to open an issue or a pr for the legacy client, you should do that in https://github.com/elastic/elasticsearch-js-legacy

@stale stale bot added the stale label Jan 9, 2022
@delvedor delvedor removed the stale label Jan 11, 2022
@stale
Copy link

stale bot commented Apr 16, 2022

We understand that this might be important for you, but this issue has been automatically marked as stale because it has not had recent activity either from our end or yours.
It will be closed if no further activity occurs, please write a comment if you would like to keep this going.

Note: in the past months we have built a new client, that has just landed in master. If you want to open an issue or a pr for the legacy client, you should do that in https://github.com/elastic/elasticsearch-js-legacy

@stale stale bot added the stale label Apr 16, 2022
@stale stale bot closed this as completed Apr 27, 2022
@sethmlarson sethmlarson unpinned this issue Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants