Skip to content

Preserve ServiceName (segment name) when tracing external http services with dynamic urls #190

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

Closed

Conversation

shcherbin
Copy link

Issue #, if available:

Description of changes:

The changes prevent AWS X-RAY service from creating Cloud Watch metrics (ErrorRate, OkRate, ThrottleRate etc) for each unique URI when tracing a remote service with the dynamic URI segments (e.g. https://example.com/orders/{orderID} )

The traces influx with unique ServiceNames makes the X-RAY Service Map unusable and Cloud Watch daily charges extremely high.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@shcherbin shcherbin force-pushed the support-services-with-dynamic-urls branch from e24d3d7 to 2896bf0 Compare December 4, 2019 18:55
@codecov-io
Copy link

codecov-io commented Dec 4, 2019

Codecov Report

Merging #190 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
+ Coverage   83.33%   83.41%   +0.08%     
==========================================
  Files          77       77              
  Lines        2910     2913       +3     
==========================================
+ Hits         2425     2430       +5     
+ Misses        485      483       -2
Impacted Files Coverage Δ
aws_xray_sdk/ext/httplib/patch.py 75.78% <100%> (ø) ⬆️
aws_xray_sdk/ext/util.py 89.79% <100%> (+0.66%) ⬆️
aws_xray_sdk/ext/aiohttp/client.py 97.5% <100%> (ø) ⬆️
aws_xray_sdk/ext/requests/patch.py 100% <100%> (ø) ⬆️
aws_xray_sdk/core/sampling/local/reservoir.py 100% <0%> (+11.11%) ⬆️

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 324d185...2896bf0. Read the comment docs.

@shcherbin
Copy link
Author

Hi @chanchiem, I've noticed you were doing code reviews for other PRs. Could you please have a look at this one?

Thanks!

@chanchiem
Copy link
Contributor

chanchiem commented Dec 6, 2019

Thank you very much for the pull request! We really appreciate the contribution; however, we've added a push of it ourselves in favor of moving the strip_url call to the http.url field. We are also releasing the fix to version 2.4.3

We will be closing this in favor of PR #192

@chanchiem chanchiem closed this Dec 6, 2019
@shcherbin
Copy link
Author

Sure. Thanks

@shcherbin shcherbin deleted the support-services-with-dynamic-urls branch December 6, 2019 06:40
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.

3 participants