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

Test public API in app server integration test #361

Closed
wants to merge 1 commit into from

Conversation

felixbarny
Copy link
Member

This currently fails for WildFly, probably because of the bootdelegation

This currently fails for WildFly, probably because of the bootdelegation
@felixbarny
Copy link
Member Author

Two options to solve this:

  1. Add more specific packages than co.elastic.apm.* to the bootdelegation:

private static final String APM_BASE_PACKAGE = "co.elastic.apm.*";

Cons: this list would be quite long and would have to change as we introduce new packages. So that would be quite brittle.

  1. Change package structure of the non-api classes. For example to co.elastic.apm.agent or co.elastic.apm.internal and change the bootdelegation property accordingly so that co.elastic.apm.api is not matched anymore.

Cons: Almost all classes have to move but as they are internal, this is would not be a breaking change.

@eyalkoren
Copy link
Contributor

eyalkoren commented Dec 9, 2018

No need to merge this PR. Moved the test change to #376 so it will be included with the fix for it

@eyalkoren eyalkoren requested review from eyalkoren and removed request for eyalkoren December 9, 2018 10:17
@eyalkoren eyalkoren removed their assignment Dec 9, 2018
@eyalkoren eyalkoren removed their request for review December 9, 2018 10:18
@eyalkoren eyalkoren closed this Dec 9, 2018
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.

3 participants