Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structural output - RefreshLastRun #1867

Closed
jan-goral opened this issue Apr 26, 2021 · 5 comments · Fixed by #2139
Closed

Structural output - RefreshLastRun #1867

jan-goral opened this issue Apr 26, 2021 · 5 comments · Fixed by #2139

Comments

@jan-goral
Copy link
Contributor

jan-goral commented Apr 26, 2021

According to the description of epic #1728, prepare a specification document for the structural output based on the following requirements:

  • Identify all places that are producing console logs and list them in the specification.
  • Specify dedicated types and structures for logs with places (packages and namespaces) for them.
  • Specify proposition how to pass out function reference to low-level functions that producing logs.

Make sure that specification is sufficient to implement structured output without a doubt.

Command

RefreshLastRun

@piotradamczyk5
Copy link
Contributor

Places which produce output logs

  1. Loading run ftl.run.common.GetLastMatrices#L15
    logLn("Loading run $lastRun")

  2. Start info ftl.run.RefreshLastRun#L44
    logLn("RefreshMatrices")

  3. Refresh info ftl.run.RefreshLastRun#L58
    logLn(FtlConstants.indent + "Refreshing ${matrixCount}x matrices")

  4. Matrix info for each matrix ftl.run.RefreshLastRun#L65
    logLn(FtlConstants.indent + "${matrix.state} $matrixId")

  5. Update matrix file ftl.run.RefreshLastRun#L76
    logLn(FtlConstants.indent + "Updating matrix file")

  • ReportManager places which will be handled together with #1871 and #1870

Structures for messages

sealed interface RefreshLastRunState {
    data class LoadingRun(val lastRun: String) : RefreshLastRunState
    object RefreshMatricesStarted : RefreshLastRunState
    data class RefreshMatrices(val matrixCount: Int) : RefreshLastRunState
    data class RefreshMatrix(val matrixState: String, val matrixId: String) : RefreshLastRunState
    object UpdatingMatrixFile : RefreshLastRunState
}
  • ReportManager state which will be handled together with #1871 and #1870

Proposition of passing out function

Wiill be handled together with #1871 and [#1870](

@pawelpasterz
Copy link
Contributor

Places that produce output logs (ReportManager)

  1. matrix path not found warning ftl.reports.util.ReportManager#L113
    logLn("WARNING: Matrix path not found in JSON. $this")

  2. weblink not found warning ftl.reports.util.ReportManager#L108
    logLn("WARNING: Matrix path not found in JSON.")

  3. device model not found warning ftl.client.google.AndroidCatalog#L67
    logLn("Unable to find device type for $modelId. PHYSICAL used as fallback in cost calculations")

  4. uploading performance metrics ftl.client.google.GcStorage#L110
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."

  5. failure on metrics upload ftl.reports.api.PerformanceMetrics#L75
    logLn("Cannot upload performance metrics ${it.message}")

  6. print cost report ftl.reports.CostPeport#L66
    print(output) // output -> generated string, used also in report update and report upload

  7. uploading cost report ftl.client.google.GcStorage#L110 (ftl.reports.CostPeport#L68)
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."

  8. print matrix results report ftl.reports.MatrixResultsReport#L39
    log(output) // output -> generated string, used also in report update and report upload

  9. uploading matrix results report ftl.client.google.GcStorage#L110 (ftl.reports.MatrixResultsReport#L41)
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."

  10. uploading HTML report ftl.client.google.GcStorage#L110 (ftl.reports.HtmlErrorReport#L34)
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."

  11. device model not found warning ftl.client.google.AndroidCatalog#L67
    logLn("Unable to find device type for $modelId. PHYSICAL used as fallback in cost calculations")

  12. uploading performance metrics ftl.client.google.GcStorage#L110
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."

  13. failure on metrics upload ftl.reports.api.PerformanceMetrics#L75
    logLn("Cannot upload performance metrics ${it.message}")

  14. uploading full junit result report ftl.client.google.GcStorage#L110 (ftl.reports.FullJUnitReport#L20)
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."

  15. print actual shard times ftl.reports.util.ReportManager#L230

    logLn(
          "Actual shard times:\n" + list.joinToString("\n") {
              "  ${it.shard}: Expected: ${it.expectedTime.roundToInt()}s, Actual: ${it.finalTime.roundToInt()}s, Diff: ${it.timeDiff.roundToInt()}s"
          } + "\n"
      )
    
  16. upload junit xml ftl.reports.util.ReportManager#L252
    "Uploading [$name] to ${GCS_STORAGE_LINK + join(bucket, path).replace(name, "")}.."


sealed interface RunState

sealed interface ReportManagerState : RunState {
    data class Log(val message: String) : ReportManagerState
    data class Warning(val message: String) : ReportManagerState
    data class ShardTimes(val shards: List<ReportManager.ShardEfficiency>)
}

data class Uploading(val name: String, val path: String) : RunState

Since logs are not produced (and printed) in single action we can introduce a wrapper for a channel that will collect logs

object StateHolder {
    val logs = Channel<RunState>()
}

than we can replace logLn functions with StateHolder.logs.trySend(Warning(...))

The presentation layer will listen to any messages from StateHolder and depending on if it is desktop/cli (or sth else) it will be handled differently.

RefreshLastRunState, RunIosTestState etc. will implement RunState interface. That will allow to reuse logging logic and have only one source of logs (which would be easier to handle)

@piotradamczyk5
Copy link
Contributor

@jan-gogo could you verify the proposed solution?

@jan-goral
Copy link
Contributor Author

Looks ok, approved.

@zuziakaxel
Copy link
Contributor

Implementation: #2139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants