Skip to content

Commit

Permalink
[metrics] Add on-demand metrics flush with notify
Browse files Browse the repository at this point in the history
Allows clients to flush the buffer for metrics data on-demand, e.g.
just before program termination.

bug: b/159458935
Test: Added unit test
Change-Id: I6bd287eb99fba4c153a96a22881efc4353264664
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478202
Commit-Queue: Hai Bi <bihai@google.com>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818698}
  • Loading branch information
Hai Bi authored and Commit Bot committed Oct 19, 2020
1 parent 10c203c commit d0224f9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
12 changes: 11 additions & 1 deletion fuchsia/base/legacymetrics_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ void LegacyMetricsClient::DrainBuffer() {

if (is_flushing_) {
metrics_recorder_.Unbind();
std::move(on_flush_complete_).Run();
} else {
ScheduleNextReport();
}
Expand Down Expand Up @@ -154,9 +155,14 @@ void LegacyMetricsClient::OnMetricsRecorderDisconnected(zx_status_t status) {
timer_.AbandonAndStop();
}

void LegacyMetricsClient::OnCloseSoon() {
void LegacyMetricsClient::FlushAndDisconnect(
base::OnceClosure on_flush_complete) {
DVLOG(1) << __func__ << " called.";
DCHECK(on_flush_complete);
if (is_flushing_)
return;

on_flush_complete_ = std::move(on_flush_complete);
timer_.AbandonAndStop();

is_flushing_ = true;
Expand All @@ -170,4 +176,8 @@ void LegacyMetricsClient::OnCloseSoon() {
}
}

void LegacyMetricsClient::OnCloseSoon() {
FlushAndDisconnect(base::DoNothing::Once());
}

} // namespace cr_fuchsia
7 changes: 7 additions & 0 deletions fuchsia/base/legacymetrics_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class LegacyMetricsClient {
// |callback| should be invoked to signal flush completion.
void SetNotifyFlushCallback(NotifyFlushCallback callback);

// Use when caller needs an explicit flush and then disconnect, such as before
// termination. Caller will be notified when all events in the buffer are
// sent.
void FlushAndDisconnect(base::OnceClosure on_flush_complete);

private:
void ScheduleNextReport();
void StartReport();
Expand All @@ -79,6 +84,8 @@ class LegacyMetricsClient {
base::RetainingOneShotTimer timer_;
SEQUENCE_CHECKER(sequence_checker_);

base::OnceClosure on_flush_complete_;

// Prevents use-after-free if |report_additional_callback_| is invoked after
// |this| is destroyed.
base::WeakPtrFactory<LegacyMetricsClient> weak_factory_{this};
Expand Down
42 changes: 42 additions & 0 deletions fuchsia/base/legacymetrics_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/fuchsia/scoped_service_binding.h"
#include "base/fuchsia/test_component_context_for_process.h"
#include "base/metrics/histogram_macros.h"
#include "base/test/bind_test_util.h"
#include "base/test/task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "fuchsia/base/legacymetrics_client.h"
Expand Down Expand Up @@ -317,5 +318,46 @@ TEST_F(LegacyMetricsClientTest, ExternalFlushSignal) {
EXPECT_TRUE(test_recorder_.IsRecordInFlight());
}

TEST_F(LegacyMetricsClientTest, ExplicitFlush) {
client_.Start(kReportInterval);

base::RecordComputedAction("bar");
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(test_recorder_.IsRecordInFlight());

bool called = false;
client_.FlushAndDisconnect(
base::BindLambdaForTesting([&called] { called = true; }));
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(test_recorder_.IsRecordInFlight());
EXPECT_FALSE(called);

auto events = test_recorder_.WaitForEvents();
EXPECT_EQ(1u, events.size());
EXPECT_EQ("bar", events[0].user_action_event().name());

test_recorder_.SendAck();
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(called);
}

TEST_F(LegacyMetricsClientTest, ExplicitFlushMultipleBatches) {
const size_t kSizeForMultipleBatches = LegacyMetricsClient::kMaxBatchSize * 2;
client_.Start(kReportInterval);

for (size_t i = 0; i < kSizeForMultipleBatches; ++i)
base::RecordComputedAction("bar");

client_.FlushAndDisconnect(base::DoNothing::Once());
base::RunLoop().RunUntilIdle();
test_recorder_.SendAck();
base::RunLoop().RunUntilIdle();

auto events = test_recorder_.WaitForEvents();
EXPECT_EQ(kSizeForMultipleBatches, events.size());
for (size_t i = 0; i < kSizeForMultipleBatches; ++i)
EXPECT_EQ("bar", events[i].user_action_event().name());
}

} // namespace
} // namespace cr_fuchsia

0 comments on commit d0224f9

Please sign in to comment.