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

Remove Guice as a dependency #43881

Open
rjernst opened this issue Jul 2, 2019 · 6 comments
Open

Remove Guice as a dependency #43881

rjernst opened this issue Jul 2, 2019 · 6 comments
Labels
:Core/Infra/Core Core issues without another label Meta Team:Core/Infra Meta label for core/infra team

Comments

@rjernst
Copy link
Member

rjernst commented Jul 2, 2019

Elasticsearch currently uses a forked version of Guice that is embedded into the codebase to manage construction of parts of the Node on startup. In addition to the overhead of keeping a forked copy, Guice blocks us from having a stable plugin api because plugin authors can get at any internal service from any plugin.

This is a meta issue to track removal of Guice. Thanks to an effort spanning multiple years by several people, it is well relegated to just a few places within Node construction:

  • actions: all TransportAction implementations use Guice for injected construction
  • gateway services: this is mostly due to depending on action instances as a means of inter service communication
@tlrx tlrx added the :Core/Infra/Core Core issues without another label label Jul 3, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

rjernst added a commit to rjernst/elasticsearch that referenced this issue Jul 9, 2019
This commit removes most of the remaining direct execute calls, mostly
in gateway code. They are replaced with registering the actions as
internal actions.

relates elastic#43881
rjernst added a commit that referenced this issue Jul 10, 2019
)

This commit removes most of the remaining direct execute calls, mostly
in gateway code. They are replaced with registering the actions as
internal actions.

relates #43881
rjernst added a commit to rjernst/elasticsearch that referenced this issue Jul 11, 2019
There are currently 3 variants of TransportAction.execute. The
implementations of these require additional ctor arguments to all
TransportAction implementations. While the non test uses can be
converted to using NodeClient to execute other actions, using that for
test cases would be cumbersome and defeat the purpose of unit tests
testing an action's implementation directly. This commit adds a public
test-only utility method for test to use to call execute. This method
will continue to be available when the execute implementations are
collapsed and made package private.

relates elastic#43881
rjernst added a commit that referenced this issue Jul 11, 2019
There are currently 3 variants of TransportAction.execute. The
implementations of these require additional ctor arguments to all
TransportAction implementations. While the non test uses can be
converted to using NodeClient to execute other actions, using that for
test cases would be cumbersome and defeat the purpose of unit tests
testing an action's implementation directly. This commit adds a public
test-only utility method for test to use to call execute. This method
will continue to be available when the execute implementations are
collapsed and made package private.

relates #43881
@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@williamrandolph williamrandolph removed the needs:triage Requires assignment of a team area label label Jan 6, 2021
@williamrandolph
Copy link
Contributor

As of early 2021, this is still an important meta issue for the Core/Infra team.

@sarnobat
Copy link

sarnobat commented Mar 30, 2021

Guice Version question

As part of my security audit, I need to find out what version of Guice you forked from. I'm using elasticsearch-6.8.6.jar. Are you able to identify what version of Guice this would have linked to?

Thanks for the effort to clean it up. I know from experience how tough it is to remove IoC dependencies.

@rjernst
Copy link
Member Author

rjernst commented Mar 31, 2021

@sarnobat It looks like Guice was forked at version 2.0 (2a19160), but note that we have slowly removed much of the functionality of it, to the point where it has very little functionality now other than the one injector that we create, at node startup, with only handful of guice created objects.

@sarnobat
Copy link

sarnobat commented Apr 1, 2021

@rjernst - very helpful, thank you.

@arteam arteam added v8.1.0 and removed v8.0.0 labels Jan 12, 2022
@mark-vieira mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this issue Mar 21, 2022
All of this was removed via static analysis with Idea. We shouldn't have almost 6k lines of unused
code around the codebase. This also makes it at least a little clearer what parts of Guice
we still use.

relates elastic#43881
original-brownbear added a commit that referenced this issue Mar 24, 2022
All of this was removed via static analysis with Idea. We shouldn't have almost 6k lines of unused
code around the codebase. This also makes it at least a little clearer what parts of Guice
we still use.

relates #43881
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@csoulios csoulios added v8.6.0 and removed v8.5.0 labels Sep 21, 2022
@kingherc kingherc added v8.7.0 and removed v8.6.0 labels Nov 16, 2022
@rjernst rjernst added v8.8.0 and removed v8.7.0 labels Feb 8, 2023
@gmarouli gmarouli added v8.9.0 and removed v8.8.0 labels Apr 26, 2023
@quux00 quux00 added v8.11.0 and removed v8.10.0 labels Aug 16, 2023
@mattc58 mattc58 added v8.12.0 and removed v8.11.0 labels Oct 4, 2023
@rjernst rjernst removed the v8.13.0 label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label Meta Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests