Skip to content
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.

Sign up for GitHub

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

Extract quota config from service config. #101

Merged
merged 3 commits into from
Feb 17, 2017
Merged

Extract quota config from service config. #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfa5c44
Add metric_cost in config.
chowchow316 Feb 15, 2017
ad8cccf
Remove group rules.
chowchow316 Feb 16, 2017
66be352
Call loadQuotaConfig in config::create.
chowchow316 Feb 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions contrib/endpoints/src/api_manager/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,23 @@ MethodInfoImpl *Config::GetOrCreateMethodInfoImpl(const string &name,
return i->second.get();
}

bool Config::LoadQuotaRule(ApiManagerEnvInterface *env) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://cs.corp.google.com/piper///depot/google3/tech/internal/env/framework/wrappers/chemist/chemist_wrapper.cc?rcl=145718473&l=725

According to the source code

  • quota_metrics conversion from rules and metric_rules are different.
  • if there is rules, then metric_rules will be ignored.
  • cost field can be empty, it should be 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rules {
selector: "apiserving.servicecontrol.testing.v1.QuotaTest.User"
groups {
group: "ReadGroupUser"
cost: 3
}
}

will be converted to

quota_metrics: { # came from rules setting
name: "serviceruntime.googleapis.com/api/consumer/quota_used_count"
metric_values: {
Labels: {
key: "/quota_name"
value: "ReadGroupUser"
}
value: {
Int64_value: 3
}
}
}

metric_rules {
selector: "apiserving.servicecontrol.testing.v1.QuotaTest.User"
metric_costs {
key: "chemisttest.googleapis.com/operation/read_book"
value: 100
}
}

will be converted to

quota_metrics: { # came from metric_rules
name: "chemisttest.googleapis.com/operation/read_book"
metric_values: {
value: {
int64_value: 100
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add this check in a separate CL

for a selector, if there is a group_rule , don't use metric_rule

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your info, and let me make it clear: if there is rules, we do not care about anything in metric_rules, right? And we are always saving the quota info into a <metric, cost> pair, what should we use for the metric names for rules & metric_rules? It seems they have different structures.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to know the rule is came from rules or metric_rules. We can use the same data structure but additional information is required.

for (const auto &rule : service_.quota().metric_rules()) {
auto method = utils::FindOrNull(method_map_, rule.selector());
if (method) {
for (auto &metric_cost : rule.metric_costs()) {
(*method)->add_metric_cost(metric_cost.first, metric_cost.second);
}
} else {
env->LogError("Metric rule with selector " + rule.selector() +
"is mismatched.");
return false;
}
}

return true;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to call this function in ::Create() function. it can be the last one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

bool Config::LoadHttpMethods(ApiManagerEnvInterface *env,
PathMatcherBuilder *pmb) {
std::set<std::string> all_urls, urls_with_options;
Expand Down Expand Up @@ -443,6 +460,9 @@ std::unique_ptr<Config> Config::Create(ApiManagerEnvInterface *env,
if (!config->LoadBackends(env)) {
return nullptr;
}
if (!config->LoadQuotaRule(env)) {
return nullptr;
}
return config;
}

Expand Down
2 changes: 2 additions & 0 deletions contrib/endpoints/src/api_manager/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ class Config {
// Load SystemParameters info to MethodInfo.
bool LoadSystemParameters(ApiManagerEnvInterface *env);

bool LoadQuotaRule(ApiManagerEnvInterface *env);

// Gets the MethodInfoImpl creating it if necessary
MethodInfoImpl *GetOrCreateMethodInfoImpl(const std::string &name,
const std::string &api_name,
Expand Down
18 changes: 15 additions & 3 deletions contrib/endpoints/src/api_manager/method_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <map>
#include <memory>
#include <set>
#include <vector>

#include "contrib/endpoints/include/api_manager/method.h"
#include "contrib/endpoints/src/api_manager/utils/stl_util.h"
Expand Down Expand Up @@ -62,6 +63,10 @@ class MethodInfoImpl : public MethodInfo {

const std::string &backend_address() const { return backend_address_; }

const std::vector<std::pair<std::string, int>> &metric_cost_vector() const {
return metric_cost_vector_;
}

const std::string &rpc_method_full_name() const {
return rpc_method_full_name_;
}
Expand Down Expand Up @@ -90,6 +95,10 @@ class MethodInfoImpl : public MethodInfo {
url_query_parameters_[name].push_back(url_query_parameter);
}

void add_metric_cost(const std::string &metric, int64_t cost) {
metric_cost_vector_.push_back(std::make_pair(metric, cost));
}

// After add all system parameters, lookup some of them to cache
// their lookup results.
void process_system_parameters();
Expand Down Expand Up @@ -139,13 +148,13 @@ class MethodInfoImpl : public MethodInfo {
// such as API Key)?
bool allow_unregistered_calls_;
// Issuers to allowed audiences map.
std::map<std::string, std::set<std::string> > issuer_audiences_map_;
std::map<std::string, std::set<std::string>> issuer_audiences_map_;

// system parameter map of parameter name to http_header name.
std::map<std::string, std::vector<std::string> > http_header_parameters_;
std::map<std::string, std::vector<std::string>> http_header_parameters_;

// system parameter map of parameter name to url query parameter name.
std::map<std::string, std::vector<std::string> > url_query_parameters_;
std::map<std::string, std::vector<std::string>> url_query_parameters_;

// all the names of system query parameters
std::set<std::string> system_query_parameter_names_;
Expand Down Expand Up @@ -175,6 +184,9 @@ class MethodInfoImpl : public MethodInfo {

// Whether the response is streaming or not.
bool response_streaming_;

// map of metric and its cost
std::vector<std::pair<std::string, int>> metric_cost_vector_;
};

typedef std::unique_ptr<MethodInfoImpl> MethodInfoImplPtr;
Expand Down