Skip to content

Commit

Permalink
sketch
Browse files Browse the repository at this point in the history
  • Loading branch information
mcovarr committed Sep 13, 2024
1 parent 6f1f9e5 commit f532076
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 37 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ be found [here](https://cromwell.readthedocs.io/en/stable/backends/HPC/#optional
- Fixes the reference disk feature.
- Fixes pulling Docker image metadata from private GCR repositories.
- Fixed `google_project` and `google_compute_service_account` workflow options not taking effect when using GCP Batch backend
- Added a way to use a custom LogsPolicy for the job execution, setting `backend.providers.batch.config.batch.logs-policy` to "CLOUD_LOGGING" (default) keeps the current behavior, or, set it to "PATH" to save the logs into the the mounted disk, at the end, this log file gets copied to the google cloud storage bucket with "task.log" as the name.
- When "CLOUD_LOGGING" is used, many more Cromwell / WDL labels for workflow, root workflow, call, shard etc. are now assigned to GCP Batch log entries.
- A task log file with the name "task.log" that combines standard output and standard error is now streamed to the task directory in Google Cloud Storage.
- Many more Cromwell / WDL labels for workflow, root workflow, call, shard etc. are now assigned to GCP Batch log entries in Cloud Logging.

### Improved handling of Life Sciences API quota errors

Expand Down
7 changes: 5 additions & 2 deletions backend/src/main/scala/cromwell/backend/io/JobPaths.scala
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ trait JobPaths {
def memoryRetryRCFilename: String = "memory_retry_rc"
def defaultStdoutFilename = "stdout"
def defaultStderrFilename = "stderr"
def defaultTaskLogFilename = "task.log"
def isDocker: Boolean = false

// In this non-Docker version of `JobPaths` there is no distinction between host and container roots so this is
Expand Down Expand Up @@ -73,7 +74,8 @@ trait JobPaths {
// enable dynamic standard output and error file names for languages like CWL that support this feature.
var standardPaths: StandardPaths = StandardPaths(
output = callExecutionRoot.resolve(defaultStdoutFilename),
error = callExecutionRoot.resolve(defaultStderrFilename)
error = callExecutionRoot.resolve(defaultStderrFilename),
taskLog = callExecutionRoot.resolve(defaultTaskLogFilename)
)

lazy val script = callExecutionRoot.resolve(scriptFilename)
Expand All @@ -85,7 +87,8 @@ trait JobPaths {
// standard output and error file names.
def standardOutputAndErrorPaths: Map[String, Path] = Map(
CallMetadataKeys.Stdout -> standardPaths.output,
CallMetadataKeys.Stderr -> standardPaths.error
CallMetadataKeys.Stderr -> standardPaths.error,
CallMetadataKeys.TaskLog -> standardPaths.taskLog
)

private lazy val commonMetadataPaths: Map[String, Path] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ trait StandardAsyncExecutionActor
instantiatedCommand.evaluatedStdoutOverride.getOrElse(jobPaths.defaultStdoutFilename) |> absolutizeContainerPath
def executionStderr: String =
instantiatedCommand.evaluatedStderrOverride.getOrElse(jobPaths.defaultStderrFilename) |> absolutizeContainerPath
def executionTaskLog: String = jobPaths.defaultTaskLogFilename |> absolutizeContainerPath

/*
* Ensures the standard paths are correct w.r.t overridden paths. This is called in two places: when generating the command and
Expand All @@ -400,9 +401,10 @@ trait StandardAsyncExecutionActor
// .get's are safe on stdout and stderr after falling back to default names above.
jobPaths.standardPaths = StandardPaths(
output = hostPathFromContainerPath(executionStdout),
error = hostPathFromContainerPath(executionStderr)
error = hostPathFromContainerPath(executionStderr),
taskLog = hostPathFromContainerPath(executionTaskLog)
)
// Re-publish stdout and stderr paths that were possibly just updated.
// Re-publish stdout, stderr and task log paths that were possibly just updated.
tellMetadata(jobPaths.standardOutputAndErrorPaths)
jobPathsUpdated = true
}
Expand Down Expand Up @@ -430,6 +432,7 @@ trait StandardAsyncExecutionActor
val stdinRedirection = executionStdin.map("< " + _.shellQuote).getOrElse("")
val stdoutRedirection = executionStdout.shellQuote
val stderrRedirection = executionStderr.shellQuote
val taskLogRedirection = executionTaskLog.shellQuote
val rcTmpPath = rcPath.plusExt("tmp")

val errorOrDirectoryOutputs: ErrorOr[List[WomUnlistedDirectory]] =
Expand Down Expand Up @@ -498,6 +501,7 @@ trait StandardAsyncExecutionActor
|touch $stdoutRedirection $stderrRedirection
|tee $stdoutRedirection < "$$$out" &
|tee $stderrRedirection < "$$$err" >&2 &
|tail -q -f $stdoutRedirection $stderrRedirection > $taskLogRedirection &
|(
|cd ${cwd.pathAsString}
|ENVIRONMENT_VARIABLES
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/cromwell/core/core.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import mouse.boolean._
import scala.concurrent.duration.FiniteDuration
import scala.util.control.NoStackTrace

case class StandardPaths(output: Path, error: Path)
case class StandardPaths(output: Path, error: Path, taskLog: Path)

case class CallContext(root: Path, standardPaths: StandardPaths, isDocker: Boolean)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ object CallMetadataKeys {
val Failures = "failures"
val Stdout = "stdout"
val Stderr = "stderr"
val TaskLog = "task.log"
val BackendLogsPrefix = "backendLogs"
val BackendStatus = "backendStatus"
val JobId = "jobId"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -838,16 +838,6 @@ class GcpBatchAsyncBackendJobExecutionActor(override val standardParams: Standar
contentType = plainTextContentType
)

val logFileOutput = GcpBatchFileOutput(
logFilename,
logGcsPath,
DefaultPathBuilder.get(logFilename),
workingDisk,
optional = true,
secondary = false,
contentType = plainTextContentType
)

val memoryRetryRCFileOutput = GcpBatchFileOutput(
memoryRetryRCFilename,
memoryRetryRCGcsPath,
Expand All @@ -864,7 +854,8 @@ class GcpBatchAsyncBackendJobExecutionActor(override val standardParams: Standar

val standardStreams = List(
StandardStream("stdout", _.output),
StandardStream("stderr", _.error)
StandardStream("stderr", _.error),
StandardStream("taskLog", _.taskLog)
) map { s =>
GcpBatchFileOutput(
s.name,
Expand All @@ -888,8 +879,7 @@ class GcpBatchAsyncBackendJobExecutionActor(override val standardParams: Standar
DetritusOutputParameters(
monitoringScriptOutputParameter = monitoringOutput,
rcFileOutputParameter = rcFileOutput,
memoryRetryRCFileOutputParameter = memoryRetryRCFileOutput,
logFileOutputParameter = logFileOutput
memoryRetryRCFileOutputParameter = memoryRetryRCFileOutput
),
List.empty
)
Expand All @@ -908,10 +898,7 @@ class GcpBatchAsyncBackendJobExecutionActor(override val standardParams: Standar
runtimeAttributes = runtimeAttributes,
batchAttributes = batchAttributes,
projectId = batchAttributes.project,
region = batchAttributes.location,
logfile = createParameters.commandScriptContainerPath.sibling(
batchParameters.detritusOutputParameters.logFileOutputParameter.name
)
region = batchAttributes.location
)

drsLocalizationManifestCloudPath = jobPaths.callExecutionRoot / GcpBatchJobPaths.DrsLocalizationManifestName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ trait GcpBatchJobCachingActorHelper extends StandardCachingActorHelper {
lazy val memoryRetryRCFilename: String = gcpBatchCallPaths.memoryRetryRCFilename
lazy val memoryRetryRCGcsPath: Path = gcpBatchCallPaths.memoryRetryRC

lazy val logFilename: String = "task.log"
lazy val logGcsPath: Path = gcpBatchCallPaths.callExecutionRoot.resolve(logFilename)

lazy val batchAttributes: GcpBatchConfigurationAttributes = batchConfiguration.batchAttributes

lazy val defaultLabels: Labels = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,9 @@ object GcpBatchRequestFactory {
case class DetritusOutputParameters(
monitoringScriptOutputParameter: Option[GcpBatchFileOutput],
rcFileOutputParameter: GcpBatchFileOutput,
memoryRetryRCFileOutputParameter: GcpBatchFileOutput,
logFileOutputParameter: GcpBatchFileOutput
memoryRetryRCFileOutputParameter: GcpBatchFileOutput
) {
def all: List[GcpBatchFileOutput] = memoryRetryRCFileOutputParameter ::
logFileOutputParameter ::
rcFileOutputParameter ::
monitoringScriptOutputParameter.toList
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,7 @@ class GcpBatchRequestFactoryImpl()(implicit gcsTransferConfiguration: GcsTransfe
case GcpBatchLogsPolicy.CloudLogging =>
LogsPolicy.newBuilder.setDestination(Destination.CLOUD_LOGGING).build
case GcpBatchLogsPolicy.Path =>
LogsPolicy.newBuilder
.setDestination(Destination.PATH)
.setLogsPath(data.gcpBatchParameters.logfile.toString)
.build
???

Check warning on line 246 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/api/GcpBatchRequestFactoryImpl.scala

View check run for this annotation

Codecov / codecov/patch

supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/api/GcpBatchRequestFactoryImpl.scala#L246

Added line #L246 was not covered by tests
}

val googleLabels = data.createParameters.googleLabels.map(l => Label(l.key, l.value))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package cromwell.backend.google.batch.models

import cromwell.backend.BackendJobDescriptor
import cromwell.core.path.Path

case class CreateGcpBatchParameters(jobDescriptor: BackendJobDescriptor,
runtimeAttributes: GcpBatchRuntimeAttributes,
batchAttributes: GcpBatchConfigurationAttributes,
projectId: String,
region: String,
logfile: Path
region: String
)

0 comments on commit f532076

Please sign in to comment.