We have an annoying cyclic dependency (Ruby <-> Java) because of `LogStash::Json` and `org.logstash.ext.JrubyEventExtLibrary#load`
- Dominant language
- Java
- Stars
- 14.9k
- Forks
- 3.5k
- Avg merge
- 19h 14m
- Merged PRs (30d)
- 63
Description
Came across this twice the last few days.
`org.logstash.ext.JrubyEventExtLibrary#load` requires `logstash/json.rb` to be loaded because of the second half of its call stack:
```java
PARSER_ERROR = module.defineOrGetModuleUnder("Json").getClass("ParserError");
if (PARSER_ERROR == null) {
throw new RaiseException(runtime, runtime.getClass("StandardError"), "Could not find LogStash::Json::ParserError class", true);
}
GENERATOR_ERROR = module.defineOrGetModuleUnder("Json").getClass("GeneratorError");
if (GENERATOR_ERROR == null) {
throw new RaiseException(runtime, runtime.getClass("StandardError"), "Could not find LogStash::Json::GeneratorError class", true);
}
LOGSTASH_ERROR = module.getClass("Error");
if (LOGSTASH_ERROR == null) {
throw new RaiseException(runtime, runtime.getClass("StandardError"), "Could not find LogStash::Error class", true);
}
```
This is kind of a dirt anti-pattern for one, but this is also causing some practical trouble in writing better (faster to execute + easier to debug mostly) tests for the Java execution:
* You can't actually load `JrubyEventExtLibrary` in Java UTs, so not all functionality of the Ruby Event can be tested from Java
My suggestion would be to simply move `json.rb` to Java completely and drop JrJackson, this gives us a few benefits:
* We don't break timestamp serialization on reloading the pipeline because we have full control of all `ObjectMapper`s from the Java side (https://github.com/guyboertje/jrjackson/issues/62)
* would allow me to fix #8128 instantly :)
* JrJackson won't leak afterburner into out classpath anymore
* We fix the cyclic dependency (better Java tests + cleaner code)
* We can finally upgrade Jackson dep. versions as we please :)
Contributor guide
Assessment
This issue has not been assessed yet.