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

Use dynamic resolution of SQS to allow working without it. #2421

Merged
merged 3 commits into from Mar 2, 2021

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Feb 26, 2021

Using reflection is probably the worst possible solution but the simplest for now. Some ideas that come to mind for improvement.

  • Add an annotation that can suppress classes from muzzle, similar to animal sniffer IgnoreJreRequirement. We'd be expected to thoroughly code review suppressed classes
  • Have a system for defining classes that operate on plugins of the instrumented library.
interface PluginInstrumentation {
  boolean isEnabled();

  ... various methods
}

Muzzle would implement isEnabled to return true only if the references check out. And instrumentation would guard calls into "various methods" with isEnabled. Muzzle could probably also verify this guard is happening with some work.

Fixes #2408

Anuraag Agrawal added 2 commits March 2, 2021 13:20
@anuraaga anuraaga marked this pull request as ready for review March 2, 2021 05:19
@anuraaga
Copy link
Contributor Author

anuraaga commented Mar 2, 2021

Implemented dynamic dispatch as a stopgap - it'd be nice if 1.0 doesn't have broken aws sdk v1 instrumentation :)

@mateuszrzeszutek
Copy link
Member

I think that we could add a method annotation that both stops muzzle from analyzing it and wraps the method with try {...} catch (LinkageError) to make sure that the runtime won't blow up no matter what - we'd still need to make sure that there are appropriate ifs guarding the annotated code, catching an error should be the last resort.

The plugin approach would probably be really complicated to implement (and understand)

@trask trask merged commit 12baba1 into open-telemetry:main Mar 2, 2021
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.

AWS SDK V1 instrumentation doesn't apply if SQS isn't on the classpath
3 participants