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

AndroidInstrumentationRegistry should have a clearer purpose/api #502

Closed
breedx-splk opened this issue Jul 30, 2024 · 1 comment
Closed

Comments

@breedx-splk
Copy link
Contributor

This is a follow-up from this comment.

The AndroidInstrumentationRegistryImpl seems like it's intended to ONLY encapsulate the auto-discovered instrumentations available on the classpath. Even though internal, having a register() method is slightly confusing (it's unclear when that should/could be used). Furthermore, it appears that any interactions with the registry (get() or getAll()) will cause the lazy field to be initialized, which will load instrumentation classes. Depending on the configuration, this may not be what users expect, and at worst could have side effects if instrumentations are behaving poorly during construction.

Propose removing register() from the registry and renaming the registry to something like AutodetectedClasspathInstrumentations, and only calling get() or getAll() when the configuration has discoverInstrumenations = TRUE.

@LikeTheSalad
Copy link
Contributor

Done in #516

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