elastic / elastic-transport-js

Transport classes and utilities shared among Node.js Elastic client libraries
Apache License 2.0
4 stars 24 forks source link

Make it possible to provide custom user agent to avoid `os` calls? #77

Open karfau opened 7 months ago

karfau commented 7 months ago

🚀 Feature Proposal

Instead of relying on the node builtin os to create the user agent, it would be nice to be able to avoid those calls by configuring a the user agent string when creating the client or via an ENV variable

Motivation

In order to use the client in a runtime that restricts access to the os builtin, like https://val.town ~or https://deno.com/deploy~ (on deno deploy it works, providing (linux 0.0.0-00000000-generic-x64; Node.js v18.18.0))

Example

when creating the client instance:

const client = new Client({
  cloud: { id: '<cloud-id>' },
  auth: { apiKey: 'base64EncodedKey' },
  userAgent: 'my custom user agent string',
})

or via env variable:

process.env.ELASTICSEARCH_USER_AGENT = 'my custom user agent string';

one of the options would be good enough and maybe you are aware of even better options?

karfau commented 7 months ago

Another idea that just occurred to me would be to build the userAgent in a fault tolerant way

const tryOr = (callback:() => string) => {
  try {
    return callback();
  } catch {
    return `unknown ${callback.name}`;
  }
};
const userAgent = `elastic-transport-js/${clientVersion} (${tryOr(os.platform)} ${tryOr(os.release)}-${tryOr(os.arch)}; Node.js ${process.version})` // eslint-disable-line

or something similar, but of course the options suggested in the OP give the users more options, even if it would just replace the things insice the () and leave everything until the / as is.

JoshMock commented 7 months ago

Thanks for opening this @karfau! We have a few open issues on the JS client to support other JavaScript runtime environments besides Node.js. It's not currently at the top of our priority list, but we are tracking all these requests internally so we can get to it at some point. I'll add yours there, and do my best to reach out if and when we do make that improvement.

karfau commented 7 months ago

Do I understand correctly that you are not up for a pull request of some kind?

JoshMock commented 7 months ago

Pull requests are absolutely welcome! If you're interested in contributing an implementation, I'm more than happy to talk through it here and review PRs. There are some notes in our CONTRIBUTING.md about it to set your expectations and to make sure we're all in agreement about the best approach.

One of the most important parts of this particular improvement will be ensuring that we can run our test suites in a Deno environment (or Bun, etc.) as part of our CI process, for both this repo and elasticsearch-js. That will give us the confidence to merge and ship this kind of change.