Skip to content

Commit

Permalink
When Browser goes offline, add metrics on pending Aggregatable reports
Browse files Browse the repository at this point in the history 
Upon storing an Aggregatable Report at `creation_time`, it is scheduled with a`reporting_delay` to send at time `report_time` and is pending
until then.

If the browser closes or the device goes offline, and until it re-opens
or re-connects, we are unable to send reports. On such events, for each
pending reports, we record two {TimeSpan}s:
[1] TimeSinceCreation: The time elapsed since the report was created
(creation_time - now).
[2] TimeUntilReportTime: The time remaining for the report to be sent
(report_time - now).

The metrics will enable us to quantify the extent to which reducing
`reporting_delay` would increase the number of reports sent before an
unavailability event.

[Limitation] To limit memory usage, a maximum of 50 pending reports
timings are handled. Past this number, reports are dropped (until
reports get sent or an unavailability event occurs). Given that the most
recent reports can be dropped, we can expect the `TimeSinceCreation` to
be biased down slightly and `TimeUntilReportTime` to be biased up slightly. We expect this situation to be infrequent given the 10-60
minutes reporting window.

Bug: 1422713
Change-Id: If3e96b829e0b12916acdf0f59eece3a7177c9411
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4296410
Commit-Queue: Anthony Garant <anthonygarant@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115728}
  • Loading branch information
@agarant
agarant authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent 9470000 commit 6455453
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 0 deletions.
4 changes: 4 additions & 0 deletions content/browser/aggregation_service/report_scheduler_timer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ void ReportSchedulerTimer::OnConnectionChanged(

if (offline_) {
reporting_time_reached_timer_.Stop();
if (!was_offline) {
delegate_->OnReportingPaused(base::Time::Now());
}

} else if (was_offline) {
// Add delay to all reports that should have been sent while the browser was
// offline so they are not temporally joinable. We only need to do this if
Expand Down
6 changes: 6 additions & 0 deletions content/browser/aggregation_service/report_scheduler_timer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ class CONTENT_EXPORT ReportSchedulerTimer
// call, that will be passed the same `now`.
virtual void OnReportingTimeReached(base::Time now) = 0;

// Called when the connection changes from online to offline. When this
// happens the timer is paused which means `OnReportingTimeReached` will not
// be called until it gets resumed. Before resuming the timer,
// `AdjustOfflinereportTimes` will be called.
virtual void OnReportingPaused(base::Time now) {}

// Called when the connection changes from offline to online. May also be
// called on a connection change if there are no stored reports, see
// `OnConnectionChanged()`. Running the callback will call `MaybeSet()` with
Expand Down
3 changes: 3 additions & 0 deletions content/browser/aggregation_service/report_scheduler_timer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class MockReportSchedulerTimerDelegate : public ReportSchedulerTimer::Delegate {
AdjustOfflineReportTimes,
(base::OnceCallback<void(absl::optional<base::Time>)>),
(override));

MOCK_METHOD(void, OnReportingPaused, (base::Time), (override));
};

class ReportSchedulerTimerTest : public testing::Test {
Expand Down Expand Up @@ -144,6 +146,7 @@ TEST_F(ReportSchedulerTimerTest, NetworkChange) {
InSequence seq;

EXPECT_CALL(*timer_delegate_, OnReportingTimeReached).Times(0);
EXPECT_CALL(*timer_delegate_, OnReportingPaused).Times(1);
EXPECT_CALL(checkpoint, Call(1));
EXPECT_CALL(*timer_delegate_, AdjustOfflineReportTimes);
}
Expand Down
55 changes: 55 additions & 0 deletions content/browser/attribution_reporting/attribution_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "content/browser/attribution_reporting/attribution_report.h"
#include "content/browser/attribution_reporting/attribution_report_network_sender.h"
#include "content/browser/attribution_reporting/attribution_report_sender.h"
#include "content/browser/attribution_reporting/attribution_reporting.mojom-shared.h"
#include "content/browser/attribution_reporting/attribution_storage.h"
#include "content/browser/attribution_reporting/attribution_storage_delegate.h"
#include "content/browser/attribution_reporting/attribution_storage_delegate_impl.h"
Expand Down Expand Up @@ -93,6 +94,8 @@ namespace {
using ScopedUseInMemoryStorageForTesting =
::content::AttributionManagerImpl::ScopedUseInMemoryStorageForTesting;

using ReportType = attribution_reporting::mojom::ReportType;

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class ConversionReportSendOutcome {
Expand All @@ -117,8 +120,10 @@ class AttributionReportScheduler : public ReportSchedulerTimer::Delegate {
public:
AttributionReportScheduler(
base::RepeatingClosure send_reports,
base::RepeatingClosure on_reporting_paused_cb,
base::SequenceBound<AttributionStorage>& attribution_storage)
: send_reports_(std::move(send_reports)),
on_reporting_paused_cb_(std::move(on_reporting_paused_cb)),
attribution_storage_(attribution_storage) {}
~AttributionReportScheduler() override = default;

Expand Down Expand Up @@ -150,7 +155,12 @@ class AttributionReportScheduler : public ReportSchedulerTimer::Delegate {
.Then(std::move(maybe_set_timer_cb));
}

void OnReportingPaused(base::Time now) override {
on_reporting_paused_cb_.Run();
}

base::RepeatingClosure send_reports_;
base::RepeatingClosure on_reporting_paused_cb_;
const raw_ref<base::SequenceBound<AttributionStorage>> attribution_storage_;
};

Expand Down Expand Up @@ -343,6 +353,11 @@ bool g_run_in_memory = false;

} // namespace

struct AttributionManagerImpl::PendingReportTimings {
base::Time creation_time;
base::Time report_time;
};

BASE_FEATURE(kAttributionVerboseDebugReporting,
"AttributionVerboseDebugReporting",
base::FEATURE_ENABLED_BY_DEFAULT);
Expand Down Expand Up @@ -464,6 +479,9 @@ AttributionManagerImpl::AttributionManagerImpl(
scheduler_timer_(std::make_unique<AttributionReportScheduler>(
base::BindRepeating(&AttributionManagerImpl::GetReportsToSend,
base::Unretained(this)),
base::BindRepeating(
&AttributionManagerImpl::RecordPendingAggregatableReportsTimings,
base::Unretained(this)),
attribution_storage_)),
data_host_manager_(std::move(data_host_manager)),
special_storage_policy_(std::move(special_storage_policy)),
Expand All @@ -485,6 +503,8 @@ AttributionManagerImpl::AttributionManagerImpl(
}

AttributionManagerImpl::~AttributionManagerImpl() {
RecordPendingAggregatableReportsTimings();

// Browser contexts are not required to have a special storage policy.
if (!special_storage_policy_ ||
!special_storage_policy_->HasSessionOnlyOrigins()) {
Expand Down Expand Up @@ -548,6 +568,22 @@ void AttributionManagerImpl::StoreSource(
cleared_debug_key, is_debug_cookie_set));
}

void AttributionManagerImpl::RecordPendingAggregatableReportsTimings() {
const base::Time now = base::Time::Now();

for (const auto& [key, timing] : pending_aggregatable_reports_) {
UMA_HISTOGRAM_LONG_TIMES(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeSinceCreation",
now - timing.creation_time);
UMA_HISTOGRAM_LONG_TIMES(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeUntilReportTime",
timing.report_time - now);
}
pending_aggregatable_reports_.clear();
}

void AttributionManagerImpl::OnSourceStored(
const StorableSource& source,
absl::optional<uint64_t> cleared_debug_key,
Expand Down Expand Up @@ -695,6 +731,20 @@ void AttributionManagerImpl::ProcessNextEvent(bool is_debug_cookie_set) {
std::move(event));
}

void AttributionManagerImpl::AddPendingAggregatableReportTiming(
const AttributionReport::Id& id,
const base::Time& report_time) {
// The maximum number of pending reports that should be considered. Past this
// value, events will be ignored.
constexpr size_t kMaxPendingReportsTimings = 50;
if (pending_aggregatable_reports_.size() >= kMaxPendingReportsTimings) {
return;
}

pending_aggregatable_reports_[id] = {.creation_time = base::Time::Now(),
.report_time = report_time};
}

void AttributionManagerImpl::OnReportStored(
const AttributionTrigger& trigger,
absl::optional<uint64_t> cleared_debug_key,
Expand All @@ -712,6 +762,10 @@ void AttributionManagerImpl::OnReportStored(
if (auto& report = result.new_aggregatable_report()) {
min_new_report_time = AttributionReport::MinReportTime(
min_new_report_time, report->report_time());

AddPendingAggregatableReportTiming(report->ReportId(),
report->report_time());

MaybeSendDebugReport(std::move(*report));
}

Expand Down Expand Up @@ -912,6 +966,7 @@ void AttributionManagerImpl::SendReports(
}

if (!web_ui_callback) {
pending_aggregatable_reports_.erase(report.ReportId());
LogMetricsOnReportSend(report, now);
}

Expand Down
14 changes: 14 additions & 0 deletions content/browser/attribution_reporting/attribution_manager_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <vector>

#include "base/containers/circular_deque.h"
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/feature_list.h"
#include "base/functional/callback_forward.h"
Expand All @@ -24,6 +25,7 @@
#include "build/buildflag.h"
#include "content/browser/aggregation_service/aggregation_service.h"
#include "content/browser/aggregation_service/report_scheduler_timer.h"
#include "content/browser/attribution_reporting/attribution_internals.mojom-forward.h"
#include "content/browser/attribution_reporting/attribution_manager.h"
#include "content/browser/attribution_reporting/attribution_report.h"
#include "content/browser/attribution_reporting/attribution_report_sender.h"
Expand Down Expand Up @@ -187,6 +189,8 @@ class CONTENT_EXPORT AttributionManagerImpl : public AttributionManager {
using ReportSentCallback = AttributionReportSender::ReportSentCallback;
using SourceOrTrigger = absl::variant<StorableSource, AttributionTrigger>;

struct PendingReportTimings;

AttributionManagerImpl(
StoragePartitionImpl* storage_partition,
const base::FilePath& user_data_directory,
Expand Down Expand Up @@ -261,6 +265,10 @@ class CONTENT_EXPORT AttributionManagerImpl : public AttributionManager {
bool is_debug_cookie_set,
const CreateReportResult& result);

void AddPendingAggregatableReportTiming(const AttributionReport::Id& id,
const base::Time& report_time);
void RecordPendingAggregatableReportsTimings();

void OnClearDataComplete();

#if BUILDFLAG(IS_ANDROID)
Expand Down Expand Up @@ -317,6 +325,12 @@ class CONTENT_EXPORT AttributionManagerImpl : public AttributionManager {
// is expected to be small, so a `flat_set` is used.
base::flat_set<AttributionReport::Id> reports_being_sent_;

// We keep track of pending reports timings in memory to recordd metrics
// when the browser becomes unavailable to send reports due to becoming
// offline or being shutdown.
base::flat_map<AttributionReport::Id, PendingReportTimings>
pending_aggregatable_reports_;

base::ObserverList<AttributionObserver> observers_;

#if BUILDFLAG(IS_ANDROID)
Expand Down
141 changes: 141 additions & 0 deletions content/browser/attribution_reporting/attribution_manager_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/memory/scoped_refptr.h"
#include "base/run_loop.h"
#include "base/scoped_observation.h"
#include "base/strings/string_util.h"
#include "base/task/task_traits.h"
#include "base/task/thread_pool.h"
#include "base/task/updateable_sequenced_task_runner.h"
Expand Down Expand Up @@ -110,6 +111,8 @@ using ReportSentCallback =
::content::AttributionReportSender::ReportSentCallback;

constexpr size_t kMaxPendingEvents = 5;
constexpr size_t kMaxPendingReportsTimings = 50;

const GlobalRenderFrameHostId kFrameId = {0, 1};

constexpr AttributionStorageDelegate::OfflineReportDelayConfig
Expand Down Expand Up @@ -331,6 +334,24 @@ class AttributionManagerImplTest : public testing::Test {
partition->OverrideAggregationServiceForTesting(nullptr);
}

void RegisterAggregatableSourceAndMatchingTrigger(
const std::string& origin_prefix) {
const auto origin = *SuitableOrigin::Create(GURL(base::JoinString(
{"https://", origin_prefix,
".example/.well-known/attribution-reporting/report-event-attribution"},
"")));

attribution_manager_->HandleSource(TestAggregatableSourceProvider()
.GetBuilder()
.SetExpiry(kImpressionExpiry)
.SetReportingOrigin(origin)
.Build(),
kFrameId);
attribution_manager_->HandleTrigger(
DefaultAggregatableTriggerBuilder().SetReportingOrigin(origin).Build(),
kFrameId);
}

#if BUILDFLAG(IS_ANDROID)
void OverrideOsLevelManager(
std::unique_ptr<AttributionOsLevelManager> os_level_manager) {
Expand Down Expand Up @@ -2679,6 +2700,126 @@ TEST_F(AttributionManagerImplTest,
task_environment_.RunUntilIdle();
}

TEST_F(AttributionManagerImplTest, PendingReportsMetrics) {
base::HistogramTester histograms;

RegisterAggregatableSourceAndMatchingTrigger("a");
task_environment_.FastForwardBy(base::Seconds(10));

RegisterAggregatableSourceAndMatchingTrigger("b");
task_environment_.FastForwardBy(base::Seconds(20));

RegisterAggregatableSourceAndMatchingTrigger("c");
task_environment_.FastForwardBy(base::Seconds(40));

ShutdownManager();

histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeSinceCreation",
3);
EXPECT_EQ(
histograms.GetTotalSum("Conversions.AggregatableReport."
"PendingAndBrowserWentOffline.TimeSinceCreation"),
base::Seconds(70 + 60 + 40).InMilliseconds());

histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeUntilReportTime",
3);
EXPECT_EQ(histograms.GetTotalSum(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeUntilReportTime"),
((kFirstReportingWindow - base::Seconds(70)) +
(kFirstReportingWindow - base::Seconds(60)) +
(kFirstReportingWindow - base::Seconds(40)))
.InMilliseconds());
}

TEST_F(AttributionManagerImplTest,
PendingReportsMetrics_WithoutPendingReports) {
base::HistogramTester histograms;

RegisterAggregatableSourceAndMatchingTrigger("a");
task_environment_.FastForwardBy(base::Seconds(10));

// Advancing time enough for reports to send
task_environment_.FastForwardBy(kFirstReportingWindow);

ShutdownManager();

// Expect no histograms on shutdown as the report have already been sent.
histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeSinceCreation",
0);
histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeUntilReportTime",
0);
}

TEST_F(AttributionManagerImplTest, PendingReportsMetrics_Offline) {
base::HistogramTester histograms;

RegisterAggregatableSourceAndMatchingTrigger("a");
task_environment_.FastForwardBy(base::Seconds(10));

SetConnectionTypeAndWaitForObserversToBeNotified(
network::mojom::ConnectionType::CONNECTION_NONE);
SetConnectionTypeAndWaitForObserversToBeNotified(
network::mojom::ConnectionType::CONNECTION_WIFI);

RegisterAggregatableSourceAndMatchingTrigger("b");
task_environment_.FastForwardBy(base::Seconds(20));

RegisterAggregatableSourceAndMatchingTrigger("c");
task_environment_.FastForwardBy(base::Seconds(40));

task_environment_.FastForwardBy(kFirstReportingWindow);

ShutdownManager();

// Expect only one histogram as there was only one pending report when it
// first went offline.
histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeSinceCreation",
1);
histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeUntilReportTime",
1);
}

TEST_F(AttributionManagerImplTest, PendingReportsMetrics_OverLimits) {
base::HistogramTester histograms;

for (size_t i = 0; i < (kMaxPendingReportsTimings + 5); i++) {
RegisterAggregatableSourceAndMatchingTrigger(base::NumberToString(i));
}

task_environment_.FastForwardBy(base::Seconds(10));
RegisterAggregatableSourceAndMatchingTrigger("a");

ShutdownManager();

// Expect that events registered past the limit should be dropped.
histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeSinceCreation",
kMaxPendingReportsTimings);
histograms.ExpectTotalCount(
"Conversions.AggregatableReport.PendingAndBrowserWentOffline."
"TimeUntilReportTime",
kMaxPendingReportsTimings);

EXPECT_EQ(
histograms.GetTotalSum("Conversions.AggregatableReport."
"PendingAndBrowserWentOffline.TimeSinceCreation"),
(base::Seconds(10) * kMaxPendingReportsTimings).InMilliseconds());
}

class AttributionManagerImplDebugReportTest
: public AttributionManagerImplTest {
protected:
Expand Down

0 comments on commit 6455453

Please sign in to comment.