From 9828f012d57be0813e4324537ceda6ccdff9acdb Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Wed, 11 Oct 2017 13:52:11 -0400 Subject: [PATCH 1/5] Add command-line options support. --- src/Makefile | 4 ++-- src/configuration.cc | 41 ++++++++++++++++++++++++++++++++++++++--- src/configuration.h | 9 ++++++++- src/metadatad.cc | 7 ++++++- 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/Makefile b/src/Makefile index c8677151..44cdcc88 100644 --- a/src/Makefile +++ b/src/Makefile @@ -8,8 +8,8 @@ SED_I=/usr/bin/env sed -i CMAKE=cmake CXXFLAGS=-std=c++11 -g -DENABLE_KUBERNETES_METADATA -I$(CPP_NETLIB_DIR) -I$(YAML_CPP_DIR)/include LDFLAGS=-L$(CPP_NETLIB_LIBDIR) -L$(YAML_CPP_LIBDIR) -LDLIBS=-lcppnetlib-uri -lcppnetlib-client-connections -lboost_system \ - -lboost_thread -lpthread -lyajl -lssl -lcrypto -lyaml-cpp +LDLIBS=-lcppnetlib-uri -lcppnetlib-client-connections -lboost_program_options \ + -lboost_system -lboost_thread -lpthread -lyajl -lssl -lcrypto -lyaml-cpp SED_EXTRA= UNAME_S=$(shell uname -s) diff --git a/src/configuration.cc b/src/configuration.cc index c1019c67..05918397 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -16,6 +16,7 @@ #include "configuration.h" +#include #include #include @@ -23,6 +24,7 @@ namespace google { namespace { +constexpr const char kConfigFileFlag[] = "config-file"; constexpr const char kDefaultProjectId[] = ""; constexpr const char kDefaultCredentialsFile[] = ""; @@ -51,6 +53,7 @@ constexpr const char kDefaultInstanceZone[] = ""; MetadataAgentConfiguration::MetadataAgentConfiguration() : project_id_(kDefaultProjectId), credentials_file_(kDefaultCredentialsFile), + verbose_logging_(false), metadata_api_num_threads_(kMetadataApiDefaultNumThreads), metadata_api_port_(kMetadataApiDefaultPort), metadata_reporter_interval_seconds_( @@ -71,9 +74,41 @@ MetadataAgentConfiguration::MetadataAgentConfiguration() instance_id_(kDefaultInstanceId), instance_zone_(kDefaultInstanceZone) {} -MetadataAgentConfiguration::MetadataAgentConfiguration( - const std::string& filename) : MetadataAgentConfiguration() -{ +int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { + std::string config_file; + boost::program_options::options_description flags_desc; + flags_desc.add_options() + ("help", "Print help message") + ("verbose", boost::program_options::value(&verbose_logging_), + "Enable verbose logging") + ; + boost::program_options::options_description hidden_desc; + hidden_desc.add_options() + (kConfigFileFlag, + boost::program_options::value(&config_file) + ->default_value(""), + "Configuration file location") + ; + boost::program_options::options_description all_desc; + all_desc.add(flags_desc).add(hidden_desc); + boost::program_options::positional_options_description positional_desc; + positional_desc.add(kConfigFileFlag, 1); + boost::program_options::variables_map flags; + boost::program_options::store( + boost::program_options::command_line_parser(ac, av) + .options(all_desc).positional(positional_desc).run(), flags); + boost::program_options::notify(flags); + + if (flags.count("help")) { + std::cout << flags_desc << std::endl; + return 1; + } + + ParseConfigFile(config_file); + return 0; +} + +void MetadataAgentConfiguration::ParseConfigFile(const std::string& filename) { std::lock_guard lock(mutex_); if (filename.empty()) return; diff --git a/src/configuration.h b/src/configuration.h index d9876212..64e18e6f 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -24,7 +24,7 @@ namespace google { class MetadataAgentConfiguration { public: MetadataAgentConfiguration(); - MetadataAgentConfiguration(const std::string& filename); + int ParseArguments(int ac, char** av); // Shared configuration. const std::string& ProjectId() const { @@ -35,6 +35,10 @@ class MetadataAgentConfiguration { std::lock_guard lock(mutex_); return credentials_file_; } + bool VerboseLogging() const { + std::lock_guard lock(mutex_); + return verbose_logging_; + } // Metadata API server configuration options. int MetadataApiNumThreads() const { std::lock_guard lock(mutex_); @@ -102,9 +106,12 @@ class MetadataAgentConfiguration { } private: + void ParseConfigFile(const std::string& filename); + mutable std::mutex mutex_; std::string project_id_; std::string credentials_file_; + bool verbose_logging_; int metadata_api_num_threads_; int metadata_api_port_; int metadata_reporter_interval_seconds_; diff --git a/src/metadatad.cc b/src/metadatad.cc index b4bfee38..30d72dc5 100644 --- a/src/metadatad.cc +++ b/src/metadatad.cc @@ -25,7 +25,12 @@ #include "updater.h" int main(int ac, char** av) { - google::MetadataAgentConfiguration config(ac > 1 ? av[1] : ""); + google::MetadataAgentConfiguration config; + int parse_result = config.ParseArguments(ac, av); + if (parse_result) { + return parse_result; + } + google::MetadataAgent server(config); google::DockerReader docker(config); From 55be0f411717ac5bf07beef3c14c946b4db8e451 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Thu, 12 Oct 2017 11:38:22 -0400 Subject: [PATCH 2/5] Guard most logging with the verbose logging option. --- src/api_server.cc | 76 +++++++++++++++++++++++++++++++--------------- src/api_server.h | 4 +++ src/docker.cc | 20 +++++++++--- src/environment.cc | 22 ++++++++++---- src/environment.h | 4 +++ src/kubernetes.cc | 56 +++++++++++++++++++++++++--------- src/oauth2.cc | 36 ++++++++++++++++------ src/updater.cc | 24 ++++++++++----- 8 files changed, 174 insertions(+), 68 deletions(-) diff --git a/src/api_server.cc b/src/api_server.cc index 7e636b83..2b473779 100644 --- a/src/api_server.cc +++ b/src/api_server.cc @@ -109,10 +109,12 @@ void MetadataApiServer::Handler::operator()(const HttpServer::request& request, static const std::string kPrefix = "/monitoredResource/"; // The format for the local metadata API request is: // {host}:{port}/monitoredResource/{id} - LOG(INFO) << "Handler called: " << request.method - << " " << request.destination - << " headers: " << request.headers - << " body: " << request.body; + if (agent_.config_.VerboseLogging()) { + LOG(INFO) << "Handler called: " << request.method + << " " << request.destination + << " headers: " << request.headers + << " body: " << request.body; + } if (request.method == "GET" && request.destination.find(kPrefix) == 0) { std::string id = request.destination.substr(kPrefix.size()); const auto result = agent_.resource_map_.find(id); @@ -120,12 +122,16 @@ void MetadataApiServer::Handler::operator()(const HttpServer::request& request, // TODO: This could be considered log spam. // As we add more resource mappings, these will become less and less // frequent, and could be promoted to ERROR. - LOG(WARNING) << "No matching resource for " << id; + if (agent_.config_.VerboseLogging()) { + LOG(WARNING) << "No matching resource for " << id; + } response = HttpServer::response::stock_reply( HttpServer::response::not_found, ""); } else { const MonitoredResource& resource = result->second; - LOG(INFO) << "Found resource for " << id << ": " << resource; + if (agent_.config_.VerboseLogging()) { + LOG(INFO) << "Found resource for " << id << ": " << resource; + } response = HttpServer::response::stock_reply( HttpServer::response::ok, resource.ToJSON()->ToString()); } @@ -176,9 +182,13 @@ void MetadataReporter::ReportMetadata() { std::this_thread::sleep_for(std::chrono::seconds(3)); // TODO: Do we need to be able to stop this? while (true) { - LOG(INFO) << "Sending metadata request to server"; + if (agent_.config_.VerboseLogging()) { + LOG(INFO) << "Sending metadata request to server"; + } SendMetadata(agent_.GetMetadataMap()); - LOG(INFO) << "Metadata request sent successfully"; + if (agent_.config_.VerboseLogging()) { + LOG(INFO) << "Metadata request sent successfully"; + } std::this_thread::sleep_for(period_); } //LOG(INFO) << "Metadata reporter exiting"; @@ -188,13 +198,16 @@ namespace { void SendMetadataRequest(std::vector&& entries, const std::string& endpoint, - const std::string& auth_header) { + const std::string& auth_header, + bool verbose_logging) { json::value update_metadata_request = json::object({ {"entries", json::array(std::move(entries))}, }); - LOG(INFO) << "About to send request: POST " << endpoint - << " " << *update_metadata_request; + if (verbose_logging) { + LOG(INFO) << "About to send request: POST " << endpoint + << " " << *update_metadata_request; + } http::client client; http::client::request request(endpoint); @@ -205,7 +218,9 @@ void SendMetadataRequest(std::vector&& entries, request << boost::network::header("Authorization", auth_header); request << boost::network::body(request_body); http::client::response response = client.post(request); - LOG(INFO) << "Server responded with " << body(response); + if (verbose_logging) { + LOG(INFO) << "Server responded with " << body(response); + } // TODO: process response. } @@ -214,11 +229,15 @@ void SendMetadataRequest(std::vector&& entries, void MetadataReporter::SendMetadata( std::map&& metadata) { if (metadata.empty()) { - LOG(INFO) << "No data to send"; + if (agent_.config_.VerboseLogging()) { + LOG(INFO) << "No data to send"; + } return; } - LOG(INFO) << "Sending request to the server"; + if (agent_.config_.VerboseLogging()) { + LOG(INFO) << "Sending request to the server"; + } const std::string project_id = environment_.NumericProjectId(); // The endpoint template is expected to be of the form // "https://stackdriver.googleapis.com/.../projects/{{project_id}}/...". @@ -261,7 +280,8 @@ void MetadataReporter::SendMetadata( continue; } if (total_size + size > limit_bytes) { - SendMetadataRequest(std::move(entries), endpoint, auth_header); + SendMetadataRequest(std::move(entries), endpoint, auth_header, + agent_.config_.VerboseLogging()); entries.clear(); total_size = empty_size; } @@ -269,7 +289,8 @@ void MetadataReporter::SendMetadata( total_size += size; } if (!entries.empty()) { - SendMetadataRequest(std::move(entries), endpoint, auth_header); + SendMetadataRequest(std::move(entries), endpoint, auth_header, + agent_.config_.VerboseLogging()); } } @@ -285,17 +306,22 @@ void MetadataAgent::UpdateResource(const std::vector& resource_ids, // TODO: How do we handle deleted resources? // TODO: Do we care if the value was already there? for (const std::string& id : resource_ids) { - LOG(INFO) << "Updating resource map '" << id << "'->" << resource; + if (config_.VerboseLogging()) { + LOG(INFO) << "Updating resource map '" << id << "'->" << resource; + } resource_map_.emplace(id, resource); } - LOG(INFO) << "Updating metadata map " << resource << "->{" - << "version: " << entry.version << ", " - << "is_deleted: " << entry.is_deleted << ", " - << "created_at: " << rfc3339::ToString(entry.created_at) << ", " - << "collected_at: " << rfc3339::ToString(entry.collected_at) << ", " - << "metadata: " << *entry.metadata << ", " - << "ignore: " << entry.ignore - << "}"; + if (config_.VerboseLogging()) { + LOG(INFO) << "Updating metadata map " << resource << "->{" + << "version: " << entry.version << ", " + << "is_deleted: " << entry.is_deleted << ", " + << "created_at: " << rfc3339::ToString(entry.created_at) << ", " + << "collected_at: " << rfc3339::ToString(entry.collected_at) + << ", " + << "metadata: " << *entry.metadata << ", " + << "ignore: " << entry.ignore + << "}"; + } // Force value update. The repeated search is inefficient, but shouldn't // be a huge deal. metadata_map_.erase(resource); diff --git a/src/api_server.h b/src/api_server.h index 7e3c8ca7..b0ed5b9e 100644 --- a/src/api_server.h +++ b/src/api_server.h @@ -92,6 +92,10 @@ class MetadataAgent { // Starts serving. void start(); + const MetadataAgentConfiguration& config() const { + return config_; + } + private: friend class MetadataApiServer; friend class MetadataReporter; diff --git a/src/docker.cc b/src/docker.cc index f8bd2d9a..13dab92d 100644 --- a/src/docker.cc +++ b/src/docker.cc @@ -45,7 +45,9 @@ DockerReader::DockerReader(const MetadataAgentConfiguration& config) std::vector DockerReader::MetadataQuery() const { - LOG(INFO) << "Docker Query called"; + if (config_.VerboseLogging()) { + LOG(INFO) << "Docker Query called"; + } const std::string zone = environment_.InstanceZone(); const std::string docker_endpoint(config_.DockerEndpointHost() + "v" + config_.DockerApiVersion() + @@ -60,9 +62,13 @@ std::vector try { http::local_client::response list_response = client.get(list_request); Timestamp collected_at = std::chrono::system_clock::now(); - LOG(INFO) << "List response: " << body(list_response); + if (config_.VerboseLogging()) { + LOG(INFO) << "List response: " << body(list_response); + } json::value parsed_list = json::Parser::FromString(body(list_response)); - LOG(INFO) << "Parsed list: " << *parsed_list; + if (config_.VerboseLogging()) { + LOG(INFO) << "Parsed list: " << *parsed_list; + } const json::Array* container_list = parsed_list->As(); for (const json::value& element : *container_list) { try { @@ -71,10 +77,14 @@ std::vector // Inspect the container. http::local_client::request inspect_request(docker_endpoint + "/" + id + "/json"); http::local_client::response inspect_response = client.get(inspect_request); - LOG(INFO) << "Inspect response: " << body(inspect_response); + if (config_.VerboseLogging()) { + LOG(INFO) << "Inspect response: " << body(inspect_response); + } json::value parsed_metadata = json::Parser::FromString(body(inspect_response)); - LOG(INFO) << "Parsed metadata: " << *parsed_metadata; + if (config_.VerboseLogging()) { + LOG(INFO) << "Parsed metadata: " << *parsed_metadata; + } const MonitoredResource resource("docker_container", { {"location", zone}, {"container_id", id}, diff --git a/src/environment.cc b/src/environment.cc index 4a60caa7..0cbc477c 100644 --- a/src/environment.cc +++ b/src/environment.cc @@ -35,7 +35,7 @@ class NoCredentials { std::string filename_; }; -json::value ReadCredentials(const std::string& credentials_file) +json::value ReadCredentials(const std::string& credentials_file, bool verbose) throw(json::Exception, NoCredentials) { std::string filename = credentials_file; if (filename.empty()) { @@ -49,15 +49,21 @@ json::value ReadCredentials(const std::string& credentials_file) } std::ifstream input(filename); if (!input.good()) { - LOG(INFO) << "Missing credentials file " << filename; + if (verbose) { + LOG(INFO) << "Missing credentials file " << filename; + } throw NoCredentials(filename); } - LOG(INFO) << "Reading credentials from " << filename; + if (verbose) { + LOG(INFO) << "Reading credentials from " << filename; + } json::value creds_json = json::Parser::FromStream(input); if (creds_json == nullptr) { throw json::Exception("Could not parse credentials from " + filename); } - LOG(INFO) << "Retrieved credentials from " << filename << ": " << *creds_json; + if (verbose) { + LOG(INFO) << "Retrieved credentials from " << filename << ": " << *creds_json; + } return std::move(creds_json); } @@ -117,8 +123,11 @@ const std::string& Environment::NumericProjectId() const { if (project_id_.empty()) { // Query the metadata server. // TODO: Other sources. - LOG(INFO) << "Getting project id from metadata server"; + if (config_.VerboseLogging()) { + LOG(INFO) << "Getting project id from metadata server"; + } project_id_ = GetMetadataString("project/numeric-project-id"); + LOG(INFO) << "Got project id from metadata server: " << project_id_; } } } @@ -186,7 +195,8 @@ void Environment::ReadApplicationDefaultCredentials() const { return; } try { - json::value creds_json = ReadCredentials(config_.CredentialsFile()); + json::value creds_json = ReadCredentials(config_.CredentialsFile(), + config_.VerboseLogging()); const json::Object* creds = creds_json->As(); diff --git a/src/environment.h b/src/environment.h index b10520a5..1eaacef2 100644 --- a/src/environment.h +++ b/src/environment.h @@ -37,6 +37,10 @@ class Environment { std::string GetMetadataString(const std::string& path) const; + const MetadataAgentConfiguration& config() const { + return config_; + } + private: void ReadApplicationDefaultCredentials() const; diff --git a/src/kubernetes.cc b/src/kubernetes.cc index 62e7c973..6d902bd8 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -64,14 +64,18 @@ constexpr const char kServiceAccountDirectory[] = // Reads a Kubernetes service account secret file into the provided string. // Returns true if the file was read successfully. bool ReadServiceAccountSecret( - const std::string& secret, std::string& destination) { + const std::string& secret, std::string& destination, bool verbose) { std::string filename(std::string(kServiceAccountDirectory) + "/" + secret); std::ifstream input(filename); if (!input.good()) { - LOG(ERROR) << "Missing " << filename; + if (verbose) { + LOG(ERROR) << "Missing " << filename; + } return false; } - LOG(INFO) << "Reading from " << filename; + if (verbose) { + LOG(INFO) << "Reading from " << filename; + } std::getline(input, destination); return !input.fail(); } @@ -83,7 +87,9 @@ KubernetesReader::KubernetesReader(const MetadataAgentConfiguration& config) std::vector KubernetesReader::MetadataQuery() const { - LOG(INFO) << "Kubernetes Query called"; + if (config_.VerboseLogging()) { + LOG(INFO) << "Kubernetes Query called"; + } std::vector result; const std::string platform = "gce"; // TODO: detect other platforms. @@ -92,7 +98,9 @@ std::vector const std::string cluster_name = environment_.KubernetesClusterName(); const std::string node_name = CurrentNode(); - LOG(INFO) << "Current node is " << node_name; + if (config_.VerboseLogging()) { + LOG(INFO) << "Current node is " << node_name; + } const MonitoredResource k8s_node("k8s_node", { {"cluster_name", cluster_name}, @@ -128,7 +136,9 @@ std::vector })}, })}, }); - LOG(INFO) << "Raw node metadata: " << *node_raw_metadata; + if (config_.VerboseLogging()) { + LOG(INFO) << "Raw node metadata: " << *node_raw_metadata; + } #if 0 // TODO: do we need this? @@ -166,14 +176,18 @@ std::vector std::string(kKubernetesEndpointPath) + "/pods" + node_selector + pod_label_selector); Timestamp collected_at = std::chrono::system_clock::now(); - LOG(INFO) << "Parsed pod list: " << *podlist_response; + if (config_.VerboseLogging()) { + LOG(INFO) << "Parsed pod list: " << *podlist_response; + } const json::Object* podlist_object = podlist_response->As(); const std::string api_version = podlist_object->Get("apiVersion"); const json::Array* pod_list = podlist_object->Get("items"); for (const json::value& element : *pod_list) { try { - LOG(INFO) << "Pod: " << *element; + if (config_.VerboseLogging()) { + LOG(INFO) << "Pod: " << *element; + } const json::Object* pod = element->As(); const json::Object* metadata = pod->Get("metadata"); @@ -247,7 +261,9 @@ std::vector })}, })}, }); - LOG(INFO) << "Raw pod metadata: " << *pod_raw_metadata; + if (config_.VerboseLogging()) { + LOG(INFO) << "Raw pod metadata: " << *pod_raw_metadata; + } const std::string k8s_pod_id = boost::algorithm::join( std::vector{kK8sPodResourcePrefix, namespace_name, pod_id}, @@ -284,7 +300,9 @@ std::vector for (int i = 0; i < num_containers; ++i) { const json::value& c_element = (*container_list)[i]; const json::value& c_spec = (*container_specs)[i]; - LOG(INFO) << "Container: " << *c_element; + if (config_.VerboseLogging()) { + LOG(INFO) << "Container: " << *c_element; + } const json::Object* container = c_element->As(); const json::Object* container_spec = c_spec->As(); const std::string container_name = @@ -351,7 +369,9 @@ std::vector })}, })}, }); - LOG(INFO) << "Raw container metadata: " << *container_raw_metadata; + if (config_.VerboseLogging()) { + LOG(INFO) << "Raw container metadata: " << *container_raw_metadata; + } const std::string k8s_container_id = boost::algorithm::join( std::vector{kK8sContainerResourcePrefix, container_id}, @@ -394,7 +414,9 @@ json::value KubernetesReader::QueryMaster(const std::string& path) const http::client::request request(endpoint); request << boost::network::header( "Authorization", "Bearer " + KubernetesApiToken()); - LOG(INFO) << "QueryMaster: Contacting " << endpoint; + if (config_.VerboseLogging()) { + LOG(INFO) << "QueryMaster: Contacting " << endpoint; + } try { http::client::response response = client.get(request); #ifdef VERBOSE @@ -410,7 +432,8 @@ json::value KubernetesReader::QueryMaster(const std::string& path) const const std::string& KubernetesReader::KubernetesApiToken() const { std::lock_guard lock(mutex_); if (kubernetes_api_token_.empty()) { - if (!ReadServiceAccountSecret("token", kubernetes_api_token_)) { + if (!ReadServiceAccountSecret("token", kubernetes_api_token_, + config_.VerboseLogging())) { LOG(ERROR) << "Failed to read Kubernetes API token"; } } @@ -420,7 +443,8 @@ const std::string& KubernetesReader::KubernetesApiToken() const { const std::string& KubernetesReader::KubernetesNamespace() const { std::lock_guard lock(mutex_); if (kubernetes_namespace_.empty()) { - if (!ReadServiceAccountSecret("namespace", kubernetes_namespace_)) { + if (!ReadServiceAccountSecret("namespace", kubernetes_namespace_, + config_.VerboseLogging())) { LOG(ERROR) << "Failed to read Kubernetes namespace"; } } @@ -464,7 +488,9 @@ std::pair KubernetesReader::KindPath( if (found.second) { // Not found, inserted new. try { json::value apilist_response = QueryMaster(query_path); - LOG(INFO) << "Parsed API list: " << *apilist_response; + if (config_.VerboseLogging()) { + LOG(INFO) << "Parsed API list: " << *apilist_response; + } const json::Object* apilist_object = apilist_response->As(); const json::Array* api_list = apilist_object->Get("resources"); diff --git a/src/oauth2.cc b/src/oauth2.cc index 2991d4e3..bdd67725 100644 --- a/src/oauth2.cc +++ b/src/oauth2.cc @@ -199,7 +199,9 @@ json::value OAuth2::ComputeTokenFromCredentials() const { // private key to use is the one associated with the service account email // address (i.e. the email address specified in the 'iss' field above). - LOG(INFO) << "Getting an OAuth2 token"; + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "Getting an OAuth2 token"; + } http::client client; http::client::request request("https://www.googleapis.com/oauth2/v3/token"); std::string grant_type = boost::network::uri::encoded( @@ -221,9 +223,13 @@ json::value OAuth2::ComputeTokenFromCredentials() const { {"iat", json::number(SecondsSinceEpoch(now))}, {"exp", json::number(SecondsSinceEpoch(exp))}, }); - LOG(INFO) << "claim_set = " << claim_set_object->ToString(); + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "claim_set = " << claim_set_object->ToString(); + } std::string claim_set = base64::Encode(claim_set_object->ToString()); - LOG(INFO) << "encoded claim_set = " << claim_set; + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "encoded claim_set = " << claim_set; + } std::string signature = base64::Encode( Sign(jwt_header + "." + claim_set, private_key)); std::string request_body = @@ -236,13 +242,19 @@ json::value OAuth2::ComputeTokenFromCredentials() const { request << boost::network::header("Content-Type", "application/x-www-form-urlencoded"); request << boost::network::body(request_body); - LOG(INFO) << "About to send request: " << request.uri().string() - << " headers: " << request.headers() - << " body: " << request.body(); + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "About to send request: " << request.uri().string() + << " headers: " << request.headers() + << " body: " << request.body(); + } http::client::response response = client.post(request); - LOG(INFO) << "Token response: " << body(response); + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "Token response: " << body(response); + } json::value parsed_token = json::Parser::FromString(body(response)); - LOG(INFO) << "Parsed token: " << *parsed_token; + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "Parsed token: " << *parsed_token; + } return parsed_token; } catch (const json::Exception& e) { @@ -257,9 +269,13 @@ json::value OAuth2::GetMetadataToken() const { if (token_response.empty()) { return nullptr; } - LOG(INFO) << "Token response: " << token_response; + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "Token response: " << token_response; + } json::value parsed_token = json::Parser::FromString(token_response); - LOG(INFO) << "Parsed token: " << *parsed_token; + if (environment_.config().VerboseLogging()) { + LOG(INFO) << "Parsed token: " << *parsed_token; + } return parsed_token; } diff --git a/src/updater.cc b/src/updater.cc index f24cd283..6c9e5709 100644 --- a/src/updater.cc +++ b/src/updater.cc @@ -39,14 +39,18 @@ PollingMetadataUpdater::~PollingMetadataUpdater() { void PollingMetadataUpdater::start() { timer_.lock(); - LOG(INFO) << "Timer locked"; + if (store_->config().VerboseLogging()) { + LOG(INFO) << "Timer locked"; + } reporter_thread_ = std::thread(std::bind(&PollingMetadataUpdater::PollForMetadata, this)); } void PollingMetadataUpdater::stop() { timer_.unlock(); - LOG(INFO) << "Timer unlocked"; + if (store_->config().VerboseLogging()) { + LOG(INFO) << "Timer unlocked"; + } } void PollingMetadataUpdater::PollForMetadata() { @@ -58,7 +62,9 @@ void PollingMetadataUpdater::PollForMetadata() { result.ids, result.resource, std::move(result.metadata)); } // An unlocked timer means we should stop updating. - LOG(INFO) << "Trying to unlock the timer"; + if (store_->config().VerboseLogging()) { + LOG(INFO) << "Trying to unlock the timer"; + } auto start = std::chrono::high_resolution_clock::now(); auto wakeup = start + period_; done = true; @@ -68,15 +74,19 @@ void PollingMetadataUpdater::PollForMetadata() { if (now < wakeup) { continue; } - LOG(INFO) << " Timer unlock timed out after " - << std::chrono::duration_cast(now - start).count() - << "s (good)"; + if (store_->config().VerboseLogging()) { + LOG(INFO) << " Timer unlock timed out after " + << std::chrono::duration_cast(now - start).count() + << "s (good)"; + } start = now; wakeup = start + period_; done = false; } } while (!done); - LOG(INFO) << "Timer unlocked (stop polling)"; + if (store_->config().VerboseLogging()) { + LOG(INFO) << "Timer unlocked (stop polling)"; + } } } From c88b19769a81ca57e368a32d2cb42b5fb8c28ef6 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 13 Oct 2017 14:43:27 -0400 Subject: [PATCH 3/5] Make --verbose a switch (no argument); add shorthand; fixup. --- src/configuration.cc | 4 ++-- src/environment.cc | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index 05918397..8cc9e03e 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -78,8 +78,8 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { std::string config_file; boost::program_options::options_description flags_desc; flags_desc.add_options() - ("help", "Print help message") - ("verbose", boost::program_options::value(&verbose_logging_), + ("help,h", "Print help message") + ("verbose,v", boost::program_options::bool_switch(&verbose_logging_), "Enable verbose logging") ; boost::program_options::options_description hidden_desc; diff --git a/src/environment.cc b/src/environment.cc index 0cbc477c..d06d557e 100644 --- a/src/environment.cc +++ b/src/environment.cc @@ -35,7 +35,8 @@ class NoCredentials { std::string filename_; }; -json::value ReadCredentials(const std::string& credentials_file, bool verbose) +json::value ReadCredentials( + const std::string& credentials_file, bool verbose_logging) throw(json::Exception, NoCredentials) { std::string filename = credentials_file; if (filename.empty()) { @@ -49,19 +50,19 @@ json::value ReadCredentials(const std::string& credentials_file, bool verbose) } std::ifstream input(filename); if (!input.good()) { - if (verbose) { + if (verbose_logging) { LOG(INFO) << "Missing credentials file " << filename; } throw NoCredentials(filename); } - if (verbose) { + if (verbose_logging) { LOG(INFO) << "Reading credentials from " << filename; } json::value creds_json = json::Parser::FromStream(input); if (creds_json == nullptr) { throw json::Exception("Could not parse credentials from " + filename); } - if (verbose) { + if (verbose_logging) { LOG(INFO) << "Retrieved credentials from " << filename << ": " << *creds_json; } return std::move(creds_json); From 6e73a0eaff1498bd27c9d3f9065edeb6601b4790 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 20 Oct 2017 16:10:06 -0400 Subject: [PATCH 4/5] Add missing include. --- src/configuration.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/configuration.cc b/src/configuration.cc index 8cc9e03e..364ff951 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -17,6 +17,7 @@ #include "configuration.h" #include +#include #include #include From acb8d79512e6fd2acfc23d885e58b76684075102 Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Mon, 23 Oct 2017 12:21:52 -0400 Subject: [PATCH 5/5] Add libboost-program-options to runtime dependencies. (#38) --- README.md | 6 ++++-- pkg/deb/debian/control.base | 2 +- pkg/deb/debian/control.xenial | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 198230f8..3dc2b010 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,8 @@ This is the Stackdriver metadata agent. 1. Install runtime dependencies: - $ sudo apt-get install libyajl2 libboost-system1.55.0 libboost-thread1.55.0 + $ sudo apt-get install libyajl2 libboost-program-options1.55.0 \ + libboost-system1.55.0 libboost-thread1.55.0 2. Install build dependencies: @@ -18,7 +19,8 @@ This is the Stackdriver metadata agent. 1. Install runtime dependencies (Ubuntu 16.04 special edition): - $ sudo apt-get install libssl1.0.0 libyajl2 libboost-system1.58.0 \ + $ sudo apt-get install libssl1.0.0 libyajl2 \ + libboost-program-options1.58.0 libboost-system1.58.0 \ libboost-thread1.58.0 2. Install build dependencies (Ubuntu 16.04 special edition): diff --git a/pkg/deb/debian/control.base b/pkg/deb/debian/control.base index 78d6d4cd..e73d0811 100644 --- a/pkg/deb/debian/control.base +++ b/pkg/deb/debian/control.base @@ -6,7 +6,7 @@ Build-Depends: g++, cmake, dpkg-dev, libyajl-dev (>= 2.0), libssl-dev, libboost1 Package: stackdriver-metadata Architecture: amd64 -Depends: libyajl2, libboost-system1.55.0, libboost-thread1.55.0 +Depends: libyajl2, libboost-program-options1.55.0 libboost-system1.55.0, libboost-thread1.55.0 Description: Stackdriver metadata collection daemon The Stackdriver metadata daemon collects resource metadata and sends it to the Stackdriver service. diff --git a/pkg/deb/debian/control.xenial b/pkg/deb/debian/control.xenial index 03c04f1e..470c2759 100644 --- a/pkg/deb/debian/control.xenial +++ b/pkg/deb/debian/control.xenial @@ -6,7 +6,7 @@ Build-Depends: g++, cmake, dpkg-dev, libyajl-dev (>= 2.0), libssl-dev, libboost1 Package: stackdriver-metadata Architecture: amd64 -Depends: libssl1.0.0, libyajl2, libboost-system1.58.0, libboost-thread1.58.0 +Depends: libssl1.0.0, libyajl2, libboost-program-options1.58.0, libboost-system1.58.0, libboost-thread1.58.0 Description: Stackdriver metadata collection daemon The Stackdriver metadata daemon collects resource metadata and sends it to the Stackdriver service.