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

Commit c886aa14 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[SB Refactor] Update TableLogBuffer based on ABT changes. 

1) Add a header and footer so ABT can find the sections.
2) Require certain format from the column names.
3) Remove `dumpTables`, since that's now in ABT.
4) Update WifiNetwork's logging to not use -1 (-1 seemed
   unclear when reading a real bug report).

Bug: 259454430
Test: Dumped BR, uploaded to ABT, and verified that the wifi table log
appeared correctly (with corresponding ABT changes)
Test: atest TableLogBufferTest

Change-Id: I0564b3d0f423adcc3c3164d10fef062c677a0769
parent f4345bb8

packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt

+25 −78
Original line number Diff line number Diff line
@@ -16,10 +16,7 @@ 


package com.android.systemui.log.table



import androidx.annotation.VisibleForTesting

import com.android.systemui.Dumpable

import com.android.systemui.dump.DumpManager

import com.android.systemui.dump.DumpsysTableLogger

import com.android.systemui.plugins.util.RingBuffer

import com.android.systemui.util.time.SystemClock

import java.io.PrintWriter
@@ -83,7 +80,7 @@ class TableLogBuffer( 
    maxSize: Int,

    private val name: String,

    private val systemClock: SystemClock,

) {

) : Dumpable {

    init {

        if (maxSize <= 0) {

            throw IllegalArgumentException("maxSize must be > 0")
@@ -104,6 +101,9 @@ class TableLogBuffer( 
     * @param columnPrefix a prefix that will be applied to every column name that gets logged. This

     * ensures that all the columns related to the same state object will be grouped together in the

     * table.

     *

     * @throws IllegalArgumentException if [columnPrefix] or column name contain "|". "|" is used as

     * the separator token for parsing, so it can't be present in any part of the column name.

     */

    @Synchronized

    fun <T : Diffable<T>> logDiffs(columnPrefix: String, prevVal: T, newVal: T) {
@@ -135,32 +135,31 @@ class TableLogBuffer( 


    @Synchronized

    private fun obtain(timestamp: Long, prefix: String, columnName: String): TableChange {

        verifyValidName(prefix, columnName)

        val tableChange = buffer.advance()

        tableChange.reset(timestamp, prefix, columnName)

        return tableChange

    }



    /**

     * Registers this buffer as dumpables in [dumpManager]. Must be called for the table to be

     * dumped.

     *

     * This will be automatically called in [TableLogBufferFactory.create].

     */

    fun registerDumpables(dumpManager: DumpManager) {

        dumpManager.registerNormalDumpable("$name-changes", changeDumpable)

        dumpManager.registerNormalDumpable("$name-table", tableDumpable)

    private fun verifyValidName(prefix: String, columnName: String) {

        if (prefix.contains(SEPARATOR)) {

            throw IllegalArgumentException("columnPrefix cannot contain $SEPARATOR but was $prefix")

        }

        if (columnName.contains(SEPARATOR)) {

            throw IllegalArgumentException(

                "columnName cannot contain $SEPARATOR but was $columnName"

            )

        }

    }



    private val changeDumpable = Dumpable { pw, _ -> dumpChanges(pw) }

    private val tableDumpable = Dumpable { pw, _ -> dumpTable(pw) }



    /** Dumps the list of [TableChange] objects. */

    @Synchronized

    @VisibleForTesting

    fun dumpChanges(pw: PrintWriter) {

    override fun dump(pw: PrintWriter, args: Array<out String>) {

        pw.println(HEADER_PREFIX + name)

        pw.println("version $VERSION")

        for (i in 0 until buffer.size) {

            buffer[i].dump(pw)

        }

        pw.println(FOOTER_PREFIX + name)

    }



    /** Dumps an individual [TableChange]. */
@@ -170,69 +169,13 @@ class TableLogBuffer( 
        }

        val formattedTimestamp = TABLE_LOG_DATE_FORMAT.format(timestamp)

        pw.print(formattedTimestamp)

        pw.print(" ")

        pw.print(SEPARATOR)

        pw.print(this.getName())

        pw.print("=")

        pw.print(SEPARATOR)

        pw.print(this.getVal())

        pw.println()

    }



    /**

     * Coalesces all the [TableChange] objects into a table of values of time and dumps the table.

     */

    // TODO(b/259454430): Since this is an expensive process, it could cause the bug report dump to

    //   fail and/or not dump anything else. We should move this processing to ABT (Android Bug

    //   Tool), where we have unlimited time to process.

    @Synchronized

    @VisibleForTesting

    fun dumpTable(pw: PrintWriter) {

        val messages = buffer.iterator().asSequence().toList()



        if (messages.isEmpty()) {

            return

        }



        // Step 1: Create list of column headers

        val headerSet = mutableSetOf<String>()

        messages.forEach { headerSet.add(it.getName()) }

        val headers: MutableList<String> = headerSet.toList().sorted().toMutableList()

        headers.add(0, "timestamp")



        // Step 2: Create a list with the current values for each column. Will be updated with each

        // change.

        val currentRow: MutableList<String> = MutableList(headers.size) { DEFAULT_COLUMN_VALUE }



        // Step 3: For each message, make the correct update to [currentRow] and save it to [rows].

        val columnIndices: Map<String, Int> =

            headers.mapIndexed { index, headerName -> headerName to index }.toMap()

        val allRows = mutableListOf<List<String>>()



        messages.forEach {

            if (!it.hasData()) {

                return@forEach

            }



            val formattedTimestamp = TABLE_LOG_DATE_FORMAT.format(it.timestamp)

            if (formattedTimestamp != currentRow[0]) {

                // The timestamp has updated, so save the previous row and continue to the next row

                allRows.add(currentRow.toList())

                currentRow[0] = formattedTimestamp

            }

            val columnIndex = columnIndices[it.getName()]!!

            currentRow[columnIndex] = it.getVal()

        }

        // Add the last row

        allRows.add(currentRow.toList())



        // Step 4: Dump the rows

        DumpsysTableLogger(

                name,

                headers,

                allRows,

            )

            .printTableData(pw)

    }



    /**

     * A private implementation of [TableRowLogger].

     *
@@ -261,4 +204,8 @@ class TableLogBuffer( 
}



val TABLE_LOG_DATE_FORMAT = SimpleDateFormat("MM-dd HH:mm:ss.SSS", Locale.US)

private const val DEFAULT_COLUMN_VALUE = "UNKNOWN"
 


private const val HEADER_PREFIX = "SystemUI StateChangeTableSection START: "

private const val FOOTER_PREFIX = "SystemUI StateChangeTableSection END: "

private const val SEPARATOR = "|"

private const val VERSION = "1"

packages/SystemUI/src/com/android/systemui/log/table/TableLogBufferFactory.kt

+1 −1
Original line number Diff line number Diff line
@@ -34,7 +34,7 @@ constructor( 
        maxSize: Int,

    ): TableLogBuffer {

        val tableBuffer = TableLogBuffer(adjustMaxSize(maxSize), name, systemClock)

        tableBuffer.registerDumpables(dumpManager)

        dumpManager.registerNormalDumpable(name, tableBuffer)

        return tableBuffer

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiNetworkModel.kt

+7 −3
Original line number Diff line number Diff line
@@ -121,7 +121,11 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { 
                row.logChange(COL_VALIDATED, isValidated)

            }

            if (prevVal !is Active || prevVal.level != level) {

                row.logChange(COL_LEVEL, level ?: LEVEL_DEFAULT)

                if (level != null) {

                    row.logChange(COL_LEVEL, level)

                } else {

                    row.logChange(COL_LEVEL, LEVEL_DEFAULT)

                }

            }

            if (prevVal !is Active || prevVal.ssid != ssid) {

                row.logChange(COL_SSID, ssid)
@@ -201,5 +205,5 @@ const val COL_PASSPOINT_ACCESS_POINT = "isPasspointAccessPoint" 
const val COL_ONLINE_SIGN_UP = "isOnlineSignUpForPasspointAccessPoint"

const val COL_PASSPOINT_NAME = "passpointProviderFriendlyName"



const val LEVEL_DEFAULT = -1

const val NETWORK_ID_DEFAULT = -1
 
val LEVEL_DEFAULT: String? = null

val NETWORK_ID_DEFAULT: String? = null

packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt

+156 −30
Original line number Diff line number Diff line
@@ -45,6 +45,93 @@ class TableLogBufferTest : SysuiTestCase() { 
        TableLogBuffer(maxSize = 0, "name", systemClock)

    }



    @Test

    fun dumpChanges_hasHeader() {

        val dumpedString = dumpChanges()



        assertThat(logLines(dumpedString)[0]).isEqualTo(HEADER_PREFIX + NAME)

    }



    @Test

    fun dumpChanges_hasVersion() {

        val dumpedString = dumpChanges()



        assertThat(logLines(dumpedString)[1]).isEqualTo("version $VERSION")

    }



    @Test

    fun dumpChanges_hasFooter() {

        val dumpedString = dumpChanges()



        assertThat(logLines(dumpedString).last()).isEqualTo(FOOTER_PREFIX + NAME)

    }



    @Test(expected = IllegalArgumentException::class)

    fun dumpChanges_str_separatorNotAllowedInPrefix() {

        val next =

            object : TestDiffable() {

                override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {

                    row.logChange("columnName", "stringValue")

                }

            }

        underTest.logDiffs("some${SEPARATOR}thing", TestDiffable(), next)

    }



    @Test(expected = IllegalArgumentException::class)

    fun dumpChanges_bool_separatorNotAllowedInPrefix() {

        val next =

            object : TestDiffable() {

                override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {

                    row.logChange("columnName", true)

                }

            }

        underTest.logDiffs("some${SEPARATOR}thing", TestDiffable(), next)

    }



    @Test(expected = IllegalArgumentException::class)

    fun dumpChanges_int_separatorNotAllowedInPrefix() {

        val next =

            object : TestDiffable() {

                override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {

                    row.logChange("columnName", 567)

                }

            }

        underTest.logDiffs("some${SEPARATOR}thing", TestDiffable(), next)

    }



    @Test(expected = IllegalArgumentException::class)

    fun dumpChanges_str_separatorNotAllowedInColumnName() {

        val next =

            object : TestDiffable() {

                override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {

                    row.logChange("column${SEPARATOR}Name", "stringValue")

                }

            }

        underTest.logDiffs("prefix", TestDiffable(), next)

    }



    @Test(expected = IllegalArgumentException::class)

    fun dumpChanges_bool_separatorNotAllowedInColumnName() {

        val next =

            object : TestDiffable() {

                override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {

                    row.logChange("column${SEPARATOR}Name", true)

                }

            }

        underTest.logDiffs("prefix", TestDiffable(), next)

    }



    @Test(expected = IllegalArgumentException::class)

    fun dumpChanges_int_separatorNotAllowedInColumnName() {

        val next =

            object : TestDiffable() {

                override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {

                    row.logChange("column${SEPARATOR}Name", 456)

                }

            }

        underTest.logDiffs("prefix", TestDiffable(), next)

    }



    @Test

    fun dumpChanges_strChange_logsFromNext() {

        systemClock.setCurrentTimeMillis(100L)
@@ -66,11 +153,14 @@ class TableLogBufferTest : SysuiTestCase() { 


        val dumpedString = dumpChanges()



        assertThat(dumpedString).contains("prefix")

        assertThat(dumpedString).contains("stringValChange")

        assertThat(dumpedString).contains("newStringVal")

        val expected =

            TABLE_LOG_DATE_FORMAT.format(100L) +

                SEPARATOR +

                "prefix.stringValChange" +

                SEPARATOR +

                "newStringVal"

        assertThat(dumpedString).contains(expected)

        assertThat(dumpedString).doesNotContain("prevStringVal")

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))

    }



    @Test
@@ -94,11 +184,14 @@ class TableLogBufferTest : SysuiTestCase() { 


        val dumpedString = dumpChanges()



        assertThat(dumpedString).contains("prefix")

        assertThat(dumpedString).contains("booleanValChange")

        assertThat(dumpedString).contains("true")

        val expected =

            TABLE_LOG_DATE_FORMAT.format(100L) +

                SEPARATOR +

                "prefix.booleanValChange" +

                SEPARATOR +

                "true"

        assertThat(dumpedString).contains(expected)

        assertThat(dumpedString).doesNotContain("false")

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))

    }



    @Test
@@ -122,11 +215,14 @@ class TableLogBufferTest : SysuiTestCase() { 


        val dumpedString = dumpChanges()



        assertThat(dumpedString).contains("prefix")

        assertThat(dumpedString).contains("intValChange")

        assertThat(dumpedString).contains("67890")

        val expected =

            TABLE_LOG_DATE_FORMAT.format(100L) +

                SEPARATOR +

                "prefix.intValChange" +

                SEPARATOR +

                "67890"

        assertThat(dumpedString).contains(expected)

        assertThat(dumpedString).doesNotContain("12345")

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))

    }



    @Test
@@ -152,9 +248,9 @@ class TableLogBufferTest : SysuiTestCase() { 
        val dumpedString = dumpChanges()



        // THEN the dump still works

        assertThat(dumpedString).contains("booleanValChange")

        assertThat(dumpedString).contains("true")

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))

        val expected =

            TABLE_LOG_DATE_FORMAT.format(100L) + SEPARATOR + "booleanValChange" + SEPARATOR + "true"

        assertThat(dumpedString).contains(expected)

    }



    @Test
@@ -186,15 +282,34 @@ class TableLogBufferTest : SysuiTestCase() { 


        val dumpedString = dumpChanges()



        assertThat(dumpedString).contains("valChange")

        assertThat(dumpedString).contains("stateValue12")

        assertThat(dumpedString).contains("stateValue20")

        assertThat(dumpedString).contains("stateValue40")

        assertThat(dumpedString).contains("stateValue45")

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(12000L))

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(20000L))

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(40000L))

        assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(45000L))

        val expected1 =

            TABLE_LOG_DATE_FORMAT.format(12000L) +

                SEPARATOR +

                "valChange" +

                SEPARATOR +

                "stateValue12"

        val expected2 =

            TABLE_LOG_DATE_FORMAT.format(20000L) +

                SEPARATOR +

                "valChange" +

                SEPARATOR +

                "stateValue20"

        val expected3 =

            TABLE_LOG_DATE_FORMAT.format(40000L) +

                SEPARATOR +

                "valChange" +

                SEPARATOR +

                "stateValue40"

        val expected4 =

            TABLE_LOG_DATE_FORMAT.format(45000L) +

                SEPARATOR +

                "valChange" +

                SEPARATOR +

                "stateValue45"

        assertThat(dumpedString).contains(expected1)

        assertThat(dumpedString).contains(expected2)

        assertThat(dumpedString).contains(expected3)

        assertThat(dumpedString).contains(expected4)

    }



    @Test
@@ -214,10 +329,11 @@ class TableLogBufferTest : SysuiTestCase() { 


        val dumpedString = dumpChanges()



        assertThat(dumpedString).contains("status")

        assertThat(dumpedString).contains("in progress")

        assertThat(dumpedString).contains("connected")

        assertThat(dumpedString).contains("false")

        val timestamp = TABLE_LOG_DATE_FORMAT.format(100L)

        val expected1 = timestamp + SEPARATOR + "status" + SEPARATOR + "in progress"

        val expected2 = timestamp + SEPARATOR + "connected" + SEPARATOR + "false"

        assertThat(dumpedString).contains(expected1)

        assertThat(dumpedString).contains(expected2)

    }



    @Test
@@ -247,14 +363,24 @@ class TableLogBufferTest : SysuiTestCase() { 
    }



    private fun dumpChanges(): String {

        underTest.dumpChanges(PrintWriter(outputWriter))

        underTest.dump(PrintWriter(outputWriter), arrayOf())

        return outputWriter.toString()

    }



    private abstract class TestDiffable : Diffable<TestDiffable> {

    private fun logLines(string: String): List<String> {

        return string.split("\n").filter { it.isNotBlank() }

    }



    private open class TestDiffable : Diffable<TestDiffable> {

        override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {}

    }

}



private const val NAME = "TestTableBuffer"

private const val MAX_SIZE = 10



// Copying these here from [TableLogBuffer] so that we catch any accidental versioning change

private const val HEADER_PREFIX = "SystemUI StateChangeTableSection START: "

private const val FOOTER_PREFIX = "SystemUI StateChangeTableSection END: "

private const val SEPARATOR = "|" // TBD

private const val VERSION = "1"