Allow configuring different Sampler in Django App #252
Conversation
Extended aws_xray_sdk.ext.django app to be able to select Sampler, and avoid being stuck with the DefaultSampler
|
|
||
| class XRayConfigurationTestCase(TestCase): | ||
| def test_sampler_can_be_configured(self): | ||
| assert isinstance(settings.XRAY_RECORDER['SAMPLER'], LocalSampler) |
There was a problem hiding this comment.
I would have loved to overwrite settings.XRAY_RECORDER['SAMPLER'] here, to show the recorder working with different samplers.
But because xray_recorder is configured in the ready method of the app, and the ready method is only called once when the test suite is run, that's not easy to implement (cleanly) currently. (you can read more about it here, or verify with a pdb.set_trace() on the ready method)
There was a problem hiding this comment.
I guess there is way to test different samplers by having different django settings and have it loaded using settings.configure(default_settings=settings_with_LocalSampler) and then calling django.setup() for each test case. More info here. But I agree with you that it may not be clean. I think this is good for now since it tests what we want at this point.
There was a problem hiding this comment.
Hi @lalvarezguillen
Thanks for filling in this gap for the SDK. I'll update the docs to reflect the new settings parameter.
|
|
||
| class XRayConfigurationTestCase(TestCase): | ||
| def test_sampler_can_be_configured(self): | ||
| assert isinstance(settings.XRAY_RECORDER['SAMPLER'], LocalSampler) |
There was a problem hiding this comment.
I guess there is way to test different samplers by having different django settings and have it loaded using settings.configure(default_settings=settings_with_LocalSampler) and then calling django.setup() for each test case. More info here. But I agree with you that it may not be clean. I think this is good for now since it tests what we want at this point.
Codecov Report
@@ Coverage Diff @@
## master #252 +/- ##
=======================================
Coverage 79.36% 79.36%
=======================================
Files 80 80
Lines 3159 3159
=======================================
Hits 2507 2507
Misses 652 652
Continue to review full report at Codecov.
|
Extended aws_xray_sdk.ext.django app to be able to select Sampler, and avoid being stuck with the DefaultSampler Co-authored-by: Prashant Srivastava <[email protected]>
This patch adapts
aws_xray_sdk.ext.djangoto be able to configureSamplertype.Because right now it uses
aws_xray_sdk.core.sampling.sampler.DefaultSampler, and doesn't allow configuring an alternative. Which means that it's impossible to use local sampling rules with DjangoBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.