pyramid_swagger
pyramid_swagger copied to clipboard
striglia
Add support for random response validation
Performing response validation is important to ensure that the provided responses are "good", but validating responses has impact on the overall response time. According to the profiling tests on bravado-core validating response is responsible for ~2/3 of the response unmarshaling time.
Name (time in ms) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
large_objects[not_validate-full-deref-100] 6.9844 (1.0) 26.9779 (1.0) 7.2673 (1.0) 1.7472 (4.23) 7.0800 (1.0) 0.0764 (1.0) 1;8 137.6017 (1.0) 131 1
large_objects[not_validate-with-refs-100] 10.7621 (1.54) 31.4821 (1.17) 11.2619 (1.55) 2.3074 (5.59) 10.9386 (1.55) 0.0955 (1.25) 2;7 88.7950 (0.65) 86 1
large_objects[validate-full-deref-100] 20.2593 (2.90) 40.8244 (1.51) 20.9659 (2.88) 2.9113 (7.05) 20.4715 (2.89) 0.2009 (2.63) 1;6 47.6965 (0.35) 49 1
large_objects[validate-with-refs-100] 28.0228 (4.01) 30.2325 (1.12) 28.4346 (3.91) 0.4131 (1.0) 28.3672 (4.01) 0.2481 (3.25) 2;2 35.1684 (0.26) 36 1
large_objects[not_validate-full-deref-1000] 71.5863 (10.25) 94.4015 (3.50) 75.5684 (10.40) 7.5661 (18.32) 72.4621 (10.23) 1.2367 (16.18) 2;3 13.2330 (0.10) 15 1
large_objects[not_validate-with-refs-1000] 109.7313 (15.71) 131.4665 (4.87) 113.2843 (15.59) 6.9370 (16.79) 110.8814 (15.66) 1.3490 (17.65) 2;2 8.8273 (0.06) 15 1
large_objects[validate-full-deref-1000] 201.3343 (28.83) 224.4196 (8.32) 206.3566 (28.40) 7.4360 (18.00) 203.7418 (28.78) 2.3413 (30.63) 2;2 4.8460 (0.04) 15 1
large_objects[validate-with-refs-1000] 280.9136 (40.22) 304.3246 (11.28) 284.2836 (39.12) 5.8347 (14.12) 282.3307 (39.88) 3.0268 (39.60) 1;1 3.5176 (0.03) 15 1
large_objects[not_validate-full-deref-5000] 366.9331 (52.54) 398.9429 (14.79) 383.4994 (52.77) 14.7368 (35.68) 391.4602 (55.29) 28.8145 (377.00) 9;0 2.6076 (0.02) 15 1
large_objects[not_validate-with-refs-5000] 556.1794 (79.63) 591.9109 (21.94) 573.9847 (78.98) 14.5869 (35.31) 581.6492 (82.15) 26.4046 (345.47) 6;0 1.7422 (0.01) 15 1
large_objects[validate-full-deref-5000] 1,021.2229 (146.22) 1,063.9244 (39.44) 1,038.9285 (142.96) 14.9543 (36.20) 1,044.3556 (147.51) 26.0122 (340.33) 5;0 0.9625 (0.01) 15 1
large_objects[validate-with-refs-5000] 1,411.4028 (202.08) 1,458.9073 (54.08) 1,433.4528 (197.25) 15.1330 (36.63) 1,433.0088 (202.40) 25.3398 (331.54) 4;0 0.6976 (0.01) 15 1
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Raw data in https://travis-ci.org/Yelp/bravado-core/jobs/536472510#L574 and https://termbin.com/6wyi
Currently pyramid-swagger does allow an "on/off" toggle for response validation. I'm wondering if we might have response validation performed on a percentage of traffic, such that we might have some confidence on the quality of the responses without impacting all the requests.
It would be nice to have a new configuration that might look like
{
'<pyramid_route_name>': route_validation_percentage, # value between 0 and 1
None: general_validation_percentage, # value between 0 and 1 # If the value is missing it is assumed to be 1
}
The idea is to have:
-
pyramid_swagger.enable_response_validation: enabling or disabling response validation - if response validation is enabled then
if:
- the new configuration map is defined then response validation will respect the defined percentages
- the new configuration map is not defined then all the responses are validated
Before moving on with PR-ing the idea I would like to have some input. (ie. Does the idea make sense? Do the configurations be eventually be easily understandable?)
cc: @sjaensch , @striglia
I like the idea! I'm not sure we need a per-route percentage configuration, I'd probably go with a global percentage value for response validation instead. You could even think of allowing that value for the existing enable_response_validation option, in addition to true/false.
Yep, I’d support the idea and probably also just do it globally.
We probably need a wave of deprecations and eventual removals of config options anyway (especially 1.2 support) so we can remove the binary true/false then.
On Mon, May 27, 2019 at 8:34 AM Stephan Jaensch [email protected] wrote:
I like the idea! I'm not sure we need a per-route percentage configuration, I'd probably go with a global percentage value for response validation instead. You could even think of allowing that value for the existing enable_response_validation option, in addition to true/false.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/striglia/pyramid_swagger/issues/240?email_source=notifications&email_token=AACADZTIGSBH7AMYSNUAEYDPXPIPBA5CNFSM4HPQRA7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWJWDQI#issuecomment-496198081, or mute the thread https://github.com/notifications/unsubscribe-auth/AACADZQXGMXDUNMDGKJQDD3PXPIPBANCNFSM4HPQRA7A .
And we may as well do this for both request and response validation right? Keep them separate but allow sampling between zero and all requests?
On Mon, May 27, 2019 at 9:10 AM Scott Triglia [email protected] wrote:
Yep, I’d support the idea and probably also just do it globally.
We probably need a wave of deprecations and eventual removals of config options anyway (especially 1.2 support) so we can remove the binary true/false then.
On Mon, May 27, 2019 at 8:34 AM Stephan Jaensch [email protected] wrote:
I like the idea! I'm not sure we need a per-route percentage configuration, I'd probably go with a global percentage value for response validation instead. You could even think of allowing that value for the existing enable_response_validation option, in addition to true/false.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/striglia/pyramid_swagger/issues/240?email_source=notifications&email_token=AACADZTIGSBH7AMYSNUAEYDPXPIPBA5CNFSM4HPQRA7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWJWDQI#issuecomment-496198081, or mute the thread https://github.com/notifications/unsubscribe-auth/AACADZQXGMXDUNMDGKJQDD3PXPIPBANCNFSM4HPQRA7A .
@striglia request validation, at least according to the current implementation, is needed in order to have request.swagger_data populated.
https://github.com/striglia/pyramid_swagger/blob/master/pyramid_swagger/tween.py#L194
I'm find with having the global percentage configuration.
The idea of having a per-route configuration was mainly driven by the fact that a global percentage has higher changes to not perform validation on a rarely used endpoint. An extreme example could be done with a service that defines 2 endpoints one of which gets a very little traffic share, let's say 0.1%.
But as mentioned I'm not against the idea of having a global percentage as it would not prevent us to have, eventually later, per-route specific percentage if we get enough feedback to justify the implementation.
Fair point -- maybe we think separately about request validation then. I think in general, response bodies tend to be far larger than request bodies, so there's probably more value in doing responses first anyway.
Definitely agree that starting global first, waiting to see how much of a
problem that is, then potentially expanding later makes sense to me. Come
to think of it, it'd be an interesting X- param in the spec itself.
On Mon, May 27, 2019 at 6:35 AM Samuele Maci [email protected] wrote:
@striglia https://github.com/striglia request validation, at least according to the current implementation, is needed in order to have request.swagger_data populated.
https://github.com/striglia/pyramid_swagger/blob/master/pyramid_swagger/tween.py#L194
I'm find with having the global percentage configuration.
The idea of having a per-route configuration was mainly driven by the fact that a global percentage has higher changes to not perform validation on a rarely used endpoint. An extreme example could be done with a service that defines 2 endpoints one of which gets a very little traffic share, let's say 0.1%.
But as mentioned I'm not against the idea of having a global percentage as it would not prevent us to have, eventually later, per-route specific percentage if we get enough feedback to justify the implementation.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/striglia/pyramid_swagger/issues/240?email_source=notifications&email_token=AACADZUHEHVEOQS42BETAWTPXPPR5A5CNFSM4HPQRA7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWJ2JQY#issuecomment-496215235, or mute the thread https://github.com/notifications/unsubscribe-auth/AACADZRYWBNAF6VUNGZ2RIDPXPPR5ANCNFSM4HPQRA7A .
Having information in the specs is possible, but generally speaking specs are "public" so we should make sure that the vendor extension is stripped out while serving /swagger.(json|yaml) endpoint.
Additionally having the information in the specs would lead to the impossibility to change the validation rate while the service is running, while we could do it with pyramid settings.
Let's agree for now on global first and then we'll decide if we want to expand the case.
About request validation, if we update the code we might decide to not validate the requests. The more philosophical question there is: "could we trust clients?". Not performing validation could lead to the service implementation believe that certain parameters/values are present while they might be missing and it would cause, in the best case, an unexpected exception (HTTP/5xx) while the request is just "bad (HTTP/4xx)