diff --git a/ipc/chromium/src/base/histogram.cc b/ipc/chromium/src/base/histogram.cc index 97b10e25e6e7..8cb68e61a653 100644 --- a/ipc/chromium/src/base/histogram.cc +++ b/ipc/chromium/src/base/histogram.cc @@ -436,7 +436,8 @@ Histogram::Histogram(const std::string& name, Sample minimum, bucket_count_(bucket_count), flags_(kNoFlags), ranges_(bucket_count + 1, 0), - range_checksum_(0) { + range_checksum_(0), + recording_enabled_(true) { Initialize(); } @@ -449,7 +450,8 @@ Histogram::Histogram(const std::string& name, TimeDelta minimum, bucket_count_(bucket_count), flags_(kNoFlags), ranges_(bucket_count + 1, 0), - range_checksum_(0) { + range_checksum_(0), + recording_enabled_(true) { Initialize(); } diff --git a/ipc/chromium/src/base/histogram.h b/ipc/chromium/src/base/histogram.h index 289206d30c06..9bd33f3b6916 100644 --- a/ipc/chromium/src/base/histogram.h +++ b/ipc/chromium/src/base/histogram.h @@ -41,6 +41,7 @@ #define BASE_METRICS_HISTOGRAM_H_ #pragma once +#include "mozilla/Atomics.h" #include "mozilla/MemoryReporting.h" #include @@ -405,6 +406,12 @@ class Histogram { void Add(int value); void Subtract(int value); + // TODO: Currently recording_enabled_ is not used by any Histogram class, but + // rather examined only by the telemetry code (via IsRecordingEnabled). + // Move handling to Histogram's Add() etc after simplifying Histogram. + void SetRecordingEnabled(bool aEnabled) { recording_enabled_ = aEnabled; }; + bool IsRecordingEnabled() const { return recording_enabled_; }; + // This method is an interface, used only by BooleanHistogram. virtual void AddBoolean(bool value); @@ -589,6 +596,9 @@ class Histogram { // have been corrupted. uint32_t range_checksum_; + // When false, new samples are completely ignored. + mozilla::Atomic recording_enabled_; + DISALLOW_COPY_AND_ASSIGN(Histogram); }; diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 4e338a442c33..8f568414ae2d 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -5058,6 +5058,17 @@ "kind": "count", "description": "a testing histogram; not meant to be touched" }, + "TELEMETRY_TEST_COUNT_INIT_NO_RECORD": { + "expires_in_version": "never", + "kind": "count", + "description": "a testing histogram; not meant to be touched - initially not recording" + }, + "TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD": { + "expires_in_version": "never", + "kind": "count", + "keyed": true, + "description": "a testing histogram; not meant to be touched - initially not recording" + }, "TELEMETRY_TEST_KEYED_FLAG": { "expires_in_version": "never", "kind": "flag", diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 9d1689a334ae..62dff73b199f 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -11,6 +11,7 @@ #include #include "mozilla/dom/ToJSValue.h" +#include "mozilla/Atomics.h" #include "mozilla/Attributes.h" #include "mozilla/DebugOnly.h" #include "mozilla/Likely.h" @@ -840,6 +841,9 @@ public: nsresult GetJSSnapshot(JSContext* cx, JS::Handle obj, bool subsession, bool clearSubsession); + void SetRecordingEnabled(bool aEnabled) { mRecordingEnabled = aEnabled; }; + bool IsRecordingEnabled() const { return mRecordingEnabled; }; + nsresult Add(const nsCString& key, uint32_t aSample); void Clear(bool subsession); @@ -862,6 +866,7 @@ private: const uint32_t mMax; const uint32_t mBucketCount; const uint32_t mDataset; + mozilla::Atomic mRecordingEnabled; }; // Hardcoded probes @@ -895,6 +900,30 @@ TelemetryHistogram::expiration() const return &gHistogramStringTable[this->expiration_offset]; } +bool +IsHistogramEnumId(Telemetry::ID aID) +{ + static_assert(((Telemetry::ID)-1 > 0), "ID should be unsigned."); + return aID < Telemetry::HistogramCount; +} + +// List of histogram IDs which should have recording disabled initially. +const Telemetry::ID kRecordingInitiallyDisabledIDs[] = { + + // The array must not be empty. Leave these item here. + Telemetry::TELEMETRY_TEST_COUNT_INIT_NO_RECORD, + Telemetry::TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD +}; + +void +InitHistogramRecordingEnabled() +{ + const size_t length = mozilla::ArrayLength(kRecordingInitiallyDisabledIDs); + for (size_t i = 0; i < length; i++) { + SetHistogramRecordingEnabled(kRecordingInitiallyDisabledIDs[i], false); + } +} + bool IsExpired(const char *expiration){ static Version current_version = Version(MOZ_APP_VERSION); @@ -1142,7 +1171,7 @@ nsresult HistogramAdd(Histogram& histogram, int32_t value, uint32_t dataset) { // Check if we are allowed to record the data. - if (!CanRecordDataset(dataset)) { + if (!CanRecordDataset(dataset) || !histogram.IsRecordingEnabled()) { return NS_OK; } @@ -1925,6 +1954,11 @@ mFailedLockCount(0) mKeyedHistograms.Put(id, new KeyedHistogram(id, expiration, h.histogramType, h.min, h.max, h.bucketCount, h.dataset)); } + + // Setup of the initial recording-enabled state (for not-recording-by-default + // histograms) using InitHistogramRecordingEnabled() will happen after instantiating + // sTelemetry since it depends on the static GetKeyedHistogramById(...) - which + // uses the singleton instance at sTelemetry. } TelemetryImpl::~TelemetryImpl() { @@ -2266,6 +2300,25 @@ TelemetryImpl::UnregisterAddonHistograms(const nsACString &id) return NS_OK; } +NS_IMETHODIMP +TelemetryImpl::SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled) +{ + Histogram *h; + nsresult rv = GetHistogramByName(id, &h); + if (NS_SUCCEEDED(rv)) { + h->SetRecordingEnabled(aEnabled); + return NS_OK; + } + + KeyedHistogram* keyed = GetKeyedHistogramById(id); + if (keyed) { + keyed->SetRecordingEnabled(aEnabled); + return NS_OK; + } + + return NS_ERROR_FAILURE; +} + nsresult TelemetryImpl::CreateHistogramSnapshots(JSContext *cx, JS::MutableHandle ret, @@ -3306,6 +3359,7 @@ TelemetryImpl::CreateTelemetryInstance() nsCOMPtr ret = sTelemetry; sTelemetry->InitMemoryReporter(); + InitHistogramRecordingEnabled(); // requires sTelemetry to exist return ret.forget(); } @@ -3796,6 +3850,34 @@ RecordShutdownEndTimeStamp() { namespace Telemetry { +// The external API for controlling recording state +void +SetHistogramRecordingEnabled(ID aID, bool aEnabled) +{ + if (!IsHistogramEnumId(aID)) { + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); + return; + } + + if (gHistograms[aID].keyed) { + const nsDependentCString id(gHistograms[aID].id()); + KeyedHistogram* keyed = TelemetryImpl::GetKeyedHistogramById(id); + if (keyed) { + keyed->SetRecordingEnabled(aEnabled); + return; + } + } else { + Histogram *h; + nsresult rv = GetHistogramByEnumId(aID, &h); + if (NS_SUCCEEDED(rv)) { + h->SetRecordingEnabled(aEnabled); + return; + } + } + + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); +} + void Accumulate(ID aHistogram, uint32_t aSample) { @@ -4282,6 +4364,7 @@ KeyedHistogram::KeyedHistogram(const nsACString &name, const nsACString &expirat , mMax(max) , mBucketCount(bucketCount) , mDataset(dataset) + , mRecordingEnabled(true) { } @@ -4369,6 +4452,10 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample) } #endif + if (!IsRecordingEnabled()) { + return NS_OK; + } + histogram->Add(sample); #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) subsession->Add(sample); diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index 9c7bc3b8c6d0..1b16d0ed743a 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -83,6 +83,16 @@ void Accumulate(const char *name, const nsCString& key, uint32_t sample = 1); */ void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end = TimeStamp::Now()); +/** + * Enable/disable recording for this histogram at runtime. + * Recording is enabled by default, unless listed at kRecordingInitiallyDisabledIDs[]. + * id must be a valid telemetry enum, otherwise an assertion is triggered. + * + * @param id - histogram id + * @param enabled - whether or not to enable recording from now on. + */ +void SetHistogramRecordingEnabled(ID id, bool enabled); + /** * Return a raw Histogram for direct manipulation for users who can not use Accumulate(). */ diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index 0c0a4db5ef11..e2b58b82a7c9 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -12,7 +12,7 @@ interface nsIFetchTelemetryDataCallback : nsISupports void complete(); }; -[scriptable, uuid(fabde631-c128-41c3-b7cb-9eb96f1276ff)] +[scriptable, uuid(273d2dd0-6c63-475a-b864-cb65160a1909)] interface nsITelemetry : nsISupports { /** @@ -327,6 +327,16 @@ interface nsITelemetry : nsISupports */ void unregisterAddonHistograms(in ACString addon_id); + /** + * Enable/disable recording for this histogram at runtime. + * Recording is enabled by default, unless listed at kRecordingInitiallyDisabledIDs[]. + * Name must be a valid Histogram identifier, otherwise an assertion will be triggered. + * + * @param id - unique identifier from histograms.json + * @param enabled - whether or not to enable recording from now on. + */ + void setHistogramRecordingEnabled(in ACString id, in boolean enabled); + /** * An object containing a snapshot from all of the currently * registered addon histograms. diff --git a/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js b/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js index 3544a086b632..3c57afd1e17d 100644 --- a/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js +++ b/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js @@ -638,6 +638,95 @@ function test_keyed_histogram_recording() { Assert.equal(h.snapshot(TEST_KEY).sum, 1); } +function test_histogram_recording_enabled() { + Telemetry.canRecordBase = true; + Telemetry.canRecordExtended = true; + + // Check that a "normal" histogram respects recording-enabled on/off + var h = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT"); + var orig = h.snapshot(); + + h.add(1); + Assert.equal(orig.sum + 1, h.snapshot().sum, + "add should record by default."); + + // Check that when recording is disabled - add is ignored + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT", false); + h.add(1); + Assert.equal(orig.sum + 1, h.snapshot().sum, + "When recording is disabled add should not record."); + + // Check that we're back to normal after recording is enabled + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT", true); + h.add(1); + Assert.equal(orig.sum + 2, h.snapshot().sum, + "When recording is re-enabled add should record."); + + // Check that a histogram with recording disabled by default behaves correctly + h = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT_INIT_NO_RECORD"); + orig = h.snapshot(); + + h.add(1); + Assert.equal(orig.sum, h.snapshot().sum, + "When recording is disabled by default, add should not record by default."); + + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT_INIT_NO_RECORD", true); + h.add(1); + Assert.equal(orig.sum + 1, h.snapshot().sum, + "When recording is enabled add should record."); + + // Restore to disabled + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT_INIT_NO_RECORD", false); + h.add(1); + Assert.equal(orig.sum + 1, h.snapshot().sum, + "When recording is disabled add should not record."); + +} + +function test_keyed_histogram_recording_enabled() { + Telemetry.canRecordBase = true; + Telemetry.canRecordExtended = true; + + // Check RecordingEnabled for keyed histograms which are recording by default + const TEST_KEY = "record_foo"; + h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT"); + + h.clear(); + h.add(TEST_KEY, 1); + Assert.equal(h.snapshot(TEST_KEY).sum, 1, + "Keyed histogram add should record by default"); + + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT", false); + h.add(TEST_KEY, 1); + Assert.equal(h.snapshot(TEST_KEY).sum, 1, + "Keyed histogram add should not record when recording is disabled"); + + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT", true); + h.clear(); + h.add(TEST_KEY, 1); + Assert.equal(h.snapshot(TEST_KEY).sum, 1, + "Keyed histogram add should record when recording is re-enabled"); + + // Check that a histogram with recording disabled by default behaves correctly + h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD"); + h.clear(); + + h.add(TEST_KEY, 1); + Assert.equal(h.snapshot(TEST_KEY).sum, 0, + "Keyed histogram add should not record by default for histograms which don't record by default"); + + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD", true); + h.add(TEST_KEY, 1); + Assert.equal(h.snapshot(TEST_KEY).sum, 1, + "Keyed histogram add should record when recording is enabled"); + + // Restore to disabled + Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD", false); + h.add(TEST_KEY, 1); + Assert.equal(h.snapshot(TEST_KEY).sum, 1, + "Keyed histogram add should not record when recording is disabled"); +} + function test_keyed_histogram() { // Check that invalid names get rejected. @@ -914,4 +1003,6 @@ function run_test() test_datasets(); test_subsession(); test_keyed_subsession(); + test_histogram_recording_enabled(); + test_keyed_histogram_recording_enabled(); }