Prepend instrumentation #565
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
PR review checklist:
|
tannalynn
force-pushed
the
prepend_instrumentation
branch
from
7d1f2cd to
2c8debb
Compare
This was
linked to
issues
Feb 8, 2021
This was referenced Feb 12, 2021
added 12 commits
February 12, 2021 18:17
…ager; support new config options
…rphan config tests
…to dependency logic
This was referenced Feb 22, 2021
due to jruby incompatibility with newer i18n
Deprecate old gems redux
* default prepend to true * add prepend for bunny * fox for bunny prepend * changed bunny prepend to module * updated bunny testing for chain and prepend * updated bunny * update bunny instrumentation * fix require_relative * reset default for config not related to bunny * added config check for bunny * DRYed up bunny instrumentation * fix leftover end * fix missing args * cleaned up and simplified Co-authored-by: Michael Lang <[email protected]>
prepend implemented for memcache
* set up chain for sinatra * update sinatra instrumentation * added prepend file * drying up prepend changes * most tests passing * got prepend working * updated sinatra tests * updated tested versions * fix for sintra prepend build * updated unit tests for sintra instrumentation * updated deferred_instrumentation suite for sinatra * WIP added padrino * fix for padrino test app * changes based on code review * untangled includes/extends and simplify for better readability * fixed weird unintialized constant error * fix for unit test failures Co-authored-by: Michael Lang <[email protected]>
tannalynn
commented
Apr 5, 2021
tannalynn
commented
Apr 5, 2021
This was referenced Apr 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
The agent instruments many libraries using the method chaining (a.k.a. method aliasing) approach to monkey-patching. This is a form of monkey-patching was deprecated with Ruby's introduction of
:prependin Ruby version 2.0.0 as a means to insert additional functionality into a module's method lookup mechanism ahead of the library's original method implementation.Prepend strategy will be the default choice in 7.0+. However, the prepend strategy may be reverted back to method chaining by adding the following (sidekiq example) configuration flag:
This PR collects all auto-instrumentation prepend implementation efforts and should be merged when all related work is completed and merged to this PR's branch. Each individual instrumentation issue should have its own PR against this branch.
To disable instrumentation altogether:
In some cases, we may know specific gems conflict with prepend. To facilitate, we offer an
autoconfig option (which is the default) which will automatically degrade tochainin such cases.The default setting in most cases is thus:
It is possible to force using
prependstrategy by specifying it in the config like so:If you find a conflict between Prepend and another gem in your application's
Gemfile, we would highly appreciate opening an Issue or commenting on this one specific to this gem so that we are aware and potentially work with gem authors to eliminate conflicts.Additional Context
We are switching to prepending by default and deprecating method-chaining as this is the most acceptable and dependable way to extend libraries with auto-instrumentation support outside of using the subscriber/notification hooks pattern that some gem authors already provide for observability purposes. However, this change is potentially a breaking change for some depending on other gems involved in an application's
Gemfilemakeup. To facilitate switching over to 7.0 release, the method chaining approach is only deprecated (not removed) and a configuration switch is provided to revert back to method-chaining for customers that are affected by prepend auto-instrumentation strategies.Related Github Issue
Include a link to the related GitHub issue, if applicable