Skip to content
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.

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

Add .loadIntegrations() method to builder #80

Closed
wants to merge 1 commit into from

Conversation

lhotari
Copy link
Contributor

@lhotari lhotari commented Feb 10, 2020

  • makes it easier to customize BlockHound

- makes it easier to customize BlockHound
@bsideup
Copy link
Contributor

bsideup commented Feb 10, 2020

Hi @lhotari,

Could you please provide an example how the new API makes it easier compared to the existing ones

Thank you!

@lhotari
Copy link
Contributor Author

lhotari commented Feb 11, 2020

Could you please provide an example how the new API makes it easier compared to the existing ones

I'm sorry I didn't properly explain the context at first.

My use case is such that I'd like to use the builder and also get the integrations loaded. I believe that this would be useful in most cases when customizing the behavior of BlockHound.

I had the problem that some tests were hanging after enabling BlockHound for all tests. Since the exception was swallowed in one particular case, I followed the tip in https://github.com/reactor/BlockHound/blob/master/docs/tips.md#debugging to add a blockingMethodCallback that prints out the stacktrace:

    static {
        BlockHound.Builder builder = BlockHound.builder()
                .blockingMethodCallback(method -> {
                    Error error = new BlockingOperationError(method);
                    // Print out the blocking call to find out problem when exceptions are swallowed
                    error.printStackTrace(System.err);
                    throw error;
                });
        ServiceLoader<BlockHoundIntegration> serviceLoader = ServiceLoader.load(BlockHoundIntegration.class);
        StreamSupport.stream(serviceLoader.spliterator(), false).sorted().forEach(builder::with);
        builder.install();
    }

I had to look into the source code of BlockHound to find out how to get the integrations loaded with the ServiceLoader. After adding the .loadIntegrations to the builder, this type of use case would become simpler:

    static {
        BlockHound.builder()
                .blockingMethodCallback(method -> {
                    Error error = new BlockingOperationError(method);
                    // Print out the blocking call to find out problem when exceptions are swallowed
                    error.printStackTrace(System.err);
                    throw error;
                })
                .loadIntegrations()
                .install();
    }

@bsideup
Copy link
Contributor

bsideup commented Feb 11, 2020

@lhotari
No problem, thanks for providing a rich context!

My use case is such that I'd like to use the builder and also get the integrations loaded.

This sounds like the option number 2:

  1. BlockHound.install(BlockHoundIntegration... integrations) - same as BlockHound.install(), but adds user-provided integrations to the list.

So that you can do:

BlockHound.install(b -> {
    b. blockingMethodCallback(...);
});

WDYT about adding it to the tips instead? :)

@lhotari
Copy link
Contributor Author

lhotari commented Feb 11, 2020

BlockHound.install(b -> {
    b. blockingMethodCallback(...);
});

Thanks, now I got it. I missed that the existing API could be used that way.
I'll close the PR since it's not needed.

WDYT about adding it to the tips instead? :)

Good idea. I'll do that instead.

@lhotari lhotari closed this Feb 11, 2020
@lhotari
Copy link
Contributor Author

lhotari commented Feb 11, 2020

I created #81 to improve the example in tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants