JsonProvider : Support specifying the classloader used to resolve the JsonProvider or support specifying the JsonProvider implementation

[cyrille-leclerc]

The jakarta.json.spi.JsonProvider used by the Elasticsearch Java Client is resolved using the jakarta.json.spi.JsonProvider#provider() method which unfortunately relies on the java.util.ServiceLoader#load(java.lang.Class<S>) API without search in the classloader that loaded the Elasticsearch Java Client (java.util.ServiceLoader#load(java.lang.Class<S>, java.lang.ClassLoader)).
In runtimes like Jenkins, this fails with ClassNotFoundException: org.glassfish.json.JsonProviderImpl.

I would like the Elasticsearch Java client to either search the jakarta.json.spi.JsonProvider in the classloader that Elasticsearch Java Client classes or to support manually setting it.
I like very much the pattern chosen by OpenTelemetry with io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder#setServiceClassLoader()

https://github.com/open-telemetry/opentelemetry-java/blob/v1.11.0/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java#L283-L289

See

elasticsearch-java/java-client/src/main/java/co/elastic/clients/json/jackson/JsonValueParser.java

Line 39 in 74164d8

private final JsonProvider provider = JsonProvider.provider();

java.lang.ClassNotFoundException: org.glassfish.json.JsonProviderImpl
	at jenkins.util.AntClassLoader.findClassInComponents(AntClassLoader.java:1417)
	at jenkins.util.AntClassLoader.findClass(AntClassLoader.java:1372)
	at jenkins.util.AntClassLoader.loadClass(AntClassLoader.java:1127)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:315)
	at jakarta.json.spi.JsonProvider.provider(JsonProvider.java:72)
Caused: jakarta.json.JsonException: Provider org.glassfish.json.JsonProviderImpl not found
	at jakarta.json.spi.JsonProvider.provider(JsonProvider.java:75)
	at co.elastic.clients.json.jackson.JsonValueParser.<init>(JsonValueParser.java:39)
	at co.elastic.clients.json.jackson.JacksonJsonpParser.getValue(JacksonJsonpParser.java:233)
	at co.elastic.clients.json.JsonpDeserializerBase$8.deserialize(JsonpDeserializerBase.java:255)
	at co.elastic.clients.json.JsonpDeserializerBase$8.deserialize(JsonpDeserializerBase.java:249)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.transport.endpoints.EndpointWithResponseMapperAttr$1.deserialize(EndpointWithResponseMapperAttr.java:56)
	at co.elastic.clients.transport.rest_client.RestClientTransport.decodeResponse(RestClientTransport.java:328)
	at co.elastic.clients.transport.rest_client.RestClientTransport.getHighLevelResponse(RestClientTransport.java:294)
	at co.elastic.clients.transport.rest_client.RestClientTransport.performRequest(RestClientTransport.java:147)
	at co.elastic.clients.elasticsearch.ElasticsearchClient.search(ElasticsearchClient.java:1526)
        ...

[swallez]

java.util.ServiceLoader#load(java.lang.Class<S>) loads from the current thread's context classloader. A well-behaved Java application that creates classloaders to load plugins should set the context classloader before calling plugins.

It appears Jenkins made a deliberate choice of not following this rule (see "Context class loader" in the Jenkins plugin development docs and JENKINS-58130). So setting the context classloader in the entry points of your plugin (and restoring it when leaving) should solve the issue.

That being said, it's likely that Jenkins isn't the only misbehaving application and the workaround you suggest of falling back to the Elasticsearch client's classloader will certainly solve most issues. We'll add that. However I don't think the complexity added by allowing to set the classloader is needed as it will most always be that of the client. If ever another classloader needs to be set, users can still set the thread's context classloader.

Finally, we do value your suggestions and would appreciate that you avoid "I would like" and express them as what they are, suggestions that start a conversation. Thanks.

[timja]

Hi ya

Using the elastic search client class loader should work fine.

Do you have a reference for a well behaved application should set the context classloader? From my understanding it’s the other way around you should not rely on it at all. Ref: https://stackoverflow.com/a/36228195