fix(plugin-openlineage-event-listener): Fix NoClassDefFoundError when emitting query statistics - #28463
Open
jja725 wants to merge 2 commits into
Open
fix(plugin-openlineage-event-listener): Fix NoClassDefFoundError when emitting query statistics#28463jja725 wants to merge 2 commits into
jja725 wants to merge 2 commits into
Conversation
… emitting query statistics ObjectMapper.findAndRegisterModules() service-loads every Jackson module on the plugin class path, including jackson-module-blackbird, a runtime dependency of openlineage-java. Blackbird generates its accessor lambdas inside the class loader of the serialized class. QueryStatistics is an SPI class loaded by the server's application class loader, which cannot see the plugin's jars, so serializing it on a real server fails with NoClassDefFoundError: com/fasterxml/jackson/module/blackbird/ser/ToBooleanFunction and no COMPLETE event is emitted. Register Jdk8Module and JavaTimeModule explicitly instead; they were the only functional modules discovered before, so the emitted facet is unchanged. Add a test that runs the listener through an isolated plugin class loader mirroring PluginClassLoader, so this class of bug shows up in the regular unit tests instead of only on a packaged server. Fixes prestodb#28460 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Reviewer's GuideThe PR fixes packaged-server OpenLineage failures by replacing Jackson ServiceLoader discovery with explicit functional modules, then verifies the class-loader behavior both in-process and against a Docker-packaged coordinator through a dedicated CI workflow. Sequence diagram for isolated OpenLineage query statistics emissionsequenceDiagram
participant Coordinator as PrestoCoordinator
participant Listener as OpenLineageEventListener
participant Mapper as QUERY_STATISTICS_MAPPER
participant SPI as QueryStatistics
participant Receiver as OpenLineageReceiver
Coordinator->>Listener: getCompletedEvent
Listener->>Mapper: writeValueAsString
Mapper->>SPI: serialize QueryStatistics
Note over Mapper,SPI: Explicit Jdk8Module and JavaTimeModule avoid Blackbird ServiceLoader discovery
Mapper-->>Listener: JSON presto_query_statistics facet
Listener->>Receiver: emit COMPLETE event
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
jja725
force-pushed
the
jj/fix-openlineage-blackbird
branch
from
September 9, 2026 21:30
5a72904 to
2b395ba
Compare
jja725
force-pushed
the
jj/fix-openlineage-blackbird
branch
2 times, most recently
from
September 10, 2026 07:48
e95a79c to
92b6c5c
Compare
…penLineage event listener Run the listener inside the packaged coordinator image with ContainerQueryRunner in Java-cluster mode and send its events to a real Marquez server started on the cluster network, following Trino's TestOpenLineageEventListenerMarquezIntegration. Marquez validates events against the OpenLineage specification before storing them, so the test asserts through the Marquez API that the query's job reached COMPLETED, that the presto_metadata and presto_query_statistics run facets arrived, and that the tpch.tiny.nation input dataset was registered with its schema. The coordinator log must not contain NoClassDefFoundError or a stack trace from the plugin. ContainerQueryRunner.Config gains setEventListenerProperties to write the coordinator's event-listener.properties. The existing prestocpp-linux-container-tests workflow discovers the test automatically; its path filter now also covers the plugin module.
jja725
force-pushed
the
jj/fix-openlineage-blackbird
branch
from
September 10, 2026 12:07
92b6c5c to
54f286b
Compare
Contributor
Author
|
@Dilli-Babu-Godari @imjalpreet could u take a look at this when you are free? Thanks. I also added a container test for openlineage #28467. |
imjalpreet
requested changes
Sep 17, 2026
imjalpreet
left a comment
imjalpreet
left a comment
Member
There was a problem hiding this comment.
Thanks for the fix, @jja725.
I have a few suggestions.
Member
There was a problem hiding this comment.
@jja725, any reason for including this test in the presto-native-execution module?
Since .github/workflows/prestocpp-linux-container-tests.yml adds presto-openlineage-event-listener/** to the paths filter, every future one-line change to the OpenLineage plugin now triggers a multi-hour native C++ build, just to run a test that explicitly sets setNativeCluster(false) and never touches a native worker.
Member
There was a problem hiding this comment.
The test class also has nothing to do with native execution. I would recommend presto-product-tests as the module. That module is the home for many E2E docker based integration tests
Comment on lines
+64
to
+74
| private static final List<String> SPI_PACKAGES = ImmutableList.of( | ||
| "com.facebook.presto.spi.", | ||
| "com.fasterxml.jackson.annotation.", | ||
| "com.fasterxml.jackson.module.afterburner.", | ||
| "io.airlift.slice.", | ||
| "com.facebook.airlift.units.", | ||
| "org.openjdk.jol.", | ||
| "com.facebook.presto.common", | ||
| "com.facebook.drift.annotations.", | ||
| "com.facebook.drift.TException", | ||
| "com.facebook.drift.TApplicationException"); |
Member
There was a problem hiding this comment.
nit: could we directly use PluginManagerUtil.SPI_PACKAGES here? It's already public, so a test-scope dependency on presto-main-base should be fine.
Comment on lines
+178
to
+187
| public ContainerQueryRunner( | ||
| int coordinatorPort, | ||
| String catalog, | ||
| String schema, | ||
| int numberOfWorkers, | ||
| boolean isNativeCluster, | ||
| boolean isSidecarEnabled, | ||
| int functionServerPort, | ||
| boolean enableFunctionServer, | ||
| Map<String, String> eventListenerProperties) |
Member
There was a problem hiding this comment.
I would prefer adding the new parameter to the existing constructor since the current constructor is anyway unused.
| assertTrue(fields.containsAll(ImmutableSet.of("n_nationkey", "n_regionkey")), "dataset fields: " + fields); | ||
|
|
||
| String coordinatorLogs = ((ContainerQueryRunner) getQueryRunner()).getCoordinatorLogs(); | ||
| assertFalse(coordinatorLogs.contains("NoClassDefFoundError"), "coordinator log contains NoClassDefFoundError"); |
Member
There was a problem hiding this comment.
This will fail on any unrelated NoClassDefFoundError anywhere in the coordinator log, from any plugin. Scoping it to the OpenLineage package might be better, what do you think?
| tail(marquez.getLogs()), | ||
| tail(((ContainerQueryRunner) getQueryRunner()).getCoordinatorLogs()))); | ||
| } | ||
| MILLISECONDS.sleep(500); |
Member
There was a problem hiding this comment.
nit: it's preferred to not have Thread.sleep() in tests. A bounded poll with a deadline might be better.
| return job; | ||
| } | ||
| } | ||
| if (System.nanoTime() > deadline) { |
Member
There was a problem hiding this comment.
nit:
Suggested change
| if (System.nanoTime() > deadline) { | |
| if (System.nanoTime() - deadline > 0) { |
| protected GenericContainer<?> functionServer; | ||
| protected int functionServerPort; | ||
| protected boolean enableFunctionServer; | ||
| protected final Map<String, String> eventListenerProperties; |
Member
There was a problem hiding this comment.
nit: maybe move this before protected GenericContainer<?> functionServer; to maintain the current ordering
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The OpenLineage event listener built its
QueryStatisticsmapper withObjectMapper.findAndRegisterModules(). On a packaged server this service-loadsjackson-module-blackbird, a transitive runtime dependency ofopenlineage-java. Blackbird generates its accessor lambdas inside the class loader of the serialized class.QueryStatisticsis an SPI class loaded by the server's application class loader, which cannot see the plugin's jars, so serializing the firstbooleanproperty fails withNoClassDefFoundError: com/fasterxml/jackson/module/blackbird/ser/ToBooleanFunctionand noCOMPLETEevent is emitted.This PR:
Jdk8ModuleandJavaTimeModuleexplicitly instead of discovering modules throughServiceLoader, and declares the two dependencies. They were the only functional modules discovered before, so the emittedpresto_query_statisticsfacet is unchanged;TestOpenLineageEventListenerPluginIsolation, a unit test that rebuilds the server's plugin / SPI class loader split in-process (same delegation rules asPluginClassLoader) and runsgetCompletedEventthrough it. On master it fails with the exact error from the issue; it runs with the regular module tests, so this class of bug no longer needs a packaged server to show up;TestPrestoContainerOpenLineageEventListenerto the existing container tests inpresto-native-execution. It usesContainerQueryRunnerin Java-cluster mode with the listener enabled on the coordinator (newConfig.setEventListenerProperties), posts events over HTTP to a small sink container on the cluster network, runs a query, and asserts that theCOMPLETEevent with thepresto_query_statisticsfacet arrives and that the coordinator log has noNoClassDefFoundError. It is picked up by the existingprestocpp-linux-container-testsworkflow, whose path filter now also includes the plugin module.Motivation and Context
Fixes #28460. The existing unit tests run everything from one flat class path where Blackbird is visible, so the failure only appeared on a real server.
Impact
Bug fix, no user-facing configuration change. The content of the
presto_query_statisticsfacet is unchanged.Test Plan
TestOpenLineageEventListenerPluginIsolationfails on master withjava.lang.InternalError: java.lang.NoClassDefFoundError: com/fasterxml/jackson/module/blackbird/ser/ToBooleanFunctionand passes with the fix../mvnw install -pl presto-openlineage-event-listener(all checks and tests) passes; the isolation test also passes when run from the reactor withpresto-spiandpresto-commonas class directories.TestPrestoContainerOpenLineageEventListenerruns in theprestocpp-linux-container-testsworkflow on this PR.Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
🤖 Generated with Claude Code
Summary by Sourcery
Fix OpenLineage query completion event serialization across Presto plugin class-loader boundaries.
Bug Fixes:
Enhancements:
Build:
CI:
Tests: