-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(plugins): spring custom plugins #10389
Conversation
…om-plugins # Conflicts: # metadata-io/src/main/java/com/linkedin/metadata/service/BusinessAttributeUpdateHookService.java # metadata-jobs/pe-consumer/src/main/java/com/datahub/event/hook/BusinessAttributeUpdateHook.java # metadata-jobs/pe-consumer/src/test/java/com/datahub/event/hook/BusinessAttributeUpdateHookTest.java # metadata-service/configuration/src/main/resources/application.yaml
@david-leifker in this change then, the plugin classloader will no longer have access to the core application classloader? I know we discussed this last time, and it seems like this will help by adding isolation between core application and plugin dependencies. It doesn't help us solve for initializing core components like logging and metrics in the core app though. That could be solved separately, but just wanted to confirm my assumption there |
Another question, if we are able to reference the main application spring context from the plugin spring context, can we pull in the metrics reporter exclusively? |
7ba26ff
to
b7417c2
Compare
I believe that the classes already imported would be available to plugins. The Spring context is isolated and the packageScan can include pages to load into this isolated context which is used for the plugin. The packages are not restricted to any package location so beans or components from anything can be autowired. |
Awesome, that's helpful context. So in our case in which we want to reference classes that are contained in the core application classloader, we would just initialize our plugin with the directive to load packages in datahub gms that contain beans that we want. |
…tahub into spring-custom-plugins
# Conflicts: # docs/how/updating-datahub.md
@@ -105,6 +85,95 @@ public PluginFactory loadPlugins() { | |||
return this; | |||
} | |||
|
|||
/** | |||
* Memory intensive operation because of the size of the jars. Limit packages, classes scanned, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the limit being applied?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The packages and class names are used in the acceptPackages and acceptClass
new ClassGraph()
.acceptPackages(packageNames.stream().distinct().toArray(String[]::new) .acceptClasses(classNames.stream().distinct().toArray(String[]::new))
IntStream.concat( | ||
IntStream.of(baseClazz.getName().hashCode()), | ||
configs.stream().mapToInt(AspectPluginConfig::hashCode))) | ||
.sum(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, so this is creating a key like
sum ( classLoaders.hashCode..., aspectPluginConfigClass.hashCode... )
Is this guaranteed to be unique?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unique within a given jvm execution. It might leak a bit when/if jars are being constantly replaced since each time that happens we'll be creating orphans. This was primarily added to help with tests where we run multiple concurrent test threads and memory usage was triggering OOM.
# Conflicts: # entity-registry/src/main/java/com/linkedin/metadata/aspect/validation/StructuredPropertiesValidator.java
Co-authored-by: Kevin Chun <[email protected]> Co-authored-by: Kevin Chun <[email protected]>
Allow custom plugins to use Spring for injection and autoconfiguration of custom plugins.
This is in addition to the ClassGraph (non-Spring) plugin loader. Spring is optional.
Created a single 20MB dependency for plugins to use. Created a separate metadata-io-api module to isolate most of the non-essential classes.
The artifact is
io.acryl:datahub-custom-plugin-lib
found in the module:metadata-integration:java:custom-plugin-lib
Notes: The Spring context is isolated so all required classes must be present in the
packageScan
configuration for Spring to find.Additional Changes:
Checklist