Address Puller API Feedback
1. Rename registerPullAtomCallback to setPullAtomCallback
2. Rename unregisterPullAtomCallback to clearPullAtomCallback
3. Add getters to PullAtomMetadata
4. Change Ns to Millis (when I tried to make it Nanos, I received a
built time error saying to prefer millis unless we need the precision.
We do not need the precision, so I changed it).
5. Fix out of order params.
I did not change usePooledBuffer to setPooledBuffer because I think use
is more appropriate for our use case.
Test: make
Test: atest PullAtomMetadataTest
Test: atest GtsStatsdHostTestCases
Bug: 149475498
Change-Id: Ib07aa57a6e02c77917fe0e65a3d4a77c00ce8565
diff --git a/apex/aidl/android/os/IStatsManagerService.aidl b/apex/aidl/android/os/IStatsManagerService.aidl
index 4a259f5..b59a97e 100644
--- a/apex/aidl/android/os/IStatsManagerService.aidl
+++ b/apex/aidl/android/os/IStatsManagerService.aidl
@@ -128,7 +128,7 @@
void removeConfiguration(in long configId, in String packageName);
/** Tell StatsManagerService to register a puller for the given atom tag with statsd. */
- oneway void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
+ oneway void registerPullAtomCallback(int atomTag, long coolDownMillis, long timeoutMillis,
in int[] additiveFields, IPullAtomCallback pullerCallback);
/** Tell StatsManagerService to unregister the pulller for the given atom tag from statsd. */
diff --git a/apex/aidl/android/os/IStatsd.aidl b/apex/aidl/android/os/IStatsd.aidl
index 10b1e5b..c8aec53 100644
--- a/apex/aidl/android/os/IStatsd.aidl
+++ b/apex/aidl/android/os/IStatsd.aidl
@@ -186,8 +186,9 @@
* Registers a puller callback function that, when invoked, pulls the data
* for the specified atom tag.
*/
- oneway void registerPullAtomCallback(int uid, int atomTag, long coolDownNs, long timeoutNs,
- in int[] additiveFields, IPullAtomCallback pullerCallback);
+ oneway void registerPullAtomCallback(int uid, int atomTag, long coolDownMillis,
+ long timeoutMillis,in int[] additiveFields,
+ IPullAtomCallback pullerCallback);
/**
* Registers a puller callback function that, when invoked, pulls the data
diff --git a/apex/framework/java/android/app/StatsManager.java b/apex/framework/java/android/app/StatsManager.java
index e637187..f021dcf 100644
--- a/apex/framework/java/android/app/StatsManager.java
+++ b/apex/framework/java/android/app/StatsManager.java
@@ -119,14 +119,12 @@
/**
* @hide
**/
- @VisibleForTesting
- public static final long DEFAULT_COOL_DOWN_NS = 1_000_000_000L; // 1 second.
+ @VisibleForTesting public static final long DEFAULT_COOL_DOWN_MILLIS = 1_000L; // 1 second.
/**
* @hide
**/
- @VisibleForTesting
- public static final long DEFAULT_TIMEOUT_NS = 10_000_000_000L; // 10 seconds.
+ @VisibleForTesting public static final long DEFAULT_TIMEOUT_MILLIS = 10_000L; // 10 seconds.
/**
* Constructor for StatsManagerClient.
@@ -489,23 +487,24 @@
}
/**
- * Registers a callback for an atom when that atom is to be pulled. The stats service will
+ * Sets a callback for an atom when that atom is to be pulled. The stats service will
* invoke pullData in the callback when the stats service determines that this atom needs to be
* pulled. This method should not be called by third-party apps.
*
* @param atomTag The tag of the atom for this puller callback.
* @param metadata Optional metadata specifying the timeout, cool down time, and
* additive fields for mapping isolated to host uids.
- * @param callback The callback to be invoked when the stats service pulls the atom.
* @param executor The executor in which to run the callback.
+ * @param callback The callback to be invoked when the stats service pulls the atom.
*
*/
@RequiresPermission(android.Manifest.permission.REGISTER_STATS_PULL_ATOM)
- public void registerPullAtomCallback(int atomTag, @Nullable PullAtomMetadata metadata,
+ public void setPullAtomCallback(int atomTag, @Nullable PullAtomMetadata metadata,
@NonNull @CallbackExecutor Executor executor,
@NonNull StatsPullAtomCallback callback) {
- long coolDownNs = metadata == null ? DEFAULT_COOL_DOWN_NS : metadata.mCoolDownNs;
- long timeoutNs = metadata == null ? DEFAULT_TIMEOUT_NS : metadata.mTimeoutNs;
+ long coolDownMillis =
+ metadata == null ? DEFAULT_COOL_DOWN_MILLIS : metadata.mCoolDownMillis;
+ long timeoutMillis = metadata == null ? DEFAULT_TIMEOUT_MILLIS : metadata.mTimeoutMillis;
int[] additiveFields = metadata == null ? new int[0] : metadata.mAdditiveFields;
if (additiveFields == null) {
additiveFields = new int[0];
@@ -516,8 +515,8 @@
IStatsManagerService service = getIStatsManagerServiceLocked();
PullAtomCallbackInternal rec =
new PullAtomCallbackInternal(atomTag, callback, executor);
- service.registerPullAtomCallback(atomTag, coolDownNs, timeoutNs, additiveFields,
- rec);
+ service.registerPullAtomCallback(
+ atomTag, coolDownMillis, timeoutMillis, additiveFields, rec);
} catch (RemoteException e) {
throw new RuntimeException("Unable to register pull callback", e);
}
@@ -525,14 +524,14 @@
}
/**
- * Unregisters a callback for an atom when that atom is to be pulled. Note that any ongoing
+ * Clears a callback for an atom when that atom is to be pulled. Note that any ongoing
* pulls will still occur. This method should not be called by third-party apps.
*
* @param atomTag The tag of the atom of which to unregister
*
*/
@RequiresPermission(android.Manifest.permission.REGISTER_STATS_PULL_ATOM)
- public void unregisterPullAtomCallback(int atomTag) {
+ public void clearPullAtomCallback(int atomTag) {
synchronized (sLock) {
try {
IStatsManagerService service = getIStatsManagerServiceLocked();
@@ -585,14 +584,14 @@
*
*/
public static class PullAtomMetadata {
- private final long mCoolDownNs;
- private final long mTimeoutNs;
+ private final long mCoolDownMillis;
+ private final long mTimeoutMillis;
private final int[] mAdditiveFields;
// Private Constructor for builder
- private PullAtomMetadata(long coolDownNs, long timeoutNs, int[] additiveFields) {
- mCoolDownNs = coolDownNs;
- mTimeoutNs = timeoutNs;
+ private PullAtomMetadata(long coolDownMillis, long timeoutMillis, int[] additiveFields) {
+ mCoolDownMillis = coolDownMillis;
+ mTimeoutMillis = timeoutMillis;
mAdditiveFields = additiveFields;
}
@@ -600,8 +599,8 @@
* Builder for PullAtomMetadata.
*/
public static class Builder {
- private long mCoolDownNs;
- private long mTimeoutNs;
+ private long mCoolDownMillis;
+ private long mTimeoutMillis;
private int[] mAdditiveFields;
/**
@@ -609,27 +608,28 @@
* StatsManager#registerPullAtomCallback
*/
public Builder() {
- mCoolDownNs = DEFAULT_COOL_DOWN_NS;
- mTimeoutNs = DEFAULT_TIMEOUT_NS;
+ mCoolDownMillis = DEFAULT_COOL_DOWN_MILLIS;
+ mTimeoutMillis = DEFAULT_TIMEOUT_MILLIS;
mAdditiveFields = null;
}
/**
- * Set the cool down time of the pull in nanoseconds. If two successive pulls are issued
- * within the cool down, a cached version of the first will be used for the second.
+ * Set the cool down time of the pull in milliseconds. If two successive pulls are
+ * issued within the cool down, a cached version of the first pull will be used for the
+ * second pull.
*/
@NonNull
- public Builder setCoolDownNs(long coolDownNs) {
- mCoolDownNs = coolDownNs;
+ public Builder setCoolDownMillis(long coolDownMillis) {
+ mCoolDownMillis = coolDownMillis;
return this;
}
/**
- * Set the maximum time the pull can take in nanoseconds.
+ * Set the maximum time the pull can take in milliseconds.
*/
@NonNull
- public Builder setTimeoutNs(long timeoutNs) {
- mTimeoutNs = timeoutNs;
+ public Builder setTimeoutMillis(long timeoutMillis) {
+ mTimeoutMillis = timeoutMillis;
return this;
}
@@ -652,30 +652,32 @@
*/
@NonNull
public PullAtomMetadata build() {
- return new PullAtomMetadata(mCoolDownNs, mTimeoutNs, mAdditiveFields);
+ return new PullAtomMetadata(mCoolDownMillis, mTimeoutMillis, mAdditiveFields);
}
}
/**
- * @hide
+ * Return the cool down time of this pull in milliseconds.
*/
- @VisibleForTesting
- public long getCoolDownNs() {
- return mCoolDownNs;
+ public long getCoolDownMillis() {
+ return mCoolDownMillis;
}
/**
- * @hide
+ * Return the maximum amount of time this pull can take in milliseconds.
*/
- @VisibleForTesting
- public long getTimeoutNs() {
- return mTimeoutNs;
+ public long getTimeoutMillis() {
+ return mTimeoutMillis;
}
/**
- * @hide
+ * Return the additive fields of this pulled atom.
+ *
+ * This is only applicable for atoms that have a uid field. When tasks are run in
+ * isolated processes, the data will be attributed to the host uid. Additive fields
+ * will be combined when the non-additive fields are the same.
*/
- @VisibleForTesting
+ @Nullable
public int[] getAdditiveFields() {
return mAdditiveFields;
}
diff --git a/apex/framework/test/src/android/app/PullAtomMetadataTest.java b/apex/framework/test/src/android/app/PullAtomMetadataTest.java
index 0ae6134..fd386bd 100644
--- a/apex/framework/test/src/android/app/PullAtomMetadataTest.java
+++ b/apex/framework/test/src/android/app/PullAtomMetadataTest.java
@@ -33,28 +33,28 @@
@Test
public void testEmpty() {
PullAtomMetadata metadata = new PullAtomMetadata.Builder().build();
- assertThat(metadata.getTimeoutNs()).isEqualTo(StatsManager.DEFAULT_TIMEOUT_NS);
- assertThat(metadata.getCoolDownNs()).isEqualTo(StatsManager.DEFAULT_COOL_DOWN_NS);
+ assertThat(metadata.getTimeoutMillis()).isEqualTo(StatsManager.DEFAULT_TIMEOUT_MILLIS);
+ assertThat(metadata.getCoolDownMillis()).isEqualTo(StatsManager.DEFAULT_COOL_DOWN_MILLIS);
assertThat(metadata.getAdditiveFields()).isNull();
}
@Test
- public void testSetTimeoutNs() {
- long timeoutNs = 500_000_000L;
+ public void testSetTimeoutMillis() {
+ long timeoutMillis = 500L;
PullAtomMetadata metadata =
- new PullAtomMetadata.Builder().setTimeoutNs(timeoutNs).build();
- assertThat(metadata.getTimeoutNs()).isEqualTo(timeoutNs);
- assertThat(metadata.getCoolDownNs()).isEqualTo(StatsManager.DEFAULT_COOL_DOWN_NS);
+ new PullAtomMetadata.Builder().setTimeoutMillis(timeoutMillis).build();
+ assertThat(metadata.getTimeoutMillis()).isEqualTo(timeoutMillis);
+ assertThat(metadata.getCoolDownMillis()).isEqualTo(StatsManager.DEFAULT_COOL_DOWN_MILLIS);
assertThat(metadata.getAdditiveFields()).isNull();
}
@Test
- public void testSetCoolDownNs() {
- long coolDownNs = 10_000_000_000L;
+ public void testSetCoolDownMillis() {
+ long coolDownMillis = 10_000L;
PullAtomMetadata metadata =
- new PullAtomMetadata.Builder().setCoolDownNs(coolDownNs).build();
- assertThat(metadata.getTimeoutNs()).isEqualTo(StatsManager.DEFAULT_TIMEOUT_NS);
- assertThat(metadata.getCoolDownNs()).isEqualTo(coolDownNs);
+ new PullAtomMetadata.Builder().setCoolDownMillis(coolDownMillis).build();
+ assertThat(metadata.getTimeoutMillis()).isEqualTo(StatsManager.DEFAULT_TIMEOUT_MILLIS);
+ assertThat(metadata.getCoolDownMillis()).isEqualTo(coolDownMillis);
assertThat(metadata.getAdditiveFields()).isNull();
}
@@ -63,23 +63,23 @@
int[] fields = {2, 4, 6};
PullAtomMetadata metadata =
new PullAtomMetadata.Builder().setAdditiveFields(fields).build();
- assertThat(metadata.getTimeoutNs()).isEqualTo(StatsManager.DEFAULT_TIMEOUT_NS);
- assertThat(metadata.getCoolDownNs()).isEqualTo(StatsManager.DEFAULT_COOL_DOWN_NS);
+ assertThat(metadata.getTimeoutMillis()).isEqualTo(StatsManager.DEFAULT_TIMEOUT_MILLIS);
+ assertThat(metadata.getCoolDownMillis()).isEqualTo(StatsManager.DEFAULT_COOL_DOWN_MILLIS);
assertThat(metadata.getAdditiveFields()).isEqualTo(fields);
}
@Test
public void testSetAllElements() {
- long timeoutNs = 300L;
- long coolDownNs = 9572L;
+ long timeoutMillis = 300L;
+ long coolDownMillis = 9572L;
int[] fields = {3, 2};
PullAtomMetadata metadata = new PullAtomMetadata.Builder()
- .setTimeoutNs(timeoutNs)
- .setCoolDownNs(coolDownNs)
+ .setTimeoutMillis(timeoutMillis)
+ .setCoolDownMillis(coolDownMillis)
.setAdditiveFields(fields)
.build();
- assertThat(metadata.getTimeoutNs()).isEqualTo(timeoutNs);
- assertThat(metadata.getCoolDownNs()).isEqualTo(coolDownNs);
+ assertThat(metadata.getTimeoutMillis()).isEqualTo(timeoutMillis);
+ assertThat(metadata.getCoolDownMillis()).isEqualTo(coolDownMillis);
assertThat(metadata.getAdditiveFields()).isEqualTo(fields);
}
}
diff --git a/apex/service/java/com/android/server/stats/StatsManagerService.java b/apex/service/java/com/android/server/stats/StatsManagerService.java
index 4e4bc40..58c78da 100644
--- a/apex/service/java/com/android/server/stats/StatsManagerService.java
+++ b/apex/service/java/com/android/server/stats/StatsManagerService.java
@@ -136,25 +136,25 @@
}
private static class PullerValue {
- private final long mCoolDownNs;
- private final long mTimeoutNs;
+ private final long mCoolDownMillis;
+ private final long mTimeoutMillis;
private final int[] mAdditiveFields;
private final IPullAtomCallback mCallback;
- PullerValue(long coolDownNs, long timeoutNs, int[] additiveFields,
+ PullerValue(long coolDownMillis, long timeoutMillis, int[] additiveFields,
IPullAtomCallback callback) {
- mCoolDownNs = coolDownNs;
- mTimeoutNs = timeoutNs;
+ mCoolDownMillis = coolDownMillis;
+ mTimeoutMillis = timeoutMillis;
mAdditiveFields = additiveFields;
mCallback = callback;
}
- public long getCoolDownNs() {
- return mCoolDownNs;
+ public long getCoolDownMillis() {
+ return mCoolDownMillis;
}
- public long getTimeoutNs() {
- return mTimeoutNs;
+ public long getTimeoutMillis() {
+ return mTimeoutMillis;
}
public int[] getAdditiveFields() {
@@ -169,12 +169,13 @@
private final ArrayMap<PullerKey, PullerValue> mPullers = new ArrayMap<>();
@Override
- public void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
+ public void registerPullAtomCallback(int atomTag, long coolDownMillis, long timeoutMillis,
int[] additiveFields, IPullAtomCallback pullerCallback) {
enforceRegisterStatsPullAtomPermission();
int callingUid = Binder.getCallingUid();
PullerKey key = new PullerKey(callingUid, atomTag);
- PullerValue val = new PullerValue(coolDownNs, timeoutNs, additiveFields, pullerCallback);
+ PullerValue val =
+ new PullerValue(coolDownMillis, timeoutMillis, additiveFields, pullerCallback);
// Always cache the puller in StatsManagerService. If statsd is down, we will register the
// puller when statsd comes back up.
@@ -189,8 +190,8 @@
final long token = Binder.clearCallingIdentity();
try {
- statsd.registerPullAtomCallback(
- callingUid, atomTag, coolDownNs, timeoutNs, additiveFields, pullerCallback);
+ statsd.registerPullAtomCallback(callingUid, atomTag, coolDownMillis, timeoutMillis,
+ additiveFields, pullerCallback);
} catch (RemoteException e) {
Log.e(TAG, "Failed to access statsd to register puller for atom " + atomTag);
} finally {
@@ -596,8 +597,8 @@
for (Map.Entry<PullerKey, PullerValue> entry : pullersCopy.entrySet()) {
PullerKey key = entry.getKey();
PullerValue value = entry.getValue();
- statsd.registerPullAtomCallback(key.getUid(), key.getAtom(), value.getCoolDownNs(),
- value.getTimeoutNs(), value.getAdditiveFields(), value.getCallback());
+ statsd.registerPullAtomCallback(key.getUid(), key.getAtom(), value.getCoolDownMillis(),
+ value.getTimeoutMillis(), value.getAdditiveFields(), value.getCallback());
}
}
diff --git a/apex/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java b/apex/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java
index e119b4c..d4e51e0 100644
--- a/apex/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java
+++ b/apex/tests/libstatspull/src/com/android/internal/os/statsd/libstats/LibStatsPullTests.java
@@ -229,7 +229,7 @@
// Let the current bucket finish.
Thread.sleep(LONG_SLEEP_MILLIS);
List<Atom> data = StatsConfigUtils.getGaugeMetricDataList(statsManager, sConfigId);
- statsManager.unregisterPullAtomCallback(PULL_ATOM_TAG);
+ unregisterStatsPuller(PULL_ATOM_TAG);
assertThat(data.size()).isEqualTo(sAtomsPerPull);
for (int i = 0; i < data.size(); i++) {