Add ability to ignore some requests from httplib #263
Conversation
Codecov Report
@@ Coverage Diff @@
## master #263 +/- ##
==========================================
+ Coverage 79.18% 79.37% +0.19%
==========================================
Files 82 82
Lines 3223 3248 +25
==========================================
+ Hits 2552 2578 +26
+ Misses 671 670 -1
Continue to review full report at Codecov.
|
jonathangreen
force-pushed
the
httplib_ignore
branch
from
66000c7 to
c7141fa
Compare
jonathangreen
force-pushed
the
httplib_ignore
branch
from
c7141fa to
f1b2989
Compare
|
@jonathangreen Thanks for this contribution! I'll give it a quick try. Could you also add a section in the README with an example for using the |
|
@srprash Thanks for reviewing! I've added some documentation to the README with an example of using the |
README.md
Outdated
|
|
||
| ### Ignoring httplib requests | ||
|
|
||
| If you want to ignore certain httplib requests you can do so based on the hostname or URL that is being requsted. |
There was a problem hiding this comment.
Could you explicitly call out that this supports Unix-style regex or fnmatch regex (as opposed to re)? Also, can we add an example using subclass matching?
There was a problem hiding this comment.
@willarmiros updated to add both of those items.
| def _ignored_add_default(): | ||
| # skip httplib tracing for SDK built-in centralized sampling pollers | ||
| add_ignored(subclass='botocore.awsrequest.AWSHTTPConnection', urls=['/GetSamplingRules', '/SamplingTargets']) |
There was a problem hiding this comment.
I'm conflicted here - I know that using Python's built-in string matching as we are now will be faster than using fnmatch, but it would be awkward to not use this ignore mechanism and special-case /GetSamplingRules and /SamplingTargets.
I guess the added latency is kinda peanuts compared to the actual network request, so it's probably ok, but what do you think @srprash?
There was a problem hiding this comment.
I feel using fnmatch for ignoring /GetSamplingRules and /SamplingTarget is okay since it fits well in the overall mechanism to ignore any URL. I'm not really aware of the latency of fnmatch but my guess is that special casing the sampling urls and then matching the user urls would be roughly equivalent and won't cause much of a difference here.
|
@jonathangreen |
|
@srprash I don't believe requests calles httplib under the hood, so this method likely won't work there. It would be a nice future improvement though to add a similar method for ignoring calls that come from requests. |
|
@jonathangreen from aws_xray_sdk.core import patch
from aws_xray_sdk.ext.httplib import add_ignored
libraries = (['httplib'])
patch(libraries)
import urllib.request
add_ignored(hostname="www.amazon.com")
with urllib.request.urlopen('http://www.amazon.com') as response:
html = response.read() |
* Expand ability to ignore some httplib calls. * Add tests. * Add glob match to httplib ignore hostname. * Clean up httplib tests. * Use full module path for subclass. * Add documentation for ignoring httplib requests * Code review feedback
Description of changes:
This generalizes the test that the httplib patch was doing to ignore requests to xray being made by botocore. I am using cloudwatch logging in my application, and these calls occur outside of the normal application flow, so I don't have a segment open for them.
This adds a new function
add_ignoredthat allows requests to be ignored based on thehostname,urlorsubclass. It defaults to ignoring the same things as before, but now additional requests can be ignored.I also added some tests for the new functionality.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.