Commit a3d965b0 authored by Josh Nohle's avatar Josh Nohle Committed by Chromium LUCI CQ

[Nearby] Messaging: Fix receive metric emission; add OAuth token metric

1. Only emit the receive-message metrics and logs if the callback has
   not already been invoked.
2. Record a fast-path-ready notification as a receive-message success.
3. Clarify receive-message histogram summaries to indicate that the
   metric records the result of opening the receive stream, i.e.,
   starting to receive messages, not a result for every message receipt.
4. Add metrics and logs to track send- and receive-message OAuth token
   fetches.

Manually verified logs and histograms.

Fixed: 1164009
Change-Id: I2a064cae444ec890971db40b15c93c713e32565c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2619628
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: default avatarJames Vecore <vecore@google.com>
Reviewed-by: default avatarCaitlin Fischer <caitlinfischer@google.com>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842617}
parent 5a547e6a
...@@ -70,13 +70,12 @@ void LogReceiveResult(bool success, const network::SimpleURLLoader* loader) { ...@@ -70,13 +70,12 @@ void LogReceiveResult(bool success, const network::SimpleURLLoader* loader) {
"FailureReason", "FailureReason",
http_status.GetResultCodeForMetrics()); http_status.GetResultCodeForMetrics());
} }
} else {
ss << " Missing URL loader.";
} }
if (success) { if (success) {
NS_LOG(VERBOSE) << ss.str(); NS_LOG(VERBOSE) << ss.str();
} else { } else {
NS_LOG(WARNING) << ss.str(); NS_LOG(ERROR) << ss.str();
} }
} }
...@@ -124,7 +123,12 @@ void ReceiveMessagesExpress::DoStartReceivingMessages( ...@@ -124,7 +123,12 @@ void ReceiveMessagesExpress::DoStartReceivingMessages(
ReceiveMessagesExpressRequest& request, ReceiveMessagesExpressRequest& request,
const std::string& oauth_token) { const std::string& oauth_token) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::UmaHistogramBoolean(
"Nearby.Connections.InstantMessaging.ReceiveExpress."
"OAuthTokenFetchResult",
!oauth_token.empty());
if (oauth_token.empty()) { if (oauth_token.empty()) {
NS_LOG(ERROR) << __func__ << ": Failed to fetch OAuth token.";
std::move(success_callback_).Run(false); std::move(success_callback_).Run(false);
return; return;
} }
...@@ -164,9 +168,10 @@ void ReceiveMessagesExpress::OnDataReceived(base::StringPiece data, ...@@ -164,9 +168,10 @@ void ReceiveMessagesExpress::OnDataReceived(base::StringPiece data,
void ReceiveMessagesExpress::OnComplete(bool success) { void ReceiveMessagesExpress::OnComplete(bool success) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LogReceiveResult(success, url_loader_.get()); if (!success_callback_.is_null()) {
if (!success_callback_.is_null()) LogReceiveResult(success, url_loader_.get());
std::move(success_callback_).Run(success); std::move(success_callback_).Run(success);
}
url_loader_.reset(); url_loader_.reset();
stream_parser_.reset(); stream_parser_.reset();
...@@ -178,6 +183,7 @@ void ReceiveMessagesExpress::OnRetry(base::OnceClosure start_retry) { ...@@ -178,6 +183,7 @@ void ReceiveMessagesExpress::OnRetry(base::OnceClosure start_retry) {
void ReceiveMessagesExpress::OnFastPathReady() { void ReceiveMessagesExpress::OnFastPathReady() {
if (!success_callback_.is_null()) { if (!success_callback_.is_null()) {
LogReceiveResult(/*success=*/true, /*loader=*/nullptr);
std::move(success_callback_).Run(true); std::move(success_callback_).Run(true);
} }
} }
...@@ -63,7 +63,7 @@ void LogSendResult(bool success, const NearbyShareHttpStatus& http_status) { ...@@ -63,7 +63,7 @@ void LogSendResult(bool success, const NearbyShareHttpStatus& http_status) {
if (success) { if (success) {
NS_LOG(VERBOSE) << ss.str(); NS_LOG(VERBOSE) << ss.str();
} else { } else {
NS_LOG(WARNING) << ss.str(); NS_LOG(ERROR) << ss.str();
} }
base::UmaHistogramBoolean( base::UmaHistogramBoolean(
"Nearby.Connections.InstantMessaging.SendExpress.Result", success); "Nearby.Connections.InstantMessaging.SendExpress.Result", success);
...@@ -100,7 +100,11 @@ void SendMessageExpress::DoSendMessage( ...@@ -100,7 +100,11 @@ void SendMessageExpress::DoSendMessage(
SendMessageExpressRequest& request, SendMessageExpressRequest& request,
SuccessCallback callback, SuccessCallback callback,
const std::string& oauth_token) { const std::string& oauth_token) {
base::UmaHistogramBoolean(
"Nearby.Connections.InstantMessaging.SendExpress.OAuthTokenFetchResult",
!oauth_token.empty());
if (oauth_token.empty()) { if (oauth_token.empty()) {
NS_LOG(ERROR) << __func__ << ": Failed to fetch OAuth token.";
std::move(callback).Run(false); std::move(callback).Run(false);
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment