aboutsummaryrefslogtreecommitdiffhomepage
path: root/test
diff options
context:
space:
mode:
authorGravatar Sree Kuchibhotla <sreek@google.com>2015-11-16 11:52:54 -0800
committerGravatar Sree Kuchibhotla <sreek@google.com>2015-11-16 14:41:04 -0800
commitb047c0fc9b455e6cbfd1d6b080e9bb427282b670 (patch)
tree6c8bd8598bfe51607fda1f7c92e5e04ee9de096b /test
parentc36b480ce9354daaaf829de88a0a04e76873df7a (diff)
Address code review comments
Diffstat (limited to 'test')
-rw-r--r--test/cpp/interop/metrics_client.cc12
-rw-r--r--test/cpp/interop/stress_interop_client.h2
-rw-r--r--test/cpp/interop/stress_test.cc26
-rw-r--r--test/cpp/util/metrics_server.cc12
-rw-r--r--test/cpp/util/metrics_server.h9
-rw-r--r--test/proto/metrics.proto6
6 files changed, 35 insertions, 32 deletions
diff --git a/test/cpp/interop/metrics_client.cc b/test/cpp/interop/metrics_client.cc
index a1def7299a..d68bf6d667 100644
--- a/test/cpp/interop/metrics_client.cc
+++ b/test/cpp/interop/metrics_client.cc
@@ -67,9 +67,13 @@ void PrintMetrics(grpc::string& server_address) {
long overall_qps = 0;
int idx = 0;
while (reader->Read(&gauge_response)) {
- gpr_log(GPR_INFO, "Gauge: %d (%s: %ld)", ++idx,
- gauge_response.name().c_str(), gauge_response.value());
- overall_qps += gauge_response.value();
+ if (gauge_response.value_case() == GaugeResponse::kLongValue) {
+ gpr_log(GPR_INFO, "Gauge: %d (%s: %ld)", ++idx,
+ gauge_response.name().c_str(), gauge_response.long_value());
+ overall_qps += gauge_response.long_value();
+ } else {
+ gpr_log(GPR_INFO, "Gauge %s is not a long value", gauge_response.name().c_str());
+ }
}
gpr_log(GPR_INFO, "OVERALL: %ld", overall_qps);
@@ -84,7 +88,7 @@ int main(int argc, char** argv) {
grpc::testing::InitTest(&argc, &argv, true);
// Make sure server_addresses flag is not empty
- if (FLAGS_metrics_server_address.length() == 0) {
+ if (FLAGS_metrics_server_address.empty()) {
gpr_log(
GPR_ERROR,
"Cannot connect to the Metrics server. Please pass the address of the"
diff --git a/test/cpp/interop/stress_interop_client.h b/test/cpp/interop/stress_interop_client.h
index 567c1b0a6d..6fd303d6b7 100644
--- a/test/cpp/interop/stress_interop_client.h
+++ b/test/cpp/interop/stress_interop_client.h
@@ -90,7 +90,7 @@ class StressTestInteropClient {
long test_duration_secs, long sleep_duration_ms,
long metrics_collection_interval_secs);
- // The main funciton. Use this as the thread entry point.
+ // The main function. Use this as the thread entry point.
// qps_gauge is the Gauge to record the requests per second metric
void MainLoop(std::shared_ptr<Gauge> qps_gauge);
diff --git a/test/cpp/interop/stress_test.cc b/test/cpp/interop/stress_test.cc
index fffa3dd3de..2999d6aae2 100644
--- a/test/cpp/interop/stress_test.cc
+++ b/test/cpp/interop/stress_test.cc
@@ -41,6 +41,7 @@
#include <grpc/support/time.h>
#include <grpc++/create_channel.h>
#include <grpc++/grpc++.h>
+#include <grpc++/impl/thd.h>
#include "test/cpp/interop/interop_client.h"
#include "test/cpp/interop/stress_interop_client.h"
@@ -91,11 +92,6 @@ DEFINE_string(test_cases, "",
" 'large_unary', 10% of the time and 'empty_stream' the remaining"
" 70% of the time");
-using std::make_pair;
-using std::pair;
-using std::thread;
-using std::vector;
-
using grpc::testing::kTestCaseList;
using grpc::testing::MetricsService;
using grpc::testing::MetricsServiceImpl;
@@ -119,7 +115,7 @@ TestCaseType GetTestTypeFromName(const grpc::string& test_name) {
// Converts a string of comma delimited tokens to a vector of tokens
bool ParseCommaDelimitedString(const grpc::string& comma_delimited_str,
- vector<grpc::string>& tokens) {
+ std::vector<grpc::string>& tokens) {
size_t bpos = 0;
size_t epos = grpc::string::npos;
@@ -137,10 +133,10 @@ bool ParseCommaDelimitedString(const grpc::string& comma_delimited_str,
// - Whether parsing was successful (return value)
// - Vector of (test_type_enum, weight) pairs returned via 'tests' parameter
bool ParseTestCasesString(const grpc::string& test_cases,
- vector<pair<TestCaseType, int>>& tests) {
+ std::vector<std::pair<TestCaseType, int>>& tests) {
bool is_success = true;
- vector<grpc::string> tokens;
+ std::vector<grpc::string> tokens;
ParseCommaDelimitedString(test_cases, tokens);
for (auto it = tokens.begin(); it != tokens.end(); it++) {
@@ -168,8 +164,8 @@ bool ParseTestCasesString(const grpc::string& test_cases,
}
// For debugging purposes
-void LogParameterInfo(const vector<grpc::string>& addresses,
- const vector<pair<TestCaseType, int>>& tests) {
+void LogParameterInfo(const std::vector<grpc::string>& addresses,
+ const std::vector<std::pair<TestCaseType, int>>& tests) {
gpr_log(GPR_INFO, "server_addresses: %s", FLAGS_server_addresses.c_str());
gpr_log(GPR_INFO, "test_cases : %s", FLAGS_test_cases.c_str());
gpr_log(GPR_INFO, "sleep_duration_ms: %d", FLAGS_sleep_duration_ms);
@@ -195,7 +191,7 @@ int main(int argc, char** argv) {
srand(time(NULL));
// Parse the server addresses
- vector<grpc::string> server_addresses;
+ std::vector<grpc::string> server_addresses;
ParseCommaDelimitedString(FLAGS_server_addresses, server_addresses);
// Parse test cases and weights
@@ -204,7 +200,7 @@ int main(int argc, char** argv) {
return 1;
}
- vector<pair<TestCaseType, int>> tests;
+ std::vector<std::pair<TestCaseType, int>> tests;
if (!ParseTestCasesString(FLAGS_test_cases, tests)) {
gpr_log(GPR_ERROR, "Error in parsing test cases string %s ",
FLAGS_test_cases.c_str());
@@ -218,7 +214,7 @@ int main(int argc, char** argv) {
gpr_log(GPR_INFO, "Starting test(s)..");
- vector<thread> test_threads;
+ std::vector<grpc::thread> test_threads;
int thread_idx = 0;
for (auto it = server_addresses.begin(); it != server_addresses.end(); it++) {
@@ -239,8 +235,8 @@ int main(int argc, char** argv) {
grpc::string metricName =
"/stress_test/qps/thread/" + std::to_string(thread_idx);
test_threads.emplace_back(
- thread(&StressTestInteropClient::MainLoop, client,
- metrics_service.CreateGauge(metricName, is_already_created)));
+ grpc::thread(&StressTestInteropClient::MainLoop, client,
+ metrics_service.CreateGauge(metricName, &is_already_created)));
// The Gauge should not have been already created
GPR_ASSERT(!is_already_created);
diff --git a/test/cpp/util/metrics_server.cc b/test/cpp/util/metrics_server.cc
index eac29f3093..5bb87e53f1 100644
--- a/test/cpp/util/metrics_server.cc
+++ b/test/cpp/util/metrics_server.cc
@@ -61,8 +61,8 @@ grpc::Status MetricsServiceImpl::GetAllGauges(
std::lock_guard<std::mutex> lock(mu_);
for (auto it = gauges_.begin(); it != gauges_.end(); it++) {
GaugeResponse resp;
- resp.set_name(it->first); // Gauge name
- resp.set_value(it->second->Get()); // Gauge value
+ resp.set_name(it->first); // Gauge name
+ resp.set_long_value(it->second->Get()); // Gauge value
writer->Write(resp);
}
@@ -77,14 +77,14 @@ grpc::Status MetricsServiceImpl::GetGauge(ServerContext* context,
auto it = gauges_.find(request->name());
if (it != gauges_.end()) {
response->set_name(it->first);
- response->set_value(it->second->Get());
+ response->set_long_value(it->second->Get());
}
return Status::OK;
}
-std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(string name,
- bool& already_present) {
+std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(const grpc::string& name,
+ bool* already_present) {
std::lock_guard<std::mutex> lock(mu_);
std::shared_ptr<Gauge> gauge(new Gauge(0));
@@ -92,7 +92,7 @@ std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(string name,
// p.first is an iterator pointing to <name, shared_ptr<Gauge>> pair. p.second
// is a boolean indicating if the Gauge is already present in the map
- already_present = !p.second;
+ *already_present = !p.second;
return p.first->second;
}
diff --git a/test/cpp/util/metrics_server.h b/test/cpp/util/metrics_server.h
index 19c3a0c32e..36529095f9 100644
--- a/test/cpp/util/metrics_server.h
+++ b/test/cpp/util/metrics_server.h
@@ -36,7 +36,6 @@
#include <atomic>
#include <map>
#include <mutex>
-#include <vector>
#include "test/proto/metrics.grpc.pb.h"
#include "test/proto/metrics.pb.h"
@@ -61,9 +60,8 @@
namespace grpc {
namespace testing {
-using std::map;
-using std::vector;
-
+// TODO(sreek): Add support for other types of Gauges like Double, String in
+// future
class Gauge {
public:
Gauge(long initial_val);
@@ -86,7 +84,8 @@ class MetricsServiceImpl GRPC_FINAL : public MetricsService::Service {
// is already present in the map.
// NOTE: CreateGauge can be called anytime (i.e before or after calling
// StartServer).
- std::shared_ptr<Gauge> CreateGauge(string name, bool& is_present);
+ std::shared_ptr<Gauge> CreateGauge(const grpc::string& name,
+ bool* already_present);
std::unique_ptr<grpc::Server> StartServer(int port);
diff --git a/test/proto/metrics.proto b/test/proto/metrics.proto
index f434c95390..14740eb65d 100644
--- a/test/proto/metrics.proto
+++ b/test/proto/metrics.proto
@@ -36,7 +36,11 @@ package grpc.testing;
message GaugeResponse {
string name = 1;
- int64 value = 2;
+ oneof value {
+ int64 long_value = 2;
+ double double_vale = 3;
+ string string_value = 4;
+ }
}
message GaugeRequest {