elastic / elastic/logstash

We have an annoying cyclic dependency (Ruby <-> Java) because of `LogStash::Json` and `org.logstash.ext.JrubyEventExtLibrary#load`

Open
#8,154 1 comment 1 reaction 0 assignees View on GitHub
code cleanup design
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.