aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNeil Fuller <nfuller@google.com>2021-03-08 16:00:10 +0000
committerNeil Fuller <nfuller@google.com>2021-03-08 16:51:48 +0000
commiteaec55eb04bbc2ec6ea13f292111f9d6243bf81d (patch)
tree3e861a88f426a4257416248716ef426200fa58bb
parent273a4834f5003e45d02c777ac2f53226a91faf54 (diff)
downloadGeoTZ-eaec55eb04bbc2ec6ea13f292111f9d6243bf81d.tar.gz
Add infrastructure for metrics
This contains refactoring and plumbing for adding metrics to the OfflineLocationTimeZoneDelegate. The metrics implementation is assumed to be specific to the apex deployment. Other deployments of the same code can use a different metrics mechanism if they choose. Bug: 172934905 Test: build / boot / treehugger Change-Id: Iadba9186ab294c1e8dbdbc42fb6881cec40ffe40
-rw-r--r--apex/com.android.geotz/Android.bp4
-rw-r--r--apex/com.android.geotz/resources/offlineltzprovider.properties6
-rw-r--r--apex/com.android.geotz/src/main/java/com/android/geotz/MetricsReporterImpl.java33
-rw-r--r--locationtzprovider/src/main/java/com/android/timezone/location/provider/EnvironmentImpl.java111
-rw-r--r--locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Environment.java6
-rw-r--r--locationtzprovider/src/main/java/com/android/timezone/location/provider/core/MetricsReporter.java35
-rw-r--r--locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Mode.java5
-rw-r--r--locationtzprovider/src/main/java/com/android/timezone/location/provider/core/OfflineLocationTimeZoneDelegate.java5
8 files changed, 165 insertions, 40 deletions
diff --git a/apex/com.android.geotz/Android.bp b/apex/com.android.geotz/Android.bp
index 6fad0fa..ae08a2e 100644
--- a/apex/com.android.geotz/Android.bp
+++ b/apex/com.android.geotz/Android.bp
@@ -59,6 +59,9 @@ apex {
java_library {
name: "geotz",
java_resource_dirs: ["resources/"],
+ srcs: [
+ "src/main/java/**/*.java",
+ ],
static_libs: [
"offlinelocationtimezoneprovider",
],
@@ -67,3 +70,4 @@ java_library {
"com.android.geotz",
],
}
+
diff --git a/apex/com.android.geotz/resources/offlineltzprovider.properties b/apex/com.android.geotz/resources/offlineltzprovider.properties
index f188942..f5a9f4b 100644
--- a/apex/com.android.geotz/resources/offlineltzprovider.properties
+++ b/apex/com.android.geotz/resources/offlineltzprovider.properties
@@ -11,4 +11,8 @@ geodata.path=/apex/com.android.geotz/etc/tzs2.dat
deviceconfig.namespace=system_time
# The prefix that should be applied to keys passed to android.provider.DeviceConfig.
-deviceconfig.key_prefix=geotz_ \ No newline at end of file
+deviceconfig.key_prefix=geotz_
+
+# The implementation of com.android.timezone.location.provider.core.MetricsReporter to use.
+# This implementation uses Google's standard metrics reporting.
+metrics_reporter.impl=com.android.geotz.MetricsReporterImpl \ No newline at end of file
diff --git a/apex/com.android.geotz/src/main/java/com/android/geotz/MetricsReporterImpl.java b/apex/com.android.geotz/src/main/java/com/android/geotz/MetricsReporterImpl.java
new file mode 100644
index 0000000..597b054
--- /dev/null
+++ b/apex/com.android.geotz/src/main/java/com/android/geotz/MetricsReporterImpl.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.geotz;
+
+import com.android.timezone.location.provider.core.MetricsReporter;
+import com.android.timezone.location.provider.core.OfflineLocationTimeZoneDelegate.ListenModeEnum;
+
+/**
+ * The implementation of {@link MetricsReporter} used for the com.android.geotz module deployment
+ * of {@link com.android.timezone.location.provider.OfflineLocationTimeZoneProviderService}.
+ */
+public class MetricsReporterImpl extends MetricsReporter {
+
+ @Override
+ public void reportLocationListeningCompletedEvent(@ListenModeEnum int listenMode,
+ long requestedDurationMillis, long actualTimeListeningMillis,
+ int listeningStoppedReason) {
+ // TODO(b/172934905): Implement once the atom has landed.
+ }
+}
diff --git a/locationtzprovider/src/main/java/com/android/timezone/location/provider/EnvironmentImpl.java b/locationtzprovider/src/main/java/com/android/timezone/location/provider/EnvironmentImpl.java
index 5fa3c10..efdca8b 100644
--- a/locationtzprovider/src/main/java/com/android/timezone/location/provider/EnvironmentImpl.java
+++ b/locationtzprovider/src/main/java/com/android/timezone/location/provider/EnvironmentImpl.java
@@ -43,6 +43,7 @@ import com.android.timezone.location.lookup.GeoTimeZonesFinder;
import com.android.timezone.location.provider.core.Cancellable;
import com.android.timezone.location.provider.core.Environment;
import com.android.timezone.location.provider.core.LocationListeningAccountant;
+import com.android.timezone.location.provider.core.MetricsReporter;
import com.android.timezone.location.provider.core.TimeZoneProviderResult;
import java.io.File;
@@ -82,6 +83,17 @@ class EnvironmentImpl implements Environment {
private static final String RESOURCE_CONFIG_KEY_DEVICE_CONFIG_KEY_PREFIX =
"deviceconfig.key_prefix";
+ /**
+ * The config properties key for the implementation of {@link MetricsReporter} to use.
+ */
+ private static final String RESOURCE_CONFIG_METRICS_REPORTER_IMPL = "metrics_reporter.impl";
+
+ /**
+ * An identifier that can be used to distinguish between different deployments of the same code.
+ */
+ private static final String RESOURCE_CONFIG_METRICS_REPORTER_DEPLOYMENT_IDENTIFIER =
+ "metrics_reporter.identifier";
+
/** An arbitrary value larger than the largest time we might want to hold a wake lock. */
private static final long WAKELOCK_ACQUIRE_MILLIS = Duration.ofMinutes(1).toMillis();
@@ -134,6 +146,7 @@ class EnvironmentImpl implements Environment {
@NonNull private final String mDeviceConfigNamespace;
@NonNull private final String mDeviceConfigKeyPrefix;
@NonNull private final DelegatingLocationListeningAccountant mLocationListeningAccountant;
+ @NonNull private final MetricsReporter mMetricsReporter;
EnvironmentImpl(@NonNull Context context,
@NonNull Consumer<TimeZoneProviderResult> resultConsumer) {
@@ -153,6 +166,10 @@ class EnvironmentImpl implements Environment {
mDeviceConfigKeyPrefix = Objects.requireNonNull(
configProperties.getProperty(RESOURCE_CONFIG_KEY_DEVICE_CONFIG_KEY_PREFIX));
+ String metricsReporterClassName =
+ configProperties.getProperty(RESOURCE_CONFIG_METRICS_REPORTER_IMPL);
+ mMetricsReporter = createMetricsReporter(metricsReporterClassName);
+
LocationListeningAccountant realLocationListeningAccountant =
createRealLocationListeningAccountant();
mLocationListeningAccountant =
@@ -168,6 +185,15 @@ class EnvironmentImpl implements Environment {
mDeviceConfigNamespace, mExecutor, this::handleDeviceConfigChanged);
}
+ private MetricsReporter createMetricsReporter(@NonNull String className) {
+ try {
+ Class<?> clazz = Class.forName(className);
+ return (MetricsReporter) clazz.newInstance();
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException("Unable to instantiate MetricsReporter", e);
+ }
+ }
+
private static Properties loadConfigProperties(ClassLoader classLoader) {
Properties configProperties = new Properties();
try (InputStream configStream =
@@ -271,11 +297,18 @@ class EnvironmentImpl implements Environment {
LocationListener locationListener = new LocationListener() {
@Override
public void onLocationChanged(@NonNull Location location) {
+ long resultElapsedRealtimeMillis = elapsedRealtimeMillis();
LocationListeningResult result = new LocationListeningResult(
LOCATION_LISTEN_MODE_PASSIVE,
duration, startElapsedRealtimeMillis,
- elapsedRealtimeMillis(),
+ resultElapsedRealtimeMillis,
location);
+
+ // Note: We only log metrics when listening stops (since we're primarily
+ // interested in active usage for system health reasons). Passive listening
+ // continues until it is cancelled or the timeout managed by this class
+ // triggers, so there is no need to do metrics logging here.
+
listeningResultConsumer.accept(result);
}
};
@@ -285,8 +318,8 @@ class EnvironmentImpl implements Environment {
// Using the passive provider means we will potentially see GPS-originated locations
// in addition to just "fused" locationprovider updates.
- // We don't use the setDurationMillis() since we handle our own timeout below, which
- // provides an explicit callback when listening stops.
+ // We don't use the setDurationMillis() with a real time, since we handle our own
+ // timeout below, which provides an explicit callback when listening stops.
LocationRequest locationRequest = new LocationRequest.Builder(minUpdateInterval)
.setMinUpdateDistanceMeters(0)
.setMinUpdateIntervalMillis(minUpdateInterval)
@@ -300,7 +333,7 @@ class EnvironmentImpl implements Environment {
LocationManager.PASSIVE_PROVIDER, locationRequest, mExecutor, locationListener);
String callbackIdentifier = "passive:" + duration + "@"
- + formatElapsedRealtimeMillis(elapsedRealtimeMillis());
+ + formatElapsedRealtimeMillis(startElapsedRealtimeMillis);
Consumer<String> timeoutCallback = token -> {
// When the timeout triggers we must cancel the location listening.
mLocationManager.removeUpdates(locationListener);
@@ -311,9 +344,15 @@ class EnvironmentImpl implements Environment {
// sleep in the middle and get misleading values.
try {
acquireWakeLock();
- Duration timeElapsed =
- Duration.ofMillis(elapsedRealtimeMillis() - startElapsedRealtimeMillis);
- passiveListeningCompletedCallback.accept(timeElapsed);
+ long actualTimeListeningMillis =
+ elapsedRealtimeMillis() - startElapsedRealtimeMillis;
+ Duration actualTimeListening = Duration.ofMillis(actualTimeListeningMillis);
+ passiveListeningCompletedCallback.accept(actualTimeListening);
+
+ mMetricsReporter.reportLocationListeningCompletedEvent(
+ LOCATION_LISTEN_MODE_PASSIVE, duration.toMillis(),
+ actualTimeListeningMillis,
+ MetricsReporter.LISTENING_STOPPED_REASON_TIMED_OUT);
} finally {
releaseWakeLock();
}
@@ -324,6 +363,11 @@ class EnvironmentImpl implements Environment {
return new BaseCancellable(callbackIdentifier) {
@Override
public void onCancel() {
+ long timeListeningMillis = elapsedRealtimeMillis() - startElapsedRealtimeMillis;
+ mMetricsReporter.reportLocationListeningCompletedEvent(
+ LOCATION_LISTEN_MODE_PASSIVE, duration.toMillis(), timeListeningMillis,
+ MetricsReporter.LISTENING_STOPPED_REASON_CANCELLED);
+
timeoutCancellable.cancel();
mLocationManager.removeUpdates(locationListener);
}
@@ -337,39 +381,49 @@ class EnvironmentImpl implements Environment {
@Override
public Cancellable startActiveGetCurrentLocation(@NonNull Duration duration,
@NonNull Consumer<LocationListeningResult> locationResultConsumer) {
- CancellationSignal cancellationSignal = new CancellationSignal();
- String identifier = "active:" + duration + "@"
- + formatElapsedRealtimeMillis(elapsedRealtimeMillis());
- Cancellable locationListenerCancellable = new BaseCancellable(identifier) {
- @Override
- protected void onCancel() {
- cancellationSignal.cancel();
- }
- };
-
- long intervalMillis = 0; // Not used
- long durationMillis = duration.toMillis();
- LocationRequest locationRequest = new LocationRequest.Builder(intervalMillis)
- .setDurationMillis(durationMillis)
- .setQuality(QUALITY_HIGH_ACCURACY /* try to force GPS on when it's needed */)
- .setMaxUpdateDelayMillis(0 /* no batching */)
- .build();
-
try {
// Keep a wakelock while we call getCurrentLocation() so we can calculate a fairly
// accurate (upper bound) of how long the device has been actively listening when we
// get a result.
acquireWakeLock();
+
+ CancellationSignal cancellationSignal = new CancellationSignal();
long startElapsedRealtimeMillis = elapsedRealtimeMillis();
+ String identifier = "active:" + duration + "@"
+ + formatElapsedRealtimeMillis(startElapsedRealtimeMillis);
+ Cancellable locationListenerCancellable = new BaseCancellable(identifier) {
+ @Override
+ protected void onCancel() {
+ long timeListeningMillis = elapsedRealtimeMillis() - startElapsedRealtimeMillis;
+ mMetricsReporter.reportLocationListeningCompletedEvent(
+ LOCATION_LISTEN_MODE_ACTIVE, duration.toMillis(), timeListeningMillis,
+ MetricsReporter.LISTENING_STOPPED_REASON_CANCELLED);
+
+ cancellationSignal.cancel();
+ }
+ };
+
+ long intervalMillis = 0; // Not used
+ long requestedDurationMillis = duration.toMillis();
+ LocationRequest locationRequest = new LocationRequest.Builder(intervalMillis)
+ .setDurationMillis(requestedDurationMillis)
+ .setQuality(QUALITY_HIGH_ACCURACY /* try to force GPS on when it's needed */)
+ .setMaxUpdateDelayMillis(0 /* no batching */)
+ .build();
+
Consumer<Location> locationConsumer = location -> {
+ long resultElapsedRealtimeMillis = elapsedRealtimeMillis();
LocationListeningResult result = new LocationListeningResult(
LOCATION_LISTEN_MODE_ACTIVE,
duration,
startElapsedRealtimeMillis,
- elapsedRealtimeMillis(),
+ resultElapsedRealtimeMillis,
location);
- // TODO Add a metric to record the time spent active listening. http://b/152746105
- // result.getEstimatedTimeListening();
+ mMetricsReporter.reportLocationListeningCompletedEvent(
+ LOCATION_LISTEN_MODE_ACTIVE,
+ result.getRequestedListeningDuration().toMillis(),
+ result.getTotalEstimatedTimeListening().toMillis(),
+ MetricsReporter.LISTENING_STOPPED_REASON_LOCATION_OBTAINED);
locationResultConsumer.accept(result);
};
@@ -377,6 +431,7 @@ class EnvironmentImpl implements Environment {
mLocationManager.getCurrentLocation(
LocationManager.FUSED_PROVIDER, locationRequest, cancellationSignal, mExecutor,
locationConsumer);
+
return locationListenerCancellable;
} finally {
releaseWakeLock();
diff --git a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Environment.java b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Environment.java
index 535d1c8..d1678f9 100644
--- a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Environment.java
+++ b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Environment.java
@@ -125,12 +125,12 @@ public interface Environment {
}
/**
- * Returns the estimated time between when the listening started and this result was
+ * Returns the estimated time between when the listening session started and this result was
* generated. This value is <em>not</em> incremental, i.e. if listening is continuous then
* this returns the total time listening and not the time elapsed since the last result.
*/
@NonNull
- public Duration getEstimatedTimeListening() {
+ public Duration getTotalEstimatedTimeListening() {
Duration estimatedTimeListening =
Duration.ofMillis(mResultElapsedRealtimeMillis - mStartElapsedRealtimeMillis);
// Guard against invalid times that could be caused if locations have an incorrect
@@ -169,7 +169,7 @@ public interface Environment {
+ ", mResultElapsedRealtimeMillis="
+ formatElapsedRealtimeMillis(mResultElapsedRealtimeMillis)
+ ", mLocation=" + mLocation
- + ", getEstimatedTimeListening()=" + getEstimatedTimeListening()
+ + ", getTotalEstimatedTimeListening()=" + getTotalEstimatedTimeListening()
+ '}';
}
}
diff --git a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/MetricsReporter.java b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/MetricsReporter.java
new file mode 100644
index 0000000..78fbcda
--- /dev/null
+++ b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/MetricsReporter.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.timezone.location.provider.core;
+
+import static com.android.timezone.location.provider.core.OfflineLocationTimeZoneDelegate.ListenModeEnum;
+
+/**
+ * An interface that abstracts the metrics reporting infrastructure being used and what metrics
+ * are reported.
+ */
+public abstract class MetricsReporter {
+
+ public static final int LISTENING_STOPPED_REASON_TIMED_OUT = 1;
+ public static final int LISTENING_STOPPED_REASON_CANCELLED = 2;
+ public static final int LISTENING_STOPPED_REASON_LOCATION_OBTAINED = 3;
+
+ /** Reports a period spent listening for location. */
+ public void reportLocationListeningCompletedEvent(
+ @ListenModeEnum int listenMode,
+ long requestedDurationMillis, long actualDurationMillis,
+ int listeningStoppedReason) {}
+}
diff --git a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Mode.java b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Mode.java
index 9e829d5..3c37f8e 100644
--- a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Mode.java
+++ b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/Mode.java
@@ -141,11 +141,6 @@ class Mode {
return new Mode(MODE_STOPPED, "init" /* entryCause */);
}
- @NonNull
- Cancellable getLocationListenerCancellable() {
- return Objects.requireNonNull(mLocationListenerCancellable);
- }
-
/**
* If this mode is associated with location listening, this invokes {@link
* Cancellable#cancel()}. If this mode is not associated with location listening, this is a
diff --git a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/OfflineLocationTimeZoneDelegate.java b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/OfflineLocationTimeZoneDelegate.java
index 30ee77b..56d6c39 100644
--- a/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/OfflineLocationTimeZoneDelegate.java
+++ b/locationtzprovider/src/main/java/com/android/timezone/location/provider/core/OfflineLocationTimeZoneDelegate.java
@@ -57,8 +57,7 @@ import java.util.Objects;
* <p>Implementation details:
*
* <p>The instance interacts with multiple threads, but state changes occur in a single-threaded
- * manner through the use of a lock object, {@link #mLock}, obtained from {@link
- * Environment#getSharedLockObject()}.
+ * manner through the use of a lock object, {@link #mLock}.
*
* <p>There are two listening modes:
* <ul>
@@ -311,7 +310,7 @@ public final class OfflineLocationTimeZoneDelegate {
logDebug(debugInfo);
// Recover any active listening budget we didn't use.
- Duration timeListening = activeListeningResult.getEstimatedTimeListening();
+ Duration timeListening = activeListeningResult.getTotalEstimatedTimeListening();
Duration activeListeningDuration =
activeListeningResult.getRequestedListeningDuration();
Duration activeListeningDurationNotUsed = activeListeningDuration.minus(timeListening);