Skip to content

Commit 105398b

Browse files
kpavlov00kpavlov00
committed
refactor grpc: make tracing functions noexcept & cleanups
commit_hash:4c922f957838b1e7949d43ce12daf027885621aa
1 parent 5224c99 commit 105398b

9 files changed

Lines changed: 40 additions & 33 deletions

File tree

‎grpc/include/userver/ugrpc/client/impl/async_method_invocation.hpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@ USERVER_NAMESPACE_BEGIN
1111

1212
namespace ugrpc::client::impl {
1313

14-
/// AsyncMethodInvocation for Finish method that stops stats and Span timers
15-
/// ASAP, without waiting for a Task to wake up
14+
/// AsyncMethodInvocation for Finish method that provide finish notification time
1615
class FinishAsyncMethodInvocation final : public ugrpc::impl::AsyncMethodInvocation {
1716
public:
1817
void Notify(bool ok) noexcept override;
1918

2019
/// When notify is called, we store timestamp and later use it in statistics
21-
[[nodiscard]] std::chrono::steady_clock::time_point GetFinishTime() const;
20+
[[nodiscard]] std::chrono::steady_clock::time_point GetFinishTime() const noexcept;
2221

2322
private:
2423
std::optional<std::chrono::steady_clock::time_point> finish_time_;

‎grpc/include/userver/ugrpc/client/impl/async_methods.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ void PrepareFinish(CallState& state);
6868

6969
void ProcessFinish(CallState& state, const google::protobuf::Message* final_response);
7070

71-
void ProcessFinishCancelled(CallState& state);
71+
void ProcessFinishCancelled(CallState& state) noexcept;
7272

73-
void ProcessFinishNetworkError(CallState& state);
73+
void ProcessFinishNetworkError(CallState& state) noexcept;
7474

7575
void CheckFinishStatus(CallState& state);
7676

‎grpc/include/userver/ugrpc/client/impl/call_state.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,19 @@ class CallState final {
8484
bool IsWriteAndCheckAvailable() const noexcept;
8585

8686
// please read comments for 'invocation_' private member on why
87-
// we use two different invocation types
87+
// we use different invocation types
8888
void EmplaceAsyncMethodInvocation();
8989

9090
// please read comments for 'invocation_' private member on why
91-
// we use two different invocation types
91+
// we use different invocation types
9292
void EmplaceFinishAsyncMethodInvocation();
9393

9494
// please read comments for 'invocation_' private member on why
95-
// we use two different invocation types
95+
// we use different invocation types
9696
AsyncMethodInvocation& GetAsyncMethodInvocation() noexcept;
9797

9898
// please read comments for 'invocation_' private member on why
99-
// we use two different invocation types
99+
// we use different invocation types
100100
FinishAsyncMethodInvocation& GetFinishAsyncMethodInvocation() noexcept;
101101

102102
// These are for asserts and invariants. Do NOT branch actual code

‎grpc/src/ugrpc/client/impl/async_method_invocation.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include <userver/ugrpc/client/impl/async_method_invocation.hpp>
22

3+
#include <userver/utils/assert.hpp>
4+
35
USERVER_NAMESPACE_BEGIN
46

57
namespace ugrpc::client::impl {
@@ -9,8 +11,8 @@ void FinishAsyncMethodInvocation::Notify(bool ok) noexcept {
911
AsyncMethodInvocation::Notify(ok);
1012
}
1113

12-
std::chrono::steady_clock::time_point FinishAsyncMethodInvocation::GetFinishTime() const {
13-
UINVARIANT(finish_time_.has_value(), "GetFinishTime should be called after invocation was notified");
14+
std::chrono::steady_clock::time_point FinishAsyncMethodInvocation::GetFinishTime() const noexcept {
15+
UASSERT_MSG(finish_time_.has_value(), "GetFinishTime should be called after invocation was notified");
1416
return *finish_time_;
1517
}
1618

‎grpc/src/ugrpc/client/impl/async_methods.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace ugrpc::client::impl {
1616

1717
namespace {
1818

19-
void ProcessCallStatistics(CallState& state, const grpc::Status& status) {
19+
void ProcessCallStatistics(CallState& state, const grpc::Status& status) noexcept {
2020
auto& stats = state.GetStatsScope();
2121
stats.OnExplicitFinish(status.error_code());
2222
if (status.error_code() == grpc::StatusCode::DEADLINE_EXCEEDED && state.IsDeadlinePropagated()) {
@@ -88,13 +88,13 @@ void ProcessFinish(CallState& state, const google::protobuf::Message* final_resp
8888
SetStatusAndResetSpan(state, status);
8989
}
9090

91-
void ProcessFinishCancelled(CallState& state) {
91+
void ProcessFinishCancelled(CallState& state) noexcept {
9292
state.GetStatsScope().OnCancelled();
9393
state.GetStatsScope().Flush();
9494
SetErrorAndResetSpan(state, "Task cancellation at 'Finish'");
9595
}
9696

97-
void ProcessFinishNetworkError(CallState& state) {
97+
void ProcessFinishNetworkError(CallState& state) noexcept {
9898
state.GetStatsScope().OnNetworkError();
9999
state.GetStatsScope().Flush();
100100
SetErrorAndResetSpan(state, "Network error at 'Finish'");

‎grpc/src/ugrpc/client/impl/call_state.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44

55
#include <userver/utils/assert.hpp>
66

7-
#include <ugrpc/client/impl/tracing.hpp>
87
#include <userver/ugrpc/client/impl/call_params.hpp>
98

9+
#include <ugrpc/client/impl/tracing.hpp>
10+
1011
USERVER_NAMESPACE_BEGIN
1112

1213
namespace ugrpc::client::impl {

‎grpc/src/ugrpc/client/impl/tracing.cpp‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,24 @@ void SetupSpan(
4949
}
5050
}
5151

52-
void SetErrorForSpan(tracing::Span& span, std::string_view error_message) {
53-
span.SetLogLevel(logging::Level::kWarning);
54-
span.AddTag(tracing::kErrorFlag, true);
55-
span.AddTag(tracing::kErrorMessage, std::string{error_message});
52+
void SetErrorForSpan(tracing::Span& span, std::string_view error_message) noexcept {
53+
try {
54+
span.SetLogLevel(logging::Level::kWarning);
55+
span.AddTag(tracing::kErrorFlag, true);
56+
span.AddTag(tracing::kErrorMessage, std::string{error_message});
57+
} catch (const std::exception& ex) {
58+
LOG_LIMITED_ERROR() << "Can not set error for span: " << ex;
59+
}
5660
}
5761

58-
void SetStatusForSpan(tracing::Span& span, const grpc::Status& status) {
59-
span.AddTag("grpc_code", ugrpc::ToString(status.error_code()));
60-
if (!status.ok()) {
61-
SetErrorForSpan(span, status.error_message());
62+
void SetStatusForSpan(tracing::Span& span, const grpc::Status& status) noexcept {
63+
try {
64+
span.AddTag("grpc_code", ugrpc::ToString(status.error_code()));
65+
if (!status.ok()) {
66+
SetErrorForSpan(span, status.error_message());
67+
}
68+
} catch (const std::exception& ex) {
69+
LOG_LIMITED_ERROR() << "Can not set status for span: " << ex;
6270
}
6371
}
6472

‎grpc/src/ugrpc/client/impl/tracing.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ void SetupSpan(
1919
std::string_view call_name
2020
);
2121

22-
void SetStatusForSpan(tracing::Span& span, const grpc::Status& status);
22+
void SetStatusForSpan(tracing::Span& span, const grpc::Status& status) noexcept;
2323

24-
void SetErrorForSpan(tracing::Span& span, std::string_view error_message);
24+
void SetErrorForSpan(tracing::Span& span, std::string_view error_message) noexcept;
2525

2626
} // namespace ugrpc::client::impl
2727

‎grpc/src/ugrpc/client/rpc.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ UnaryFinishFutureImpl::UnaryFinishFutureImpl(UnaryFinishFutureImpl&& other) noex
2424

2525
UnaryFinishFutureImpl& UnaryFinishFutureImpl::operator=(UnaryFinishFutureImpl&& other) noexcept {
2626
if (this == &other) return *this;
27+
// we should destruct current instance
2728
[[maybe_unused]] auto for_destruction = std::move(*this);
2829
// state_ == nullptr signals that *this is empty. Other fields may remain garbage in `other`.
2930
state_ = std::exchange(other.state_, nullptr);
@@ -61,8 +62,8 @@ engine::FutureStatus UnaryFinishFutureImpl::WaitUntil(engine::Deadline deadline)
6162
switch (wait_status) {
6263
case impl::AsyncMethodInvocation::WaitStatus::kOk:
6364
state_->SetFinishProcessed();
65+
state_->GetStatsScope().SetFinishTime(finish.GetFinishTime());
6466
try {
65-
state_->GetStatsScope().SetFinishTime(finish.GetFinishTime());
6667
ProcessFinish(*state_, response_);
6768
} catch (...) {
6869
exception_ = std::current_exception();
@@ -71,13 +72,9 @@ engine::FutureStatus UnaryFinishFutureImpl::WaitUntil(engine::Deadline deadline)
7172

7273
case impl::AsyncMethodInvocation::WaitStatus::kError:
7374
state_->SetFinishProcessed();
74-
try {
75-
state_->GetStatsScope().SetFinishTime(finish.GetFinishTime());
76-
ProcessFinishNetworkError(*state_);
77-
exception_ = std::make_exception_ptr(RpcInterruptedError(state_->GetCallName(), "Finish"));
78-
} catch (...) {
79-
exception_ = std::current_exception();
80-
}
75+
state_->GetStatsScope().SetFinishTime(finish.GetFinishTime());
76+
ProcessFinishNetworkError(*state_);
77+
exception_ = std::make_exception_ptr(RpcInterruptedError(state_->GetCallName(), "Finish"));
8178
return engine::FutureStatus::kReady;
8279

8380
case impl::AsyncMethodInvocation::WaitStatus::kCancelled:

0 commit comments

Comments
 (0)