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

Revert okhttp library instrumentation back to using standard reflection to support Android usage #3910

Merged
merged 2 commits into from
Aug 24, 2021

Conversation

jkwatson
Copy link
Contributor

It turns out that MethodHandles are not supported on Android API levels 21-25.

@jkwatson
Copy link
Contributor Author

Since this is a regression for Android, could we get a patch release with this commit, possibly?

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

Since this is a regression for Android, could we get a patch release with this commit, possibly?

yes, this definitely fits our criteria for patching

@@ -24,22 +23,19 @@
private static final Cache<Request, Context> contextsByRequest =
Cache.newBuilder().setWeakKeys().build();

@Nullable private static MethodHandle timeoutMethodHandle;
@Nullable private static MethodHandle cloneMethodHandle;
@Nullable private static Method timeoutMethod;
Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment here about not being able to use method handles (so no one changes it back), at least until we get some build tooling in place to prevent similar to otel-java repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@trask trask merged commit fa16826 into open-telemetry:main Aug 24, 2021
trask added a commit that referenced this pull request Aug 24, 2021
…on to support Android usage (#3910) (#3936)

* Revert back to using standard reflection to support Android usage

* Add a comment about not using MethodHandles

Co-authored-by: John Watson <jkwatson@gmail.com>
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.

4 participants