digma-ai / digma-intellij-plugin

Digma JetBrains plugin
MIT License
32 stars 7 forks source link

the java framework discovery classes can be changed to stateless and static services #1118

Open shalom938 opened 1 year ago

shalom938 commented 1 year ago

impact: endpoint navigation not stable might not work for users

currently the java framework discovery classes (MicronautFramework,GrpcFramework,SpringBootFramework,AbstractJaxrsFramework) are members of JavaLanguageService, they need a lateInit because they require JavaPsiFacade search methods, but indexes are not ready when JavaLanguageService is constructed. a late Init is confusing and error prone. these classes can be changed to static services without any state or initialization. when they require an annotation or class then search for it in place when required. the search facilities are very fast and anyway we do all discovery in the background. this will avoid the need to keep references to PSI elements in these classes which is discouraged by jetbrains. and also avoid the lateInit methods. see for example JavaSpanDiscoveryUtils and JavaCodeObjectDiscovery are static method and stateless, everything needed for the discovery is acquires in place when needed. so there is no need for any late init processes or keeping references to PSI elements.

if there is a need for long initialization of any of these classes then it should be a project service that is called first time when first needed to initialize itself in smart mode when indexes are ready. or it can be instantiated in a startup activity in a ReadAction in smart mode.

Minimizing read access periods: we need to minimize the periods that we hold a read access, in JavaEndpointNavigationProvider its not easy because of the way the framework classes are designed, it will require too much duplicate code to implement it for each framework discovery class. it will probably be easier to change the design in a way that will make it easier to reduce read access time. they need to be more like JavaSpanNavigationProvider.

shalom938 commented 1 year ago

@shaykeren the impact of this issue is if we get a ProcessCanceledException the endpoint links will not work.

shaykeren commented 1 year ago

will resolve #1265