[OBSOLETE] Reporting: Initial implementation.

net/reporting/reporting_metrics.cc

Index: net/reporting/reporting_metrics.cc
diff --git a/net/reporting/reporting_metrics.cc b/net/reporting/reporting_metrics.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5bed2c8b68bc4f579526a53264a1e5d8fd6e6e6b
--- /dev/null
+++ b/net/reporting/reporting_metrics.cc
@@ -0,0 +1,71 @@
+// Copyright 2016 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 "net/reporting/reporting_metrics.h"
+
+#include "base/metrics/histogram_macros.h"
+
+namespace net {
+
+void HistogramEnabledState(EnabledState state) {
+  DCHECK_NE(ENABLED_STATE_MAX, state);
+  UMA_HISTOGRAM_ENUMERATION("Reporting.EnabledState", state, ENABLED_STATE_MAX);
+}
+
+void HistogramHeader(HeaderFate fate) {
+  DCHECK_NE(HEADER_FATE_MAX, fate);

[Ryan Sleevi, 2017/01/03 21:28:13]

All of these DCHECKs are arguably unnecessary

I appreciate you're trying to guard against programmer error, but in C++, any
possible integer may have been coerced into such an enum, leaving it in an
undefined state. It would appear from these DCHECKs that you're trying to
validate that the enumeration value is in range of the histogram, but that's not
at all sufficient to do, and I'm not sure the programmer error you're attempting
to guard against is reasonable/likely.

[Julia Tuttle, 2017/01/25 20:27:45]

Done.

+  UMA_HISTOGRAM_ENUMERATION("Reporting.HeaderFate", fate, HEADER_FATE_MAX);
+}
+
+void HistogramHeaderEndpoint(HeaderEndpointFate fate, base::TimeDelta ttl) {
+  DCHECK_NE(HEADER_ENDPOINT_FATE_MAX, fate);
+  UMA_HISTOGRAM_ENUMERATION("Reporting.HeaderEndpointFate", fate,
+                            HEADER_ENDPOINT_FATE_MAX);
+  if (ttl > base::TimeDelta()) {

[Ryan Sleevi, 2017/01/03 21:28:13]

This is subtle and under-documented (in your design or your header) as to why a
negative TTL is not measured, even though you anticipated it enough to defend
against it.

[Julia Tuttle, 2017/01/25 20:27:45]

I'm not defending against a negative TTL; I'm differentiating a *zero* TTL
(which means "remove this endpoint") from a positive TTL (which means "set or
update this endpoint").

I've added a comment above the if to clarify.

+    UMA_HISTOGRAM_CUSTOM_TIMES("Reporting.HeaderEndpointTTL", ttl,
+                               base::TimeDelta::FromSeconds(1),
+                               base::TimeDelta::FromDays(7), 100);
+  }
+}
+
+void HistogramClient(base::TimeDelta ttl) {
+  UMA_HISTOGRAM_CUSTOM_TIMES("Reporting.ClientTTL", ttl,
+                             base::TimeDelta::FromSeconds(1),
+                             base::TimeDelta::FromDays(7), 100);
+}
+
+void HistogramReport(ReportFate fate, base::TimeDelta latency, int attempts) {
+  DCHECK_NE(REPORT_FATE_MAX, fate);
+  UMA_HISTOGRAM_ENUMERATION("Reporting.ReportFate", fate, REPORT_FATE_MAX);
+  if (fate == REPORT_FATE_DELIVERED) {
+    UMA_HISTOGRAM_LONG_TIMES("Reporting.ReportDeliveredAfterTime", latency);
+    UMA_HISTOGRAM_EXACT_LINEAR("Reporting.ReportDeliveredAfterAttempts",
+                               attempts, 10);
+  }
+}
+
+void HistogramDeliveryContent(int report_count, int byte_count) {
+  UMA_HISTOGRAM_COUNTS_100("Reporting.DeliveryReportCount", report_count);
+  UMA_HISTOGRAM_COUNTS_100000("Reporting.DeliveryByteCount", byte_count);

[Ryan Sleevi, 2017/01/03 21:28:13]

I'm very skeptical of a lot of these metrics, and don't see the design doc
really showing how they'll be owned or influence the design.

I appreciate and am sensitive to the fact that unless we measure from the
get-go, we won't know, but I'd like to push back on each and every one of these
measurements to make sure we know precisely why we're going to measure them and
what we anticipate using them for, so that we only collect what is needed to
influence the feature and the user experience.

[Julia Tuttle, 2017/01/25 20:27:45]

Alright, let me give it some thought.

The ones I'm particularly interested in are ones related to reports and
deliveries -- are we successfully uploading most reports within a reasonable
amount of time, and not losing too many to network changes and such?

+}
+
+void HistogramDeliveryOutcome(DeliveryFate fate,
+                              int net_error,
+                              int http_response_code) {
+  DCHECK_NE(DELIVERY_FATE_MAX, fate);
+  UMA_HISTOGRAM_ENUMERATION("Reporting.DeliveryFate", fate, DELIVERY_FATE_MAX);
+  if (fate != DELIVERY_FATE_SHUTDOWN) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Reporting.DeliveryNetError", net_error);
+    if (http_response_code > 0) {
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Reporting.DeliveryHttpResponseCode",
+                                  http_response_code);
+    }
+  }
+}
+
+void HistogramEndpoint(EndpointFate fate) {
+  UMA_HISTOGRAM_ENUMERATION("Reporting.EndpointFate", fate, ENDPOINT_FATE_MAX);
+}
+
+}  // namespace net