Skip to content

Synchronize SPI Span Decorator Loading #1462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
fatroom merged 2 commits into from
Jun 2, 2023
Merged

Synchronize SPI Span Decorator Loading #1462

fatroom merged 2 commits into from
Jun 2, 2023

Conversation

@lukasniemeier-zalando
Copy link
Member

@lukasniemeier-zalando lukasniemeier-zalando commented Jun 2, 2023

Synchronize SPI span decorator loading and have them loaded eagerly.

Motivation and Context

  1. eagerly load decorators obtained via SPI to avoid exceptions in high-concurrency setups
  2. instances of ServiceLoader are not thread-safe, iterating on the Iterable may yield exceptions if done concurrently
  3. eager loading prevents any reloading functionality of ServiceLoader to work

Example exception (sadly ServiceLoaderSpanDecorator not rendered):

java.lang.ArrayIndexOutOfBoundsException: Index 2 out of bounds for length 2
	at java.lang.CompoundEnumeration.next(ClassLoader.java:2725)
	at java.lang.CompoundEnumeration.hasMoreElements(ClassLoader.java:2734)
	... 30 frames excluded
	at org.zalando.fauxpas.ThrowingSupplier.get(ThrowingSupplier.java:19)
	at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768)
	at io.opentracing.contrib.concurrent.TracedRunnable.run(TracedRunnable.java:30)
	... 6 frames excluded
	at org.zalando.fauxpas.ThrowingFunction.apply(ThrowingFunction.java:19)
	... 5 frames excluded

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have added tests to cover my changes.

@lukasniemeier-zalando
Copy link
Member Author

lukasniemeier-zalando commented Jun 2, 2023

Does this have to be considered a Breaking Change due to point 3 of above?

@fatroom
Copy link
Member

fatroom commented Jun 2, 2023

@lukasniemeier-zalando can you sign your commit, please? Additionally can you add the note about this change to MIGRATION.md ?

@lukasniemeier-zalando
Synchronize SPI decorator loading
ed95179 
* eagerly load decorators obtained via SPI to avoid exceptions in
high-concurrency setups
* instances of `ServiceLoader` are not thread-safe, iterating on the
`Iterable` may yield exceptions if done concurrently
* eager loading prevents any reloading functionality of `ServiceLoader`
to work (acceptable)
@lukasniemeier-zalando
Copy link
Member Author

lukasniemeier-zalando commented Jun 2, 2023

@fatroom ok done ✔️

@lukasniemeier-zalando
Copy link
Member Author

lukasniemeier-zalando commented Jun 2, 2023 

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  04:05 min
[INFO] Finished at: 2023-06-02T11:32:48Z
[INFO] ------------------------------------------------------------------------
Error:  Failed to execute goal org.owasp:dependency-check-maven:8.2.1:check (default) on project riptide-core: 
Error:  
Error:  One or more dependencies were identified with vulnerabilities: 
Error:  
Error:  guava-32.0.0-jre.jar: CVE-2023-2976(6.2)

@fatroom
Copy link
Member

fatroom commented Jun 2, 2023

👍

@fatroom
Copy link
Member

fatroom commented Jun 2, 2023

Thank you for the contribution!

@fatroom fatroom merged commit 04a30f2 into main Jun 2, 2023
@fatroom fatroom deleted the spi-synchronization branch June 2, 2023 16:41
@lukasniemeier-zalando lukasniemeier-zalando mentioned this pull request Aug 14, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@whiskeysierra whiskeysierra Awaiting requested review from whiskeysierra

@fatroom fatroom Awaiting requested review from fatroom fatroom is a code owner

@Semernitskaya Semernitskaya Awaiting requested review from Semernitskaya

@dingxiangfei2009 dingxiangfei2009 Awaiting requested review from dingxiangfei2009

@Joyfolk Joyfolk Awaiting requested review from Joyfolk Joyfolk is a code owner

@gpradeepkrishna gpradeepkrishna Awaiting requested review from gpradeepkrishna gpradeepkrishna is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lukasniemeier-zalando @fatroom