Skip to content

Commit e1236f3

Browse files
laramielcopybara-github
authored andcommitted
Add more logging for gcs_grpc and some other grpc uses.
PiperOrigin-RevId: 846798126 Change-Id: I112c9dc2d60c688e84f06b57fd476b37ab76cc7d
1 parent 92249e4 commit e1236f3

File tree

9 files changed

+122
-26
lines changed

9 files changed

+122
-26
lines changed

tensorstore/internal/grpc/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ tensorstore_cc_library(
4444
"//tensorstore/proto:proto_util",
4545
"@abseil-cpp//absl/base:core_headers",
4646
"@abseil-cpp//absl/log:absl_log",
47+
"@abseil-cpp//absl/strings",
4748
"@com_google_protobuf//:protobuf",
4849
"@grpc//:grpc++",
4950
],

tensorstore/internal/grpc/clientauth/BUILD

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ tensorstore_cc_library(
6666
hdrs = ["channel_authentication.h"],
6767
deps = [
6868
":authentication_strategy",
69+
"//tensorstore/internal/log:verbose_flag",
6970
"//tensorstore/util:future",
71+
"@abseil-cpp//absl/base:core_headers",
72+
"@abseil-cpp//absl/log:absl_log",
7073
"@grpc//:grpc++",
7174
],
7275
)
@@ -77,7 +80,10 @@ tensorstore_cc_library(
7780
hdrs = ["call_authentication.h"],
7881
deps = [
7982
":authentication_strategy",
83+
"//tensorstore/internal/log:verbose_flag",
8084
"//tensorstore/util:future",
85+
"@abseil-cpp//absl/base:core_headers",
86+
"@abseil-cpp//absl/log:absl_log",
8187
"@grpc//:grpc++",
8288
],
8389
)
@@ -91,9 +97,12 @@ tensorstore_cc_library(
9197
":access_token_cache",
9298
":authentication_strategy",
9399
":iam_stub",
100+
"//tensorstore/internal/log:verbose_flag",
94101
"//tensorstore/util:executor",
95102
"//tensorstore/util:future",
96103
"//tensorstore/util:result",
104+
"@abseil-cpp//absl/base:core_headers",
105+
"@abseil-cpp//absl/log:absl_log",
97106
"@abseil-cpp//absl/status",
98107
"@abseil-cpp//absl/synchronization",
99108
"@abseil-cpp//absl/time",

tensorstore/internal/grpc/clientauth/call_authentication.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,22 @@
1919
#include <string_view>
2020
#include <utility>
2121

22+
#include "absl/base/attributes.h"
23+
#include "absl/log/absl_log.h"
2224
#include "grpcpp/client_context.h" // third_party
2325
#include "grpcpp/security/credentials.h" // third_party
2426
#include "grpcpp/support/channel_arguments.h" // third_party
2527
#include "tensorstore/internal/grpc/clientauth/authentication_strategy.h"
28+
#include "tensorstore/internal/log/verbose_flag.h"
2629
#include "tensorstore/util/future.h"
2730

2831
namespace tensorstore {
2932
namespace internal_grpc {
33+
namespace {
34+
35+
ABSL_CONST_INIT internal_log::VerboseFlag verbose_logging("grpc");
36+
37+
}
3038

3139
std::shared_ptr<grpc::ChannelCredentials>
3240
GrpcCallCredentialsAuthentication::GetChannelCredentials(
@@ -48,6 +56,7 @@ GrpcCallCredentialsAuthentication::ConfigureContext(
4856
std::shared_ptr<GrpcAuthenticationStrategy>
4957
CreateAccessTokenAuthenticationStrategy(const std::string& token,
5058
const CaInfo& ca_info) {
59+
ABSL_LOG_IF(INFO, verbose_logging) << "Using access_token credentials";
5160
grpc::SslCredentialsOptions ssl_options;
5261
auto cainfo = LoadCAInfo(ca_info);
5362
if (cainfo) ssl_options.pem_root_certs = std::move(*cainfo);
@@ -58,6 +67,7 @@ CreateAccessTokenAuthenticationStrategy(const std::string& token,
5867
std::shared_ptr<GrpcAuthenticationStrategy>
5968
CreateServiceAccountAuthenticationStrategy(const std::string& json_object,
6069
const CaInfo& ca_info) {
70+
ABSL_LOG_IF(INFO, verbose_logging) << "Using service_account credentials";
6171
grpc::SslCredentialsOptions ssl_options;
6272
auto cainfo = LoadCAInfo(ca_info);
6373
if (cainfo) ssl_options.pem_root_certs = std::move(*cainfo);

tensorstore/internal/grpc/clientauth/channel_authentication.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,22 @@
2020
#include <utility>
2121
#include <vector>
2222

23+
#include "absl/base/attributes.h"
24+
#include "absl/log/absl_log.h"
2325
#include "grpcpp/client_context.h" // third_party
2426
#include "grpcpp/security/credentials.h" // third_party
2527
#include "grpcpp/support/channel_arguments.h" // third_party
2628
#include "tensorstore/internal/grpc/clientauth/authentication_strategy.h"
29+
#include "tensorstore/internal/log/verbose_flag.h"
2730
#include "tensorstore/util/future.h"
2831

2932
namespace tensorstore {
3033
namespace internal_grpc {
34+
namespace {
35+
36+
ABSL_CONST_INIT internal_log::VerboseFlag verbose_logging("grpc");
37+
38+
}
3139

3240
std::shared_ptr<grpc::ChannelCredentials>
3341
GrpcChannelCredentialsAuthentication::GetChannelCredentials(
@@ -48,13 +56,15 @@ GrpcChannelCredentialsAuthentication::ConfigureContext(
4856
/// Creates an "insecure" authentication strategy with the given token.
4957
std::shared_ptr<GrpcAuthenticationStrategy>
5058
CreateInsecureAuthenticationStrategy() {
59+
ABSL_LOG_IF(INFO, verbose_logging) << "Using insecure credentials";
5160
return std::make_shared<GrpcChannelCredentialsAuthentication>(
5261
grpc::InsecureChannelCredentials());
5362
}
5463

5564
/// Creates a "google_default" authentication strategy.
5665
std::shared_ptr<GrpcAuthenticationStrategy>
5766
CreateGoogleDefaultAuthenticationStrategy() {
67+
ABSL_LOG_IF(INFO, verbose_logging) << "Using google_default credentials";
5868
return std::make_shared<GrpcChannelCredentialsAuthentication>(
5969
grpc::GoogleDefaultCredentials());
6070
}
@@ -63,6 +73,7 @@ std::shared_ptr<GrpcAuthenticationStrategy>
6373
CreateExternalAccountAuthenticationStrategy(
6474
const std::string& json_object, const std::vector<std::string>& scopes,
6575
const CaInfo& ca_info) {
76+
ABSL_LOG_IF(INFO, verbose_logging) << "Using external_account credentials";
6677
grpc::SslCredentialsOptions ssl_options;
6778
auto cainfo = LoadCAInfo(ca_info);
6879
if (cainfo) ssl_options.pem_root_certs = std::move(*cainfo);

tensorstore/internal/grpc/clientauth/impersonate_service_account.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
#include <utility>
2222

2323
#include "google/iam/credentials/v1/common.pb.h"
24+
#include "absl/base/attributes.h"
25+
#include "absl/log/absl_log.h"
2426
#include "absl/status/status.h"
2527
#include "absl/synchronization/mutex.h"
2628
#include "absl/time/time.h"
@@ -31,18 +33,26 @@
3133
#include "tensorstore/internal/grpc/clientauth/access_token_cache.h"
3234
#include "tensorstore/internal/grpc/clientauth/authentication_strategy.h"
3335
#include "tensorstore/internal/grpc/clientauth/iam_stub.h"
36+
#include "tensorstore/internal/log/verbose_flag.h"
3437
#include "tensorstore/util/executor.h"
3538
#include "tensorstore/util/future.h"
3639
#include "tensorstore/util/result.h"
3740

3841
namespace tensorstore {
3942
namespace internal_grpc {
43+
namespace {
4044

45+
ABSL_CONST_INIT internal_log::VerboseFlag verbose_logging("grpc");
46+
47+
}
4148
/* static */
4249
std::shared_ptr<GrpcImpersonateServiceAccount>
4350
GrpcImpersonateServiceAccount::Create(
4451
const ImpersonateServiceAccountConfig& config, const CaInfo& ca_info,
4552
std::shared_ptr<GrpcAuthenticationStrategy> base_strategy) {
53+
ABSL_LOG_IF(INFO, verbose_logging)
54+
<< "Using impersonate_service_account credentials";
55+
4656
auto source = CreateIamCredentialsSource(
4757
base_strategy, /*endpoint=*/{}, config.target_service_account,
4858
config.lifetime, config.scopes, config.delegates);

tensorstore/internal/grpc/logging_interceptor.cc

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@
1414

1515
#include "tensorstore/internal/grpc/logging_interceptor.h"
1616

17+
#include <map>
18+
#include <string>
1719
#include <string_view>
1820

1921
#include "absl/base/attributes.h"
2022
#include "absl/log/absl_log.h"
23+
#include "absl/strings/str_cat.h"
2124
#include "grpcpp/support/client_interceptor.h" // third_party
2225
#include "grpcpp/support/interceptor.h" // third_party
2326
#include "google/protobuf/message.h"
@@ -34,7 +37,7 @@ namespace tensorstore {
3437
namespace internal_grpc {
3538
namespace {
3639

37-
ABSL_CONST_INIT internal_log::VerboseFlag grpc_logging("grpc_channel");
40+
ABSL_CONST_INIT internal_log::VerboseFlag verbose_logging("grpc_channel");
3841

3942
// Implements detailed logging for gRPC calls to GCS.
4043
class LoggingInterceptor : public Interceptor {
@@ -44,46 +47,69 @@ class LoggingInterceptor : public Interceptor {
4447
std::string_view method_name() const { return info_->method(); }
4548

4649
void Intercept(InterceptorBatchMethods* methods) override {
47-
if (!grpc_logging) {
50+
if (!verbose_logging) {
4851
methods->Proceed();
4952
return;
5053
}
54+
55+
std::string log;
56+
57+
std::multimap<std::string, std::string>* metadata = nullptr;
58+
if (methods->QueryInterceptionHookPoint(
59+
InterceptionHookPoints::PRE_SEND_INITIAL_METADATA)) {
60+
metadata = methods->GetSendInitialMetadata();
61+
}
62+
if (methods->QueryInterceptionHookPoint(
63+
InterceptionHookPoints::PRE_SEND_STATUS)) {
64+
metadata = methods->GetSendTrailingMetadata();
65+
}
66+
if (metadata != nullptr) {
67+
absl::StrAppend(&log, "Metadata:");
68+
for (const auto& [key, value] : *metadata) {
69+
absl::StrAppend(&log, " ", key, ": ", value);
70+
}
71+
}
72+
5173
if (methods->QueryInterceptionHookPoint(
5274
InterceptionHookPoints::PRE_SEND_MESSAGE)) {
5375
auto* message =
5476
static_cast<const google::protobuf::Message*>(methods->GetSendMessage());
55-
bool is_first_message = !started_;
56-
started_ = true;
57-
if (grpc_logging.Level(2)) {
58-
ABSL_LOG(INFO) << "Begin: " << method_name() << " "
59-
<< ConciseDebugString(*message);
60-
} else if (is_first_message) {
61-
ABSL_LOG(INFO) << "Begin: " << method_name();
77+
if (verbose_logging.Level(2) && message != nullptr) {
78+
absl::StrAppend(&log, log.empty() ? "" : "\n",
79+
"Request: ", ConciseDebugString(*message));
6280
}
6381
}
64-
if (grpc_logging.Level(2) &&
65-
methods->QueryInterceptionHookPoint(
82+
if (methods->QueryInterceptionHookPoint(
6683
InterceptionHookPoints::POST_RECV_MESSAGE)) {
67-
ABSL_LOG(INFO) << method_name() << " "
68-
<< ConciseDebugString(*static_cast<const google::protobuf::Message*>(
69-
methods->GetRecvMessage()));
84+
auto* message =
85+
static_cast<const google::protobuf::Message*>(methods->GetRecvMessage());
86+
if (verbose_logging.Level(2) && message != nullptr) {
87+
absl::StrAppend(&log, log.empty() ? "" : "\n",
88+
"Response: ", ConciseDebugString(*message));
89+
}
90+
}
91+
92+
if (methods->QueryInterceptionHookPoint(
93+
InterceptionHookPoints::PRE_SEND_STATUS)) {
94+
if (auto status = methods->GetSendStatus(); !status.ok()) {
95+
absl::StrAppend(&log, log.empty() ? "" : "\n", "Send Status: ",
96+
internal::GrpcStatusToAbslStatus(status));
97+
}
7098
}
99+
71100
if (methods->QueryInterceptionHookPoint(
72101
InterceptionHookPoints::POST_RECV_STATUS)) {
73-
if (auto* status = methods->GetRecvStatus();
74-
status != nullptr && !status->ok()) {
75-
ABSL_LOG(INFO) << "Error: " << method_name() << " "
76-
<< internal::GrpcStatusToAbslStatus(*status);
77-
} else {
78-
ABSL_LOG(INFO) << "End: " << method_name();
102+
if (auto* status = methods->GetRecvStatus(); status != nullptr) {
103+
absl::StrAppend(&log, log.empty() ? "" : "\n", "Recv Status: ",
104+
internal::GrpcStatusToAbslStatus(*status));
79105
}
80106
}
107+
ABSL_LOG_IF(INFO, !log.empty()) << method_name() << " " << log;
81108
methods->Proceed();
82109
}
83110

84111
private:
85112
const ClientRpcInfo* info_;
86-
bool started_ = false;
87113
};
88114

89115
} // namespace

tensorstore/internal/grpc/utils.cc

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
1717
#include <grpcpp/support/status.h>
1818

1919
#include <string>
20+
#include <type_traits>
21+
#include <utility>
2022

2123
#include "absl/status/status.h"
2224
#include "absl/strings/cord.h"
25+
#include "grpcpp/support/status.h" // third_party
2326
#include "tensorstore/internal/source_location.h"
2427
#include "tensorstore/util/status.h"
2528

@@ -50,20 +53,45 @@ TENSORSTORE_STATUS_ASSERT(UNAUTHENTICATED, kUnauthenticated);
5053

5154
namespace tensorstore {
5255
namespace internal {
56+
namespace {
5357

54-
absl::Status GrpcStatusToAbslStatus(grpc::Status s, SourceLocation loc) {
55-
if (s.ok()) return absl::OkStatus();
58+
template <typename T, typename = void>
59+
struct HasAbslStatusOperator : std::false_type {};
60+
61+
template <typename T>
62+
struct HasAbslStatusOperator<
63+
T, std::void_t<decltype(std::declval<const T&>().operator absl::Status())>>
64+
: std::true_type {};
65+
66+
// Overload for when operator exists
67+
template <typename T,
68+
std::enable_if_t<HasAbslStatusOperator<T>::value, int> = 0>
69+
absl::Status ToAbslStatusImpl(const T& s) {
70+
return s.operator absl::Status();
71+
}
72+
73+
// Overload for when operator does not exist
74+
template <typename T,
75+
std::enable_if_t<!HasAbslStatusOperator<T>::value, int> = 0>
76+
absl::Status ToAbslStatusImpl(const T& s) {
5677
auto absl_code = static_cast<absl::StatusCode>(s.error_code());
5778
absl::Status status(absl_code, s.error_message());
58-
MaybeAddSourceLocation(status, loc);
59-
6079
if (!s.error_details().empty()) {
6180
// NOTE: Is error_details() a serialized protobuf::Any?
6281
status.SetPayload("grpc.Status.details", absl::Cord(s.error_details()));
6382
}
6483
return status;
6584
}
6685

86+
} // namespace
87+
88+
absl::Status GrpcStatusToAbslStatus(grpc::Status s, SourceLocation loc) {
89+
if (s.ok()) return absl::OkStatus();
90+
absl::Status status = ToAbslStatusImpl(s);
91+
MaybeAddSourceLocation(status, loc);
92+
return status;
93+
}
94+
6795
grpc::Status AbslStatusToGrpcStatus(const absl::Status& status) {
6896
if (status.ok()) return grpc::Status::OK;
6997

tensorstore/internal/grpc/utils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <grpcpp/support/status.h>
1919

2020
#include "absl/status/status.h"
21+
#include "grpcpp/support/status.h" // third_party
2122
#include "tensorstore/internal/source_location.h"
2223

2324
namespace tensorstore {

tensorstore/internal/tracing/logged_trace_span.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class LoggedTraceSpan : public LocalTraceSpan {
9595
if (id_) {
9696
EndLog(
9797
absl::LogInfoStreamer(location.file_name(), location.line()).stream())
98-
<< status;
98+
<< " " << status;
9999
id_ = 0;
100100
}
101101
return status;

0 commit comments

Comments
 (0)