-
Notifications
You must be signed in to change notification settings - Fork 641
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
Add custom query telemetry and response header to OData endpoint #6583
Conversation
14d58ad
to
74fa74a
Compare
I've solved this case with an "Unknown" state. This is registered as "Unknown" in telemetry and no |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Nice test additions
74fa74a
to
989e55c
Compare
Tried this out on DEV environment. <rate-limit-by-key
calls="10"
renewal-period="60"
counter-key="@(context.Request.IpAddress)"
increment-condition="@(context.Response.Headers.GetValueOrDefault("X-NuGet-CustomQuery", "false") == "true")" /> Works great! |
I would like to ship this to PROD without the throttle rule and observe the telemetry. Any objections? |
7b2bf49
to
d331f5d
Compare
d331f5d
to
46bdc55
Compare
46bdc55
to
dc86fe8
Compare
Progress on https://github.com/NuGet/Engineering/issues/1798.
This PR does two things:
ODataCustomQuery
to OData queries.IsCustomQuery = True | False | Unknown
X-NuGet-CustomQuery: true | false
The telemetry will allow us to verify that official clients are not doing custom queries. Note that "custom query" does not necessarily mean executed without the database. I chose to make this distinction for the
GetUpdates
OData function. This can be called by official client (2.x for many cases, 3.x+ fornuget.exe update -self
) so it should not be throttled like custom OData queries.Another case: when search hijack throws an exception, some (maybe all?) codepaths fall back to DB. This should probably not be throttled...