Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 72a70a8a authored by Tej Singh's avatar Tej Singh
Browse files

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
parent 995e2de8

apex/statsd/aidl/android/os/IStatsManagerService.aidl

+1 −1
Original line number Diff line number Diff line
@@ -128,7 +128,7 @@ interface IStatsManagerService { 
    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. */

apex/statsd/aidl/android/os/IStatsd.aidl

+3 −2
Original line number Diff line number Diff line
@@ -186,8 +186,9 @@ interface IStatsd { 
    * 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

apex/statsd/framework/java/android/app/StatsManager.java

+42 −40
Original line number Diff line number Diff line
@@ -119,14 +119,12 @@ public final class StatsManager { 
    /**

     * @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 @@ public final class StatsManager { 
    }



    /**

     * 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 @@ public final class StatsManager { 
                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 @@ public final class StatsManager { 
    }



    /**

     * 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 final class StatsManager { 
     *

     */

    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 @@ public final class StatsManager { 
         *  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 @@ public final class StatsManager { 
             * 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 @@ public final class StatsManager { 
             */

            @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;

        }

apex/statsd/framework/test/src/android/app/PullAtomMetadataTest.java

+20 −20
Original line number Diff line number Diff line
@@ -33,28 +33,28 @@ public final class PullAtomMetadataTest { 
    @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 @@ public final class PullAtomMetadataTest { 
        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);

    }

}

apex/statsd/service/java/com/android/server/stats/StatsManagerService.java

+16 −15
Original line number Diff line number Diff line
@@ -136,25 +136,25 @@ public class StatsManagerService extends IStatsManagerService.Stub { 
    }



    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 @@ public class StatsManagerService extends IStatsManagerService.Stub { 
    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 @@ public class StatsManagerService extends IStatsManagerService.Stub { 


        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 @@ public class StatsManagerService extends IStatsManagerService.Stub { 
        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());

        }

    }