Conversation
unleashed
left a comment
unleashed
left a comment
There was a problem hiding this comment.
Looks good, but check the comments out for any optional stuff you might want to consider.
| request_types = [ | ||
| [ | ||
| 'alerts', | ||
| Proc.new { get_alerts_internal_api(service_id) } |
There was a problem hiding this comment.
You can use proc as a shorthand, or lambda if checking that the arity is right and the body does not do awesome but confusing things like return on behalf of the caller.
| ] | ||
| ] | ||
|
|
||
| request_types.each do |(request_type, proc)| |
There was a problem hiding this comment.
Maybe call the 2nd parameter blk to be more idiomatic and avoid confusion with proc from Kernel#proc.
|
|
||
| request_types.each do |(request_type, proc)| | ||
| n_calls = rand(1..5) | ||
| n_calls.times { proc.call } |
| def do_internal_api_get_req(path) | ||
| do_get_req("/internal#{path}") | ||
| end | ||
|
|
||
| def do_internal_api_delete_req(path) | ||
| do_delete_req("/internal#{path}") | ||
| end | ||
|
|
||
| def get_service_internal_api(service_id) | ||
| do_internal_api_get_req("/services/#{service_id}") | ||
| end | ||
|
|
||
| def get_alerts_internal_api(service_id) | ||
| do_internal_api_get_req("/services/#{service_id}/alert_limits/") | ||
| end | ||
|
|
||
| def get_app_keys_internal_api(service_id, app_id) | ||
| do_internal_api_get_req("/services/#{service_id}/applications/#{app_id}/keys/") | ||
| end | ||
|
|
||
| def get_app_referrer_filters_internal_api(service_id, app_id) | ||
| do_internal_api_get_req("/services/#{service_id}/applications/#{app_id}/referrer_filters") | ||
| end | ||
|
|
||
| def get_app_internal_api(service_id, app_id) | ||
| do_internal_api_get_req("/services/#{service_id}/applications/#{app_id}") | ||
| end | ||
|
|
||
| def get_errors_internal_api(service_id) | ||
| do_internal_api_get_req("/services/#{service_id}/errors/") | ||
| end | ||
|
|
||
| def get_events_internal_api | ||
| do_internal_api_get_req("/events/") | ||
| end | ||
|
|
||
| def get_metric_internal_api(service_id, metric_id) | ||
| do_internal_api_get_req("/services/#{service_id}/metrics/#{metric_id}") | ||
| end | ||
|
|
||
| def get_service_token_internal_api(token, service_id) | ||
| do_internal_api_get_req("/service_tokens/#{token}/#{service_id}/provider_key") | ||
| end | ||
|
|
||
| def delete_stats_internal_api(service_id) | ||
| do_internal_api_delete_req("/services/#{service_id}/stats") | ||
| end | ||
|
|
||
| def get_usage_limits_internal_api(service_id, plan_id, metric_id, period) | ||
| do_internal_api_get_req("/services/#{service_id}/plans/#{plan_id}/usagelimits/" + | ||
| "#{metric_id}/#{period}") | ||
| end | ||
|
|
||
| def get_utilization_internal_api(service_id, app_id) | ||
| do_internal_api_get_req("/services/#{service_id}/applications/#{app_id}/utilization/") |
There was a problem hiding this comment.
What a magnificient opportunity to generate these programmatically by iterating a hashmap with method names and paths with placeholders for arguments. :D
... but now it's done and we wouldn't gain anything from it at this point, so 👍.
| # Only the first match is taken into account, that's why for example, | ||
| # "/\/services\/.*\/stats/" needs to appear before "/\/services/" |
There was a problem hiding this comment.
We could write some code to ensure this gets sorted properly regardless of people ignoring this comment and breaking it. Probably not worth the effort, though... unless you start automating the information about these endpoints and generate also their associated methods for building paths usable in tests, etc.
There was a problem hiding this comment.
I thought about that but I don't think it's worth the effort. Also, now we have tests that should fail if this is not sorted.
| [/\/services\/.*\/stats/, 'stats'], | ||
| [/\/services\/.*\/plans\/.*\/usagelimits/, 'usage_limits'], | ||
| [/\/services\/.*\/applications\/.*\/utilization/, 'utilization'], | ||
| [/\/services/, 'services'], |
There was a problem hiding this comment.
Not freezing the strings?
davidor
force-pushed
the
fix-internal-api-metrics
branch
from
1ef653f to
4ba4ef0
Compare
2 participants
Fixes THREESCALE-6453
Some metrics of the internal API were not being counted correctly. The reason is that the array of regexes that we use to determine the counter to update was not sorted properly.
This issue was not caught by our test suite, so I added tests for all the requests types.