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

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

Open
karfau opened this issue Dec 3, 2023 · 4 comments
Open

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

karfau opened this issue Dec 3, 2023 · 4 comments

Comments

@karfau
Copy link

karfau commented Dec 3, 2023

🚀 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
Copy link
Author

karfau commented Dec 3, 2023

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
Copy link
Member

JoshMock commented Dec 4, 2023

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
Copy link
Author

karfau commented Dec 4, 2023

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

@JoshMock
Copy link
Member

JoshMock commented Dec 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants