Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(584)

Side by Side Diff: components/browser_watcher/watcher_metrics_provider_win.cc

Issue 868163002: Only report exit funnels for canary and dev channels. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address Alexei's comments. Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2014 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/browser_watcher/watcher_metrics_provider_win.h" 5 #include "components/browser_watcher/watcher_metrics_provider_win.h"
6 6
7 #include <limits> 7 #include <limits>
8 #include <vector> 8 #include <vector>
9 9
10 #include "base/metrics/sparse_histogram.h" 10 #include "base/metrics/sparse_histogram.h"
(...skipping 127 matching lines...) Expand 10 before | Expand all | Expand 10 after
138 LONG res = regkey.DeleteValue(event_name.c_str()); 138 LONG res = regkey.DeleteValue(event_name.c_str());
139 if (res != ERROR_SUCCESS) { 139 if (res != ERROR_SUCCESS) {
140 LOG(ERROR) << "Failed to delete value " << event_name; 140 LOG(ERROR) << "Failed to delete value " << event_name;
141 return; 141 return;
142 } 142 }
143 } 143 }
144 144
145 events_out->swap(events); 145 events_out->swap(events);
146 } 146 }
147 147
148 void RecordSingleExitFunnel(base::win::RegKey* parent_key, 148 void MaybeRecordSingleExitFunnel(base::win::RegKey* parent_key,
149 const base::char16* name) { 149 const base::char16* name,
150 bool report) {
150 std::vector<std::pair<base::string16, int64>> events; 151 std::vector<std::pair<base::string16, int64>> events;
151 ReadSingleExitFunnel(parent_key, name, &events); 152 ReadSingleExitFunnel(parent_key, name, &events);
153 if (!report)
154 return;
152 155
153 // Find the earliest event time. 156 // Find the earliest event time.
154 int64 min_time = std::numeric_limits<int64>::max(); 157 int64 min_time = std::numeric_limits<int64>::max();
155 for (size_t i = 0; i < events.size(); ++i) 158 for (size_t i = 0; i < events.size(); ++i)
156 min_time = std::min(min_time, events[i].second); 159 min_time = std::min(min_time, events[i].second);
157 160
158 // Record the exit funnel event times in a sparse stability histogram. 161 // Record the exit funnel event times in a sparse stability histogram.
159 for (size_t i = 0; i < events.size(); ++i) { 162 for (size_t i = 0; i < events.size(); ++i) {
160 std::string histogram_name( 163 std::string histogram_name(
161 WatcherMetricsProviderWin::kExitFunnelHistogramPrefix); 164 WatcherMetricsProviderWin::kExitFunnelHistogramPrefix);
162 histogram_name.append(base::WideToUTF8(events[i].first)); 165 histogram_name.append(base::WideToUTF8(events[i].first));
163 base::TimeDelta event_time = 166 base::TimeDelta event_time =
164 base::Time::FromInternalValue(events[i].second) - 167 base::Time::FromInternalValue(events[i].second) -
165 base::Time::FromInternalValue(min_time); 168 base::Time::FromInternalValue(min_time);
166 base::HistogramBase* histogram = 169 base::HistogramBase* histogram =
167 base::SparseHistogram::FactoryGet( 170 base::SparseHistogram::FactoryGet(
168 histogram_name.c_str(), 171 histogram_name.c_str(),
169 base::HistogramBase::kUmaStabilityHistogramFlag); 172 base::HistogramBase::kUmaStabilityHistogramFlag);
170 173
171 // Record the time rounded up to the nearest millisecond. 174 // Record the time rounded up to the nearest millisecond.
172 histogram->Add(event_time.InMillisecondsRoundedUp()); 175 histogram->Add(event_time.InMillisecondsRoundedUp());
173 } 176 }
174 } 177 }
175 178
176 void RecordExitFunnels(const base::string16& registry_path) { 179 void MaybeRecordExitFunnels(const base::string16& registry_path, bool report) {
177 base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, registry_path.c_str()); 180 base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, registry_path.c_str());
178 if (!it.Valid()) 181 if (!it.Valid())
179 return; 182 return;
180 183
181 // Exit early if no work to do. 184 // Exit early if no work to do.
182 if (it.SubkeyCount() == 0) 185 if (it.SubkeyCount() == 0)
183 return; 186 return;
184 187
185 // Open the key we use for deletion preemptively to prevent reporting 188 // Open the key we use for deletion preemptively to prevent reporting
186 // multiple times on permission problems. 189 // multiple times on permission problems.
187 base::win::RegKey key(HKEY_CURRENT_USER, 190 base::win::RegKey key(HKEY_CURRENT_USER,
188 registry_path.c_str(), 191 registry_path.c_str(),
189 KEY_QUERY_VALUE); 192 KEY_QUERY_VALUE);
190 if (!key.Valid()) { 193 if (!key.Valid()) {
191 LOG(ERROR) << "Failed to open " << registry_path << " for writing."; 194 LOG(ERROR) << "Failed to open " << registry_path << " for writing.";
192 return; 195 return;
193 } 196 }
194 197
195 std::vector<base::string16> to_delete; 198 std::vector<base::string16> to_delete;
196 for (; it.Valid(); ++it) { 199 for (; it.Valid(); ++it) {
197 // Defer reporting on still-live processes. 200 // Defer reporting on still-live processes.
198 if (IsDeadProcess(it.Name())) { 201 if (IsDeadProcess(it.Name())) {
199 RecordSingleExitFunnel(&key, it.Name()); 202 MaybeRecordSingleExitFunnel(&key, it.Name(), report);
200 to_delete.push_back(it.Name()); 203 to_delete.push_back(it.Name());
201 } 204 }
202 } 205 }
203 206
204 for (size_t i = 0; i < to_delete.size(); ++i) { 207 for (size_t i = 0; i < to_delete.size(); ++i) {
205 LONG res = key.DeleteEmptyKey(to_delete[i].c_str()); 208 LONG res = key.DeleteEmptyKey(to_delete[i].c_str());
206 if (res != ERROR_SUCCESS) 209 if (res != ERROR_SUCCESS)
207 LOG(ERROR) << "Failed to delete key " << to_delete[i]; 210 LOG(ERROR) << "Failed to delete key " << to_delete[i];
208 } 211 }
209 } 212 }
210 213
211 } // namespace 214 } // namespace
212 215
213 const char WatcherMetricsProviderWin::kBrowserExitCodeHistogramName[] = 216 const char WatcherMetricsProviderWin::kBrowserExitCodeHistogramName[] =
214 "Stability.BrowserExitCodes"; 217 "Stability.BrowserExitCodes";
215 const char WatcherMetricsProviderWin::kExitFunnelHistogramPrefix[] = 218 const char WatcherMetricsProviderWin::kExitFunnelHistogramPrefix[] =
216 "Stability.ExitFunnel."; 219 "Stability.ExitFunnel.";
217 220
218 WatcherMetricsProviderWin::WatcherMetricsProviderWin( 221 WatcherMetricsProviderWin::WatcherMetricsProviderWin(
219 const base::char16* registry_path) : registry_path_(registry_path) { 222 const base::char16* registry_path, bool report_exit_funnels) :
223 registry_path_(registry_path),
224 report_exit_funnels_(report_exit_funnels) {
220 } 225 }
221 226
222 WatcherMetricsProviderWin::~WatcherMetricsProviderWin() { 227 WatcherMetricsProviderWin::~WatcherMetricsProviderWin() {
223 } 228 }
224 229
225 void WatcherMetricsProviderWin::ProvideStabilityMetrics( 230 void WatcherMetricsProviderWin::ProvideStabilityMetrics(
226 metrics::SystemProfileProto* /* system_profile_proto */) { 231 metrics::SystemProfileProto* /* system_profile_proto */) {
227 // Note that if there are multiple instances of Chrome running in the same 232 // Note that if there are multiple instances of Chrome running in the same
228 // user account, there's a small race that will double-report the exit codes 233 // user account, there's a small race that will double-report the exit codes
229 // from both/multiple instances. This ought to be vanishingly rare and will 234 // from both/multiple instances. This ought to be vanishingly rare and will
230 // only manifest as low-level "random" noise. To work around this it would be 235 // only manifest as low-level "random" noise. To work around this it would be
231 // necessary to implement some form of global locking, which is not worth it 236 // necessary to implement some form of global locking, which is not worth it
232 // here. 237 // here.
233 RecordExitCodes(registry_path_); 238 RecordExitCodes(registry_path_);
234 RecordExitFunnels(registry_path_); 239 MaybeRecordExitFunnels(registry_path_, report_exit_funnels_);
235 } 240 }
236 241
237 } // namespace browser_watcher 242 } // namespace browser_watcher
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698