Add ability to ignore some requests from httplib by jonathangreen · Pull Request #263 · aws/aws-xray-sdk-python

jonathangreen · 2020-12-17T01:49:59Z

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_ignored that allows requests to be ignored based on the hostname, url or subclass. 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.

codecov-io · 2020-12-17T02:21:09Z

Codecov Report

Merging #263 (ff94da3) into master (a16ba30) will increase coverage by 0.19%.
The diff coverage is 96.42%.

@@            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

Impacted Files	Coverage Δ
aws_xray_sdk/ext/httplib/patch.py	`81.66% <96.15%> (+5.87%)`	⬆️
aws_xray_sdk/ext/httplib/__init__.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a16ba30...ff94da3. Read the comment docs.

srprash · 2020-12-30T21:37:43Z

@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 add_ignored method for ignoring requests?

jonathangreen · 2021-01-04T17:23:02Z

@srprash Thanks for reviewing! I've added some documentation to the README with an example of using the add_ignored method.

willarmiros

Overall looks good, thanks for the contribution! Couple small comments.

willarmiros · 2021-01-04T18:16:25Z

+
+### 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. 


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?

@willarmiros updated to add both of those items.

willarmiros · 2021-01-04T18:18:47Z

+def _ignored_add_default():
+    # skip httplib tracing for SDK built-in centralized sampling pollers
+    add_ignored(subclass='botocore.awsrequest.AWSHTTPConnection', urls=['/GetSamplingRules', '/SamplingTargets'])


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?

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.

srprash · 2021-01-04T18:34:39Z

@jonathangreen httplib/http.client is a low-level module and most of the people would use requests or urllib for making HTTP calls. I think the urllib module uses the low-level http.client underneath but I'm not sure about requests. Do you think this ignore mechanism would work for someone using requests or urllib as well?

jonathangreen · 2021-01-04T20:05:35Z

@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.

willarmiros

LGTM. We should expand this mechanism to other libs like requests but doesn't need to be in this PR probably.

srprash · 2021-01-04T22:27:59Z

@jonathangreen
Yeah, requests patch should have its own handling for this mechanism.
The urllib module is automatically patched if the httplib is patched since urllib builds upon httplib/http.client. I did a quick test with the following piece of code and it works! :)

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()

srprash

Looks good. Thanks!

* 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

codecov-commenter · 2025-07-07T20:18:42Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

jonathangreen force-pushed the httplib_ignore branch from 66000c7 to c7141fa Compare December 17, 2020 03:44

jonathangreen added 5 commits December 17, 2020 13:23

Expand ability to ignore some httplib calls.

e5593f0

Add tests.

57898ce

Add glob match to httplib ignore hostname.

cf9f93d

Clean up httplib tests.

eceaae2

Use full module path for subclass.

f1b2989

jonathangreen force-pushed the httplib_ignore branch from c7141fa to f1b2989 Compare December 17, 2020 17:24

bhautikpip self-requested a review December 22, 2020 00:39

lupengamzn requested review from srprash and willarmiros December 29, 2020 00:03

jonathangreen added 2 commits January 4, 2021 13:20

Add documentation for ignoring httplib requests

7a93c1a

Merge branch 'master' into httplib_ignore

2efae24

willarmiros reviewed Jan 4, 2021

View reviewed changes

Code review feedback

ff94da3

willarmiros approved these changes Jan 4, 2021

View reviewed changes

srprash approved these changes Jan 4, 2021

View reviewed changes

srprash merged commit e121217 into aws:master Jan 4, 2021

srprash mentioned this pull request Feb 6, 2023

Custom emitter based on boto3 creates an infinite loop in the SDK #379

Open


		### 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.

Conversation

jonathangreen commented Dec 17, 2020

Uh oh!

codecov-io commented Dec 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

srprash commented Dec 30, 2020

Uh oh!

jonathangreen commented Jan 4, 2021

Uh oh!

willarmiros left a comment

Choose a reason for hiding this comment

Uh oh!

willarmiros Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

jonathangreen Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

willarmiros Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

srprash Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

srprash commented Jan 4, 2021

Uh oh!

jonathangreen commented Jan 4, 2021

Uh oh!

willarmiros left a comment

Choose a reason for hiding this comment

Uh oh!

srprash commented Jan 4, 2021

Uh oh!

srprash left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 7, 2025

Welcome to Codecov 🎉

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-io commented Dec 17, 2020 •

edited

Loading