From 7f00c38e437ae68b145ba9062ff0e07bc3abb70e Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 19 Feb 2021 13:07:58 +0000 Subject: [PATCH 1/2] feat: use structured logging for exceptions Log exceptions from the application using structured logging so they are reported to the right places. --- .../basic_integration_test.cc | 9 +++- .../cloud_event_integration_test.cc | 8 +++- .../functions/internal/call_user_function.cc | 48 ++++++++++++------- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/google/cloud/functions/integration_tests/basic_integration_test.cc b/google/cloud/functions/integration_tests/basic_integration_test.cc index 538c51eb..2b3253d6 100644 --- a/google/cloud/functions/integration_tests/basic_integration_test.cc +++ b/google/cloud/functions/integration_tests/basic_integration_test.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -138,8 +139,12 @@ TEST(RunIntegrationTest, ExceptionLogsToStderr) { std::string line; std::getline(child_stderr, line); - EXPECT_THAT(line, HasSubstr("standard C++ exception")); - EXPECT_THAT(line, HasSubstr("/exception/test-string")); + auto log = nlohmann::json::parse(line, /*cb=*/nullptr, + /*allow_exceptions=*/false); + ASSERT_TRUE(log.is_object()); + EXPECT_EQ(log.value("severity", ""), "error"); + EXPECT_THAT(log.value("message", ""), HasSubstr("standard C++ exception")); + EXPECT_THAT(log.value("message", ""), HasSubstr("/exception/test-string")); try { (void)HttpGet("localhost", "8010", "/quit/program/0"); diff --git a/google/cloud/functions/integration_tests/cloud_event_integration_test.cc b/google/cloud/functions/integration_tests/cloud_event_integration_test.cc index fdbab9f3..a0e6041f 100644 --- a/google/cloud/functions/integration_tests/cloud_event_integration_test.cc +++ b/google/cloud/functions/integration_tests/cloud_event_integration_test.cc @@ -249,8 +249,12 @@ TEST(RunIntegrationTest, ExceptionLogsToStderr) { std::string line; std::getline(child_stderr, line); - EXPECT_THAT(line, HasSubstr("standard C++ exception")); - EXPECT_THAT(line, HasSubstr("/exception/test-string")); + auto log = nlohmann::json::parse(line, /*cb=*/nullptr, + /*allow_exceptions=*/false); + ASSERT_TRUE(log.is_object()); + EXPECT_EQ(log.value("severity", ""), "error"); + EXPECT_THAT(log.value("message", ""), HasSubstr("standard C++ exception")); + EXPECT_THAT(log.value("message", ""), HasSubstr("/exception/test-string")); try { (void)HttpPost("localhost", "8020", "/quit/program/0"); diff --git a/google/cloud/functions/internal/call_user_function.cc b/google/cloud/functions/internal/call_user_function.cc index f079c246..600a55ba 100644 --- a/google/cloud/functions/internal/call_user_function.cc +++ b/google/cloud/functions/internal/call_user_function.cc @@ -16,6 +16,7 @@ #include "google/cloud/functions/internal/parse_cloud_event_http.h" #include "google/cloud/functions/internal/wrap_request.h" #include "google/cloud/functions/internal/wrap_response.h" +#include #include #include @@ -32,6 +33,33 @@ struct UnwrapResponse { } }; +namespace { +BeastResponse ApplicationError(nlohmann::json const& error) { + auto msg = error.dump(); + std::cerr << msg << std::endl; + BeastResponse response; + response.result(be::http::status::internal_server_error); + response.insert("content-type", "application/json"); + response.body() = std::move(msg); + return response; +} + +BeastResponse ReportExceptionInFunction(std::exception const& ex) { + return ApplicationError( + {{"severity", "error"}, + {"message", + std::string("standard C++ exception thrown by the function: ") + + ex.what()}}); +} + +BeastResponse ReportUnknownExceptionInFunction() { + return ApplicationError({ + {"severity", "error"}, + {"message", std::string("unknown C++ exception thrown by the function")}, + }); +} +} // namespace + BeastResponse CallUserFunction(functions::UserHttpFunction const& function, BeastRequest request) try { if (request.target() == "/favicon.ico" || request.target() == "/robots.txt") { @@ -42,15 +70,9 @@ BeastResponse CallUserFunction(functions::UserHttpFunction const& function, auto response = function(MakeHttpRequest(std::move(request))); return UnwrapResponse::unwrap(std::move(response)); } catch (std::exception const& ex) { - std::cerr << "standard C++ exception thrown: " << ex.what() << std::endl; - BeastResponse response; - response.result(be::http::status::internal_server_error); - return response; + return ReportExceptionInFunction(ex); } catch (...) { - std::cerr << "unknown c++ exception thrown" << std::endl; - BeastResponse response; - response.result(be::http::status::internal_server_error); - return response; + return ReportUnknownExceptionInFunction(); } BeastResponse CallUserFunction( @@ -67,15 +89,9 @@ BeastResponse CallUserFunction( } return BeastResponse{}; } catch (std::exception const& ex) { - std::cerr << "standard C++ exception thrown: " << ex.what() << std::endl; - BeastResponse response; - response.result(be::http::status::internal_server_error); - return response; + return ReportExceptionInFunction(ex); } catch (...) { - std::cerr << "unknown c++ exception thrown" << std::endl; - BeastResponse response; - response.result(be::http::status::internal_server_error); - return response; + return ReportUnknownExceptionInFunction(); } } // namespace FUNCTIONS_FRAMEWORK_CPP_NS From 8298825af4509d4d55403c28e7031e9cf6cf0802 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 19 Feb 2021 18:17:16 +0000 Subject: [PATCH 2/2] Address review comments --- google/cloud/functions/internal/call_user_function.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/google/cloud/functions/internal/call_user_function.cc b/google/cloud/functions/internal/call_user_function.cc index 600a55ba..b0446581 100644 --- a/google/cloud/functions/internal/call_user_function.cc +++ b/google/cloud/functions/internal/call_user_function.cc @@ -36,6 +36,9 @@ struct UnwrapResponse { namespace { BeastResponse ApplicationError(nlohmann::json const& error) { auto msg = error.dump(); + // Log the message to stderr. If the message is properly formatted, as it is + // done here, they are sent picked up and parsed by Cloud Logging: + // https://cloud.google.com/functions/docs/monitoring/logging#writing_structured_logs std::cerr << msg << std::endl; BeastResponse response; response.result(be::http::status::internal_server_error);