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 client accessible from global object #503

Closed
1 of 9 tasks
luismiramirez opened this issue Nov 16, 2021 · 3 comments
Closed
1 of 9 tasks

Make client accessible from global object #503

luismiramirez opened this issue Nov 16, 2021 · 3 comments

Comments

@luismiramirez
Copy link
Member

luismiramirez commented Nov 16, 2021

The background

The client instance needs to be moved between packages to access the tracer, which gives access to the error tracking helpers, the root, and the current span.

Currently, Client and Tracer instances are moved into the different integrations internals:

The client is now stored in global object (#493).

The proposal

  • Create a helper, like a class function for Client that provides clean access to the stored client (Add helper for globally stored client #534)
  • Change the integrations to access the Client and the Tracer from global object instead of passing them to the functions
    • Express
    • Koa
    • Apollo
    • NextJS
    • Redis
    • PG
    • HTTP

Caveats

  • This may change how the framework integrations are initialized from customer's applications, so don't consider any of these changes part of patch or minor release.
  • Avoid opening a big PR for the whole thing. One for each integration should be OK
@tombruijn
Copy link
Member

tombruijn commented Nov 17, 2021

Users won't necessarily have to change anything to make it work at the moment? They should remove the client being given to the express middleware, for example, but it would work if they pass it in anyway. So it's not necessarily a breaking change until we change how the config is initialized.

Eventually I would hope we can make some kind of function to set the global config, rather than set it when the configuration class gets initialized. That would make it a breaking change. For example:

Appsignal.configure({ active: true })

function configure(options) {
  const config = new Appsignal(options)
  storeInGlobal(config)
}

But we can do this in small steps and only have the last step (changing how the config is initialized) be the major change.

@luismiramirez
Copy link
Member Author

That way of initializing Appsignal is exactly what I was thinking about 👌🏼

But we can do this in small steps and only have the last step (changing how the config is initialized) be the major change.

Agree

@luismiramirez
Copy link
Member Author

Non relevant with the new v3.0

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

No branches or pull requests

2 participants