[CORE-267] Query BQ once per BP for spend report, handle errors

[calypsomatic]

Ticket: https://broadworkbench.atlassian.net/browse/CORE-267
For the consolidated spend report, instead of unioning all billing projects together into one BQ query, this PR does a separate query for each billing project. If BQ returns an error for any one of them, it is left out of the results.

As currently implemented, BPs that generate errors are essentially ignored, resulting in all these workspaces simply not showing up in the UI. Since the form of the result returned from the Rawls API has a list of SpendReportingForDateRange, it might not be very straightforward to return a result for all the workspaces in a problematic BP that indicates that there was a problem. So that would suggest that the UI would have to figure out which, if any, workspaces are missing and display an 'N/A' for them, but that leaves out some information that we do have. Reviewers, is it worth it to try to shove some indication of the problem into a SpendReportingResult or is it ok just to leave it to the UI?

Testing:
I compared my results from the consolidated spend report API both before and after I split the query up and I believe the substance did not change. I then created a BP and gave it a false BQ dataset in its spend report configuration. After checking that this resulted in errors on the main branch, I verified that I was able to successfully get the report on this branch.

---

PR checklist

• Include the JIRA issue number in the PR description and title
• Make sure Swagger is updated if API changes
  • ...and Orchestration's Swagger too!
• If you changed anything in model/, then you should publish a new official rawls-model and perform the corresponding dependency updates as specified in the README:
  • in the automation subdirectory
  • in workbench-libs
  • in firecloud-orchestration
• Get two thumbsworth of PR review
• Verify all tests go green, including CI tests
• Squash commits and merge to develop (branches are automatically deleted after merging)
• Inform other teams of any substantial changes via Slack and/or email

[davidangb]

some syntax suggestions inline, not a full review:

[davidangb]

the WorkspaceNames aren't used anywhere inside this method. If you changed this method to accept a Seq[GoogleProjectId] instead then some interior code gets a little simpler

[davidangb]

nit/optional: you can use the 3-argument form of mkString here instead; your call on which is more readable

Suggested change

	.replace("_PROJECT_ID_LIST", "(" + workspaces.map(tuple => s""""${tuple._1.value}"""").mkString(", ") + ")")
	.replace("_PROJECT_ID_LIST", workspaces.map(tuple => s""""${tuple._1.value}"""").mkString("(", ", ", ")"))

[davidangb]

log something here to show that we ran into a problem?

[davidangb]

Consider writing a + method in the SpendReportingResults case class, then calling it here.

if you had:

case class SpendReportingResults(spendDetails: Seq[SpendReportingAggregation],
                                 spendSummary: SpendReportingForDateRange
) {
  def +(other: SpendReportingResults): SpendReportingResults =
    new SpendReportingResults(
      this.spendDetails ++ other.spendDetails,
      SpendReportingForDateRange(
        (BigDecimal(this.spendSummary.cost) + BigDecimal(other.spendSummary.cost)).toString,
        (BigDecimal(this.spendSummary.credits) + BigDecimal(other.spendSummary.credits)).toString,
        this.spendSummary.currency,
        this.spendSummary.startTime,
        this.spendSummary.endTime
      )
    )
}

then this code could simply be:

combinedResults = results.flatten.reduceOption((acc, res) => acc + res)

[dvoet]

Suggested change

	.replace("_PROJECT_ID_LIST", "(" + workspaces.map(tuple => s""""${tuple._1.value}"""").mkString(", ") + ")")
	.replace("_PROJECT_ID_LIST", "(" + workspaces.map { case (project, _) => s""""${project.value}"""" }.mkString(", ") + ")")

the _1 syntax is tough, using the curlies and case make it look nicer

[dvoet]

should at least log this exception

[dvoet]

@davidangb jinx