Skip to content

Commit

Permalink
Add UMA Histogram Manager to Cronet.
Browse files Browse the repository at this point in the history
Part of change to collect, upload and expose histogram data
for Cronet.

BUG=441466

Committed: https://crrev.com/4eb0fd47de0736f3f4c970e8d7f8813ef2177886
Cr-Commit-Position: refs/heads/master@{#308239}

Review URL: https://codereview.chromium.org/794493002

Cr-Commit-Position: refs/heads/master@{#308427}
  • Loading branch information
rtenneti authored and Commit bot committed Dec 15, 2014
1 parent eda4efb commit 7254696
Show file tree
Hide file tree
Showing 19 changed files with 481 additions and 30 deletions.
2 changes: 2 additions & 0 deletions components/components_tests.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@
'login/screens/screen_context_unittest.cc',
'metrics/compression_utils_unittest.cc',
'metrics/daily_event_unittest.cc',
'metrics/histogram_encoder_unittest.cc',
'metrics/histogram_manager_unittest.cc',
'metrics/machine_id_provider_win_unittest.cc',
'metrics/metrics_hashes_unittest.cc',
'metrics/metrics_log_manager_unittest.cc',
Expand Down
6 changes: 6 additions & 0 deletions components/cronet.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java',
'cronet/android/java/src/org/chromium/net/ChromiumUrlRequest.java',
'cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java',
'cronet/android/java/src/org/chromium/net/HistogramManager.java',
],
'variables': {
'jni_gen_package': 'cronet',
Expand Down Expand Up @@ -101,12 +102,14 @@
'dependencies': [
'../base/base.gyp:base',
'../base/base.gyp:base_i18n',
'../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
'../third_party/icu/icu.gyp:icui18n',
'../third_party/icu/icu.gyp:icuuc',
'cronet_jni_headers',
'cronet_url_request_context_config_list',
'cronet_url_request_java',
'cronet_version',
'metrics',
'../net/net.gyp:net',
],
'sources': [
Expand All @@ -129,6 +132,8 @@
'cronet/android/cronet_url_request_context.h',
'cronet/android/cronet_url_request_context_adapter.cc',
'cronet/android/cronet_url_request_context_adapter.h',
'cronet/android/histogram_manager.cc',
'cronet/android/histogram_manager.h',
'cronet/android/url_request_adapter.cc',
'cronet/android/url_request_adapter.h',
'cronet/android/url_request_context_adapter.cc',
Expand Down Expand Up @@ -229,6 +234,7 @@
'**/CronetUrlRequest.java',
'**/CronetUrlRequestContext.java',
'**/CronetUrlRequestFactory.java',
'**/HistogramManager.java',
'**/RequestPriority.java',
'**/urlconnection/CronetInputStream.java',
'**/urlconnection/CronetHttpURLConnection.java',
Expand Down
1 change: 1 addition & 0 deletions components/cronet/android/DEPS
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
include_rules = [
"+components/metrics",
"+jni",
]
2 changes: 2 additions & 0 deletions components/cronet/android/cronet_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "components/cronet/android/chromium_url_request_context.h"
#include "components/cronet/android/cronet_url_request.h"
#include "components/cronet/android/cronet_url_request_context.h"
#include "components/cronet/android/histogram_manager.h"
#include "net/android/net_jni_registrar.h"
#include "url/android/url_jni_registrar.h"
#include "url/url_util.h"
Expand All @@ -27,6 +28,7 @@ const base::android::RegistrationMethod kCronetRegisteredMethods[] = {
{"ChromiumUrlRequestContext", cronet::ChromiumUrlRequestContextRegisterJni},
{"CronetUrlRequest", cronet::CronetUrlRequestRegisterJni},
{"CronetUrlRequestContext", cronet::CronetUrlRequestContextRegisterJni},
{"HistogramManager", cronet::HistogramManagerRegisterJni},
{"NetAndroid", net::android::RegisterJni},
{"UrlAndroid", url::android::RegisterJni},
};
Expand Down
29 changes: 29 additions & 0 deletions components/cronet/android/histogram_manager.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/cronet/android/histogram_manager.h"

#include <string>
#include <vector>

#include "base/android/jni_array.h"
#include "components/metrics/histogram_manager.h"

#include "jni/HistogramManager_jni.h"

namespace cronet {

// Explicitly register static JNI functions.
bool HistogramManagerRegisterJni(JNIEnv* env) {
return RegisterNativesImpl(env);
}

static jbyteArray GetHistogramDeltas(JNIEnv* env, jobject jcaller) {
std::vector<uint8> data;
if (!metrics::HistogramManager::GetInstance()->GetDeltas(&data))
return NULL;
return base::android::ToJavaByteArray(env, &data[0], data.size()).Release();
}

} // namespace cronet
16 changes: 16 additions & 0 deletions components/cronet/android/histogram_manager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
#define COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_

#include <jni.h>

namespace cronet {

bool HistogramManagerRegisterJni(JNIEnv* env);

} // namespace cronet

#endif // COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.net;

import org.chromium.base.JNINamespace;

/**
* Controls UMA histograms.
*/
@JNINamespace("cronet")
public final class HistogramManager {
public HistogramManager() {
}

/**
* Get histogram deltas serialized as protobuf.
*/
public byte[] getHistogramDeltas() {
return nativeGetHistogramDeltas();
}

private native byte[] nativeGetHistogramDeltas();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.cronet_test_apk;

import android.test.suitebuilder.annotation.SmallTest;

import org.chromium.base.test.util.Feature;
import org.chromium.net.UrlRequest;

/**
* Test HistogramManager.
*/
public class HistogramManagerTest extends CronetTestBase {
// URLs used for tests.
private static final String TEST_URL = "http://127.0.0.1:8000";

CronetTestActivity mActivity;

@SmallTest
@Feature({"Cronet"})
public void testHistogramManager() throws Exception {
mActivity = launchCronetTestApp();
byte delta1[] = mActivity.mHistogramManager.getHistogramDeltas();

TestUrlRequestListener listener = new TestUrlRequestListener();
UrlRequest urlRequest = mActivity.mUrlRequestContext.createRequest(
TEST_URL, listener, listener.getExecutor());
urlRequest.start();
listener.blockForDone();
byte delta2[] = mActivity.mHistogramManager.getHistogramDeltas();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import android.os.Environment;
import android.util.Log;

import org.chromium.net.HistogramManager;
import org.chromium.net.HttpUrlRequest;
import org.chromium.net.HttpUrlRequestFactory;
import org.chromium.net.HttpUrlRequestListener;
Expand Down Expand Up @@ -39,6 +40,8 @@ public class CronetTestActivity extends Activity {

HttpUrlRequestFactory mRequestFactory;
UrlRequestContext mUrlRequestContext;
HistogramManager mHistogramManager = new HistogramManager();

String mUrl;

boolean mLoading = false;
Expand Down
4 changes: 4 additions & 0 deletions components/metrics.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
'metrics/compression_utils.h',
'metrics/daily_event.cc',
'metrics/daily_event.h',
'metrics/histogram_encoder.cc',
'metrics/histogram_encoder.h',
'metrics/histogram_manager.cc',
'metrics/histogram_manager.h',
'metrics/machine_id_provider.h',
'metrics/machine_id_provider_stub.cc',
'metrics/machine_id_provider_win.cc',
Expand Down
6 changes: 6 additions & 0 deletions components/metrics/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ source_set("metrics") {
"compression_utils.h",
"daily_event.cc",
"daily_event.h",
"histogram_encoder.cc",
"histogram_encoder.h",
"histogram_manager.cc",
"histogram_manager.h",
"machine_id_provider.h",
"machine_id_provider_stub.cc",
"machine_id_provider_win.cc",
Expand Down Expand Up @@ -159,6 +163,8 @@ source_set("unit_tests") {
sources = [
"compression_utils_unittest.cc",
"daily_event_unittest.cc",
"histogram_encoder_unittest.cc",
"histogram_manager_unittest.cc",
"machine_id_provider_win_unittest.cc",
"metrics_hashes_unittest.cc",
"metrics_log_manager_unittest.cc",
Expand Down
55 changes: 55 additions & 0 deletions components/metrics/histogram_encoder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/metrics/histogram_encoder.h"

#include <string>

#include "base/basictypes.h"
#include "base/memory/scoped_ptr.h"
#include "base/metrics/histogram.h"
#include "base/metrics/histogram_samples.h"
#include "components/metrics/metrics_hashes.h"

using base::SampleCountIterator;

namespace metrics {

void EncodeHistogramDelta(const std::string& histogram_name,
const base::HistogramSamples& snapshot,
ChromeUserMetricsExtension* uma_proto) {
DCHECK_NE(0, snapshot.TotalCount());
DCHECK(uma_proto);

// We will ignore the MAX_INT/infinite value in the last element of range[].

HistogramEventProto* histogram_proto = uma_proto->add_histogram_event();
histogram_proto->set_name_hash(HashMetricName(histogram_name));
histogram_proto->set_sum(snapshot.sum());

for (scoped_ptr<SampleCountIterator> it = snapshot.Iterator(); !it->Done();
it->Next()) {
base::Histogram::Sample min;
base::Histogram::Sample max;
base::Histogram::Count count;
it->Get(&min, &max, &count);
HistogramEventProto::Bucket* bucket = histogram_proto->add_bucket();
bucket->set_min(min);
bucket->set_max(max);
bucket->set_count(count);
}

// Omit fields to save space (see rules in histogram_event.proto comments).
for (int i = 0; i < histogram_proto->bucket_size(); ++i) {
HistogramEventProto::Bucket* bucket = histogram_proto->mutable_bucket(i);
if (i + 1 < histogram_proto->bucket_size() &&
bucket->max() == histogram_proto->bucket(i + 1).min()) {
bucket->clear_max();
} else if (bucket->max() == bucket->min() + 1) {
bucket->clear_min();
}
}
}

} // namespace metrics
30 changes: 30 additions & 0 deletions components/metrics/histogram_encoder.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// This file defines an utility function that records any changes in a given
// histogram for transmission.

#ifndef COMPONENTS_METRICS_HISTOGRAM_ENCODER_H_
#define COMPONENTS_METRICS_HISTOGRAM_ENCODER_H_

#include <string>

#include "base/basictypes.h"
#include "components/metrics/proto/chrome_user_metrics_extension.pb.h"

namespace base {
class HistogramSamples;
}

namespace metrics {

// Record any changes (histogram deltas of counts from |snapshot|) into
// |uma_proto| for the given histogram (|histogram_name|).
void EncodeHistogramDelta(const std::string& histogram_name,
const base::HistogramSamples& snapshot,
ChromeUserMetricsExtension* uma_proto);

} // namespace metrics

#endif // COMPONENTS_METRICS_HISTOGRAM_ENCODER_H_
72 changes: 72 additions & 0 deletions components/metrics/histogram_encoder_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/metrics/histogram_encoder.h"

#include <string>

#include "base/basictypes.h"
#include "base/metrics/bucket_ranges.h"
#include "base/metrics/sample_vector.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace metrics {

TEST(HistogramEncoder, HistogramBucketFields) {
// Create buckets: 1-5, 5-7, 7-8, 8-9, 9-10, 10-11, 11-12.
base::BucketRanges ranges(8);
ranges.set_range(0, 1);
ranges.set_range(1, 5);
ranges.set_range(2, 7);
ranges.set_range(3, 8);
ranges.set_range(4, 9);
ranges.set_range(5, 10);
ranges.set_range(6, 11);
ranges.set_range(7, 12);

base::SampleVector samples(&ranges);
samples.Accumulate(3, 1); // Bucket 1-5.
samples.Accumulate(6, 1); // Bucket 5-7.
samples.Accumulate(8, 1); // Bucket 8-9. (7-8 skipped)
samples.Accumulate(10, 1); // Bucket 10-11. (9-10 skipped)
samples.Accumulate(11, 1); // Bucket 11-12.

ChromeUserMetricsExtension uma_proto;
EncodeHistogramDelta("Test", samples, &uma_proto);

const HistogramEventProto& histogram_proto =
uma_proto.histogram_event(uma_proto.histogram_event_size() - 1);

// Buckets with samples: 1-5, 5-7, 8-9, 10-11, 11-12.
// Should become: 1-/, 5-7, /-9, 10-/, /-12.
ASSERT_EQ(5, histogram_proto.bucket_size());

// 1-5 becomes 1-/ (max is same as next min).
EXPECT_TRUE(histogram_proto.bucket(0).has_min());
EXPECT_FALSE(histogram_proto.bucket(0).has_max());
EXPECT_EQ(1, histogram_proto.bucket(0).min());

// 5-7 stays 5-7 (no optimization possible).
EXPECT_TRUE(histogram_proto.bucket(1).has_min());
EXPECT_TRUE(histogram_proto.bucket(1).has_max());
EXPECT_EQ(5, histogram_proto.bucket(1).min());
EXPECT_EQ(7, histogram_proto.bucket(1).max());

// 8-9 becomes /-9 (min is same as max - 1).
EXPECT_FALSE(histogram_proto.bucket(2).has_min());
EXPECT_TRUE(histogram_proto.bucket(2).has_max());
EXPECT_EQ(9, histogram_proto.bucket(2).max());

// 10-11 becomes 10-/ (both optimizations apply, omit max is prioritized).
EXPECT_TRUE(histogram_proto.bucket(3).has_min());
EXPECT_FALSE(histogram_proto.bucket(3).has_max());
EXPECT_EQ(10, histogram_proto.bucket(3).min());

// 11-12 becomes /-12 (last record must keep max, min is same as max - 1).
EXPECT_FALSE(histogram_proto.bucket(4).has_min());
EXPECT_TRUE(histogram_proto.bucket(4).has_max());
EXPECT_EQ(12, histogram_proto.bucket(4).max());
}

} // namespace metrics
Loading

0 comments on commit 7254696

Please sign in to comment.