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

Auto-instrumentation when user's autoloader throws exception on not found #305

Merged
merged 13 commits into from
Feb 18, 2019

Conversation

labbati
Copy link
Member

@labbati labbati commented Feb 18, 2019

Description

This PR brings improvements to our auto-instrumentation and class loading sub-system.

Specifically, we could identify three major flaws:

  • if a user's registered autoloader is not lenient, i.e. it is not designed for chaining because it throws an exception when it does not find the file in its own search paths, we caused an exception when DDTrace\Tracer class reference was tested for existence.
  • Auto-instrumentation was 'indirectly' tested in tens of tests, but we lacked a specific suite that tested the different use cases for integration with users' autoloaders.
  • If a user registered a dependency in the composer.json file we would have picked it in place of the version bundled with the extension, but this was the result of fortunate circumstances instead of an explicit workflow in the code.

This PR fixes all the above flaws, in addition to add a bunch of comments that explain why specific choices were made.

Readiness checklist

@labbati labbati added 🐛 bug Something isn't working 🍏 core Changes to the core tracing functionality labels Feb 18, 2019
@labbati labbati added this to the 0.13.2 milestone Feb 18, 2019
@labbati labbati requested review from pawelchcki and SammyK February 18, 2019 08:31
@labbati labbati merged commit 8d1e947 into master Feb 18, 2019
@labbati labbati deleted the labbati/better-auto-instrumentation branch February 18, 2019 15:31
estringana pushed a commit that referenced this pull request Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🍏 core Changes to the core tracing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants