Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Use SenderFactory classloader for ServiceLoader #523

Merged
merged 3 commits into from
Aug 21, 2018
Merged

Use SenderFactory classloader for ServiceLoader #523

merged 3 commits into from
Aug 21, 2018

Conversation

dbuchhorn
Copy link
Contributor

@dbuchhorn dbuchhorn commented Aug 20, 2018

use ClassLoader of class SenderFactory to instantiate the ServiceLoader to prevent a ServiceConfigurationError (java.util.ServiceConfigurationError: io.jaegertracing.spi.SenderFactory: Provider io.jaegertracing.thrift.internal.senders.ThriftSenderFactory not a subtype)

Fixes #520

Signed-off-by: Dirk Buchhorn dirkbuchhorn@web.de

use ClassLoader of class SenderFactory to instantiate the ServiceLoader to prevent a ServiceConfigurationError (java.util.ServiceConfigurationError: io.jaegertracing.spi.SenderFactory: Provider io.jaegertracing.thrift.internal.senders.ThriftSenderFactory not a subtype)

Signed-off-by: Dirk Buchhorn <dirkbuchhorn@web.de>
@jpkrohling
Copy link
Collaborator

Looks like the failure is on the same line, but different reason:

https://travis-ci.org/jaegertracing/jaeger-client-java/jobs/418205475#L1469

[ant:checkstyle] [ERROR] /home/travis/build/jaegertracing/jaeger-client-java/jaeger-core/src/main/java/io/jaegertracing/internal/senders/SenderResolver.java:55: 'SenderFactory' have incorrect indentation level 6, expected level should be 8. [Indentation]

You should also be able to reproduce the problem by running ./gradlew clean check. Let me know if you have problems running this command.

Signed-off-by: Dirk Buchhorn <dirkbuchhorn@web.de>
@dbuchhorn
Copy link
Contributor Author

The problem was we do the gradle build in the root directory. This build reports only errors from the jaeger-thrift project. After run the build in the jaeger-core project the error was reported. We never used gradle before.

@codecov
Copy link

codecov bot commented Aug 21, 2018

Codecov Report

Merging #523 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #523      +/-   ##
============================================
+ Coverage     88.26%   88.27%   +<.01%     
  Complexity      500      500              
============================================
  Files            65       65              
  Lines          1849     1850       +1     
  Branches        239      239              
============================================
+ Hits           1632     1633       +1     
  Misses          140      140              
  Partials         77       77
Impacted Files Coverage Δ Complexity Δ
...jaegertracing/internal/senders/SenderResolver.java 96.77% <100%> (+0.1%) 9 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27bd0e0...541681d. Read the comment docs.

@jpkrohling
Copy link
Collaborator

We never used gradle before.

I'm glad this PR introduced you to new tools!

Copy link
Collaborator

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. This LGTM!

@ghost ghost assigned jpkrohling Aug 21, 2018
@ghost ghost added the review label Aug 21, 2018
@jpkrohling jpkrohling removed their assignment Aug 21, 2018
@jpkrohling jpkrohling changed the title Issue #520 ClassLoader problem Use SenderFactory classloader for ServiceLoader Aug 21, 2018
@jpkrohling jpkrohling merged commit 4934922 into jaegertracing:master Aug 21, 2018
@ghost ghost removed the review label Aug 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants