Skip to content

Commit

Permalink
P3A: Remove alternate log store.
Browse files Browse the repository at this point in the history
Now that we only have one message format, replace the parallel
log stores with a single one.

Follow-up to brave/brave-browser#15967
  • Loading branch information
rillian committed Jul 29, 2022
1 parent 09c511e commit f4a189c
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 38 deletions.
10 changes: 1 addition & 9 deletions components/p3a/brave_p3a_log_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,7 @@ const std::string& BraveP3ALogStore::staged_log() const {
auto iter = log_.find(staged_entry_key_);
DCHECK(iter != log_.end());

return staged_log_.legacy_log;
}

const std::string& BraveP3ALogStore::staged_json_log() const {
DCHECK(!staged_entry_key_.empty());
auto iter = log_.find(staged_entry_key_);
DCHECK(iter != log_.end());

return staged_log_.json_log;
return staged_log_;
}

std::string BraveP3ALogStore::staged_log_type() const {
Expand Down
18 changes: 3 additions & 15 deletions components/p3a/brave_p3a_log_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,11 @@ namespace brave {
// for now persisted entries never expire.
class BraveP3ALogStore : public metrics::LogStore {
public:
struct LogForJsonMigration {
std::string legacy_log;
std::string json_log;

void clear() {
legacy_log.clear();
json_log.clear();
}
};

class Delegate {
public:
// Prepares a string representaion of an entry.
virtual LogForJsonMigration Serialize(base::StringPiece histogram_name,
uint64_t value) = 0;
virtual std::string Serialize(base::StringPiece histogram_name,
uint64_t value) = 0;
// Returns false if the metric is obsolete and should be cleaned up.
virtual bool IsActualMetric(base::StringPiece histogram_name) const = 0;
virtual ~Delegate() {}
Expand All @@ -63,8 +53,6 @@ class BraveP3ALogStore : public metrics::LogStore {
bool has_unsent_logs() const override;
bool has_staged_log() const override;
const std::string& staged_log() const override;
// This will replace `staged_log` once we migrate to JSON pings.
const std::string& staged_json_log() const;
std::string staged_log_type() const;
const std::string& staged_log_hash() const override;
const std::string& staged_log_signature() const override;
Expand Down Expand Up @@ -105,7 +93,7 @@ class BraveP3ALogStore : public metrics::LogStore {
base::flat_set<std::string> unsent_entries_;

std::string staged_entry_key_;
LogForJsonMigration staged_log_;
std::string staged_log_;

// Not used for now.
std::string staged_log_hash_;
Expand Down
15 changes: 4 additions & 11 deletions components/p3a/brave_p3a_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/json/json_writer.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/histogram_samples.h"
#include "base/metrics/metrics_hashes.h"
#include "base/metrics/sample_vector.h"
#include "base/metrics/statistics_recorder.h"
#include "base/no_destructor.h"
Expand Down Expand Up @@ -180,27 +179,21 @@ void BraveP3AService::Init(
}
}

BraveP3ALogStore::LogForJsonMigration BraveP3AService::Serialize(
base::StringPiece histogram_name,
uint64_t value) {
std::string BraveP3AService::Serialize(base::StringPiece histogram_name,
uint64_t value) {
// TRACE_EVENT0("brave_p3a", "SerializeMessage");
// TODO(iefremov): Maybe we should store it in logs and pass here?
// We cannot directly query |base::StatisticsRecorder::FindHistogram| because
// the serialized value can be obtained from persisted log storage at the
// point when the actual histogram is not ready yet.
const uint64_t histogram_name_hash = base::HashMetricName(histogram_name);

UpdateMessageMeta();
brave_pyxis::RawP3AValue message;
GenerateP3AMessage(histogram_name_hash, value, message_meta_, &message);

base::Value p3a_json_value =
GenerateP3AMessageDict(histogram_name, value, message_meta_);
std::string p3a_json_message;
const bool ok = base::JSONWriter::Write(p3a_json_value, &p3a_json_message);
DCHECK(ok);

return {message.SerializeAsString(), p3a_json_message};
return p3a_json_message;
}

bool
Expand Down Expand Up @@ -297,7 +290,7 @@ void BraveP3AService::StartScheduledUpload() {
const std::string log_type = log_store_->staged_log_type();
VLOG(2) << "StartScheduledUpload - Uploading " << log.size() << " bytes "
<< "of type " << log_type;
uploader_->UploadLog(log_store_->staged_json_log(), log_type);
uploader_->UploadLog(log_store_->staged_log(), log_type);
}
}

Expand Down
5 changes: 2 additions & 3 deletions components/p3a/brave_p3a_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ class BraveP3AService : public base::RefCountedThreadSafe<BraveP3AService>,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);

// BraveP3ALogStore::Delegate
BraveP3ALogStore::LogForJsonMigration Serialize(
base::StringPiece histogram_name,
uint64_t value) override;
std::string Serialize(base::StringPiece histogram_name,
uint64_t value) override;

// May be accessed from multiple threads, so this is thread-safe.
bool IsActualMetric(base::StringPiece histogram_name) const override;
Expand Down

0 comments on commit f4a189c

Please sign in to comment.