From 65653b92e9a6cb4a30042e8cf4426ee280bd766b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Mon, 20 Nov 2023 06:43:03 -0600 Subject: [PATCH] improvement(core): better action lifecycle logs (#5428) * improvement(core): better action lifecycle logs + fix types This commits adds more logging to the action lifecycle. That is logs about getting status of a given action, the results, next steps, etc. The logging is done in the task decorators that are also used for emitting events. The goal is obviously to make the logs more informative but also more consistent. Furthermore, this commit fixes the type for the `styles` map which were way off. This means you can "break out" of the map and call all chalk methods when chaining and it would be nice to narrow that down. But we're going for loose and correct for now. * chore(logger): address PR comments (TBS) --- core/src/actions/helpers.ts | 12 --- core/src/commands/base.ts | 2 +- core/src/garden.ts | 2 +- core/src/logger/renderers.ts | 46 +++------- core/src/logger/styles.ts | 49 +++-------- core/src/logger/util.ts | 6 +- .../kubernetes/container/build/common.ts | 8 +- .../kubernetes/nginx/default-backend.ts | 3 +- .../plugins/kubernetes/nginx/nginx-helm.ts | 5 +- .../plugins/kubernetes/nginx/nginx-kind.ts | 3 +- .../kubernetes/nginx/nginx-microk8s.ts | 3 +- .../kubernetes/nginx/nginx-minikube.ts | 5 +- core/src/tasks/base.ts | 88 +++++++++++++++++-- core/src/tasks/build.ts | 10 +-- core/src/tasks/deploy.ts | 22 ++--- core/src/tasks/run.ts | 28 ++---- core/src/tasks/test.ts | 17 +--- core/test/unit/src/logger/renderers.ts | 15 +--- 18 files changed, 144 insertions(+), 180 deletions(-) diff --git a/core/src/actions/helpers.ts b/core/src/actions/helpers.ts index 4f9b514942..1e054c7a48 100644 --- a/core/src/actions/helpers.ts +++ b/core/src/actions/helpers.ts @@ -101,18 +101,6 @@ export async function warnOnLinkedActions(garden: Garden, log: Log, actions: Act } } -const displayStates = { - failed: "in a failed state", - unknown: "in an unknown state", -} - -/** - * Just to make action states look nicer in print. - */ -export function displayState(state: ActionState) { - return displayStates[state] || state.replace("-", " ") -} - /** * Get the state of an Action */ diff --git a/core/src/commands/base.ts b/core/src/commands/base.ts index afe14920d2..8045b30bfa 100644 --- a/core/src/commands/base.ts +++ b/core/src/commands/base.ts @@ -323,7 +323,7 @@ export abstract class Command< }).href const cloudLog = log.createLog({ name: getCloudLogSectionName(distroName) }) - cloudLog.info(`View command results at: ${styles.highlight(commandResultUrl)}\n`) + cloudLog.info(`View command results at: ${styles.link(commandResultUrl)}\n`) } let analytics: AnalyticsHandler | undefined diff --git a/core/src/garden.ts b/core/src/garden.ts index 95dbfbb162..618e9e4593 100644 --- a/core/src/garden.ts +++ b/core/src/garden.ts @@ -1831,7 +1831,7 @@ export const resolveGardenParams = profileAsync(async function _resolveGardenPar const isCommunityEdition = !config.domain const cloudLog = log.createLog({ name: getCloudLogSectionName(distroName) }) - cloudLog.verbose(`Connecting to ${distroName}...`) + cloudLog.info(`Connecting to ${distroName}...`) cloudProject = await getCloudProject({ cloudApi, diff --git a/core/src/logger/renderers.ts b/core/src/logger/renderers.ts index d643bc36ee..f3a6928403 100644 --- a/core/src/logger/renderers.ts +++ b/core/src/logger/renderers.ts @@ -18,12 +18,12 @@ import { highlightYaml, safeDumpYaml } from "../util/serialization.js" import type { Logger } from "./logger.js" import { logLevelMap, LogLevel } from "./logger.js" import { toGardenError } from "../exceptions.js" -import type { Styles } from "./styles.js" import { styles } from "./styles.js" +import type { Chalk } from "chalk" type RenderFn = (entry: LogEntry, logger: Logger) => string -export const SECTION_PADDING = 20 +export const SECTION_PADDING = 25 export function padSection(section: string, width: number = SECTION_PADDING) { const diff = width - stringWidth(section) @@ -92,7 +92,7 @@ export function renderTimestamp(entry: LogEntry, logger: Logger): string { } export function getStyle(level: LogLevel) { - let style: Styles + let style: Chalk if (level === LogLevel.error) { style = styles.error } else if (level === LogLevel.warn) { @@ -116,20 +116,15 @@ export function getSection(entry: LogEntry): string | null { export function renderMsg(entry: LogEntry): string { const { context, level } = entry - const msg = resolveMsg(entry) - const { origin } = context + const msg = resolveMsg(entry) || "" const style = getStyle(level) - if (!msg) { - return "" - } + // For log levels higher than "info" we print the log level name. + const logLevelName = entry.level > LogLevel.info ? `[${logLevelMap[entry.level]}] ` : "" - // TODO: @eysi Should we strip here? - // if (level > LogLevel.info) { - // msg = stripAnsi(msg) - // } + const origin = context.origin ? `[${styles.italic(context.origin)}] ` : "" - return style(origin ? `[${styles.italic(origin)}] ` + msg : msg) + return style(`${logLevelName}${origin}${msg}`) } export function renderData(entry: LogEntry): string { @@ -146,31 +141,12 @@ export function renderData(entry: LogEntry): string { export function renderSection(entry: LogEntry): string { const { msg } = entry - let section = getSection(entry) - - // For log levels higher than "info" we print the log level name. - // This should technically happen when we render the symbol but it's harder - // to deal with the padding that way. - const logLevelName = styles.secondary(`[${logLevelMap[entry.level]}]`) - - // Just print the log level name directly without padding. E.g: - // ℹ api → Deploying version v-37d6c44559... - // [verbose] Some verbose level stuff that doesn't have a section - if (!section && entry.level > LogLevel.info) { - return logLevelName + " " - } - - // Print the log level name after the section name to preserve alignment. E.g.: - // ℹ api → Deploying version v-37d6c44559... - // ℹ api [verbose] → Some verbose level stuff that has a section - if (entry.level > LogLevel.info) { - section = section ? `${section} ${logLevelName}` : logLevelName - } + const section = getSection(entry) if (section && msg) { - return `${styles.section(padSection(section))} ${styles.accent.bold("→")} ` + return `${padSection(styles.section(section))} ${styles.accent.bold("→")} ` } else if (section) { - return styles.section(padSection(section)) + return padSection(styles.section(section)) } return "" } diff --git a/core/src/logger/styles.ts b/core/src/logger/styles.ts index 74d299613e..11b6420b47 100644 --- a/core/src/logger/styles.ts +++ b/core/src/logger/styles.ts @@ -8,39 +8,18 @@ import chalk from "chalk" -// Helper types for ensuring the consumer of the "styles" map defined below -// can only call the allowed keys when chaining styles. -// Otherwise you could do something like `styles.primary.red` and "break out" -// of the pre-defined styles. -// -// Requires and ugly cast in the maps below but I couldn't find a more elegant -// way to do this with just Typescript. -type ThemeKey = - | "primary" - | "secondary" - | "accent" - | "highlight" - | "highlightSecondary" - | "warning" - | "error" - | "success" -type StyleKey = "bold" | "underline" | "italic" | "link" | "section" | "command" -type StyleFn = (s: string) => string - -export type Styles = StyleFn & { [key in ThemeKey | StyleKey]: Styles } - /** * A map of all the colors we use to render text in the terminal. */ const theme = { - primary: chalk.grey as unknown as Styles, - secondary: chalk.grey as unknown as Styles, - accent: chalk.white as unknown as Styles, - highlight: chalk.cyan as unknown as Styles, - highlightSecondary: chalk.magenta as unknown as Styles, - warning: chalk.yellow as unknown as Styles, - error: chalk.red as unknown as Styles, - success: chalk.green as unknown as Styles, + primary: chalk.grey, + secondary: chalk.grey, + accent: chalk.white, + highlight: chalk.cyan, + highlightSecondary: chalk.magenta, + warning: chalk.yellow, + error: chalk.red, + success: chalk.green, } /** @@ -63,10 +42,10 @@ const theme = { */ export const styles = { ...theme, - bold: chalk.bold as unknown as Styles, - underline: chalk.underline as unknown as Styles, - italic: chalk.italic as unknown as Styles, - link: theme.highlight.underline as unknown as Styles, - section: theme.highlight.italic as unknown as Styles, - command: theme.highlightSecondary.bold as unknown as Styles, + bold: chalk.bold, + underline: chalk.underline, + italic: chalk.italic, + link: theme.highlight.underline, + section: theme.highlight.italic, + command: theme.highlightSecondary.bold, } diff --git a/core/src/logger/util.ts b/core/src/logger/util.ts index 7566279906..16f3099868 100644 --- a/core/src/logger/util.ts +++ b/core/src/logger/util.ts @@ -10,8 +10,8 @@ import hasAnsi from "has-ansi" import dedent from "dedent" import stringWidth from "string-width" import { DEFAULT_BROWSER_DIVIDER_WIDTH } from "../constants.js" -import type { Styles } from "./styles.js" import { styles } from "./styles.js" +import type { Chalk } from "chalk" // Add platforms/terminals? export function envSupportsEmoji() { @@ -74,7 +74,7 @@ interface DividerOpts { width?: number char?: string titlePadding?: number - color?: Styles + color?: Chalk title?: string padding?: number } @@ -144,7 +144,7 @@ export function renderMessageWithDivider({ prefix: string msg: string isError: boolean - color?: Styles + color?: Chalk }) { // Allow overwriting color as an escape hatch. Otherwise defaults to white or red in case of errors. const msgColor = color || (isError ? styles.error : styles.accent) diff --git a/core/src/plugins/kubernetes/container/build/common.ts b/core/src/plugins/kubernetes/container/build/common.ts index eea6254b0d..59ca5020d2 100644 --- a/core/src/plugins/kubernetes/container/build/common.ts +++ b/core/src/plugins/kubernetes/container/build/common.ts @@ -32,6 +32,7 @@ import { getRunningDeploymentPod } from "../../util.js" import { buildSyncVolumeName, dockerAuthSecretKey, k8sUtilImageName, rsyncPortName } from "../../constants.js" import { styles } from "../../../../logger/styles.js" import type { StringMap } from "../../../../config/common.js" +import { LogLevel } from "../../../../logger/logger.js" export const inClusterBuilderServiceAccount = "garden-in-cluster-builder" export const sharedBuildSyncDeploymentName = "garden-build-sync" @@ -95,8 +96,11 @@ export async function syncToBuildSync(params: SyncToSharedBuildSyncParams) { // Sync using mutagen const key = `k8s--build-sync--${ctx.environmentName}--${namespace}--${action.name}--${randomString(8)}` const targetPath = `/data/${ctx.workingCopyId}/${action.name}` + // We print the sync logs from Mutagen at a higher level for builds + const mutagenLog = log.createLog({ fixLevel: LogLevel.verbose }) + const mutagen = new Mutagen({ ctx, log: mutagenLog }) - const mutagen = new Mutagen({ ctx, log }) + const syncLog = log.createLog().info(`Syncing build context to cluster...`) // Make sure the target path exists const runner = new PodRunner({ @@ -153,7 +157,7 @@ export async function syncToBuildSync(params: SyncToSharedBuildSyncParams) { log.debug(`Sync connection terminated`) } - log.info("File sync to cluster complete") + syncLog.success("File sync to cluster complete") return { contextRelPath, contextPath, dataPath } } diff --git a/core/src/plugins/kubernetes/nginx/default-backend.ts b/core/src/plugins/kubernetes/nginx/default-backend.ts index a3aca3935a..0589e45e90 100644 --- a/core/src/plugins/kubernetes/nginx/default-backend.ts +++ b/core/src/plugins/kubernetes/nginx/default-backend.ts @@ -11,7 +11,6 @@ import type { Log } from "../../../logger/log-entry.js" import type { DeployState } from "../../../types/service.js" import { KubeApi } from "../api.js" import { checkResourceStatus, waitForResources } from "../status/status.js" -import chalk from "chalk" import type { KubernetesDeployment, KubernetesService } from "../types.js" import { defaultGardenIngressControllerDefaultBackendImage } from "../constants.js" import { GardenIngressComponent } from "./ingress-controller-base.js" @@ -51,7 +50,7 @@ export class GardenDefaultBackend extends GardenIngressComponent { const { deployment } = defaultBackendGetManifests(ctx) const deploymentStatus = await checkResourceStatus({ api, namespace, manifest: deployment, log }) - log.debug(chalk.yellow(`Status of ingress controller default-backend: ${deploymentStatus}`)) + log.debug(`Status of ingress controller default-backend: ${deploymentStatus}`) return deploymentStatus.state } diff --git a/core/src/plugins/kubernetes/nginx/nginx-helm.ts b/core/src/plugins/kubernetes/nginx/nginx-helm.ts index 5e3e6bb7eb..f1d39bfea8 100644 --- a/core/src/plugins/kubernetes/nginx/nginx-helm.ts +++ b/core/src/plugins/kubernetes/nginx/nginx-helm.ts @@ -6,7 +6,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import chalk from "chalk" import type { Log } from "../../../logger/log-entry.js" import type { DeployState } from "../../../types/service.js" import type { KubernetesPluginContext } from "../config.js" @@ -106,7 +105,7 @@ export abstract class HelmGardenIngressController extends GardenIngressComponent const releaseStatus = statusRes.info?.status || "unknown" if (releaseStatus !== "deployed") { - log.debug(chalk.yellow(`Helm release status for ${HELM_INGRESS_NGINX_RELEASE_NAME}: ${releaseStatus}`)) + log.debug(`Helm release status for ${HELM_INGRESS_NGINX_RELEASE_NAME}: ${releaseStatus}`) return helmStatusMap[releaseStatus] || "unknown" } @@ -116,7 +115,7 @@ export abstract class HelmGardenIngressController extends GardenIngressComponent const deploymentStatus = await checkResourceStatus({ api, namespace, manifest: nginxHelmMainResource, log }) return deploymentStatus.state } catch (error) { - log.debug(chalk.yellow(`Helm release ${HELM_INGRESS_NGINX_RELEASE_NAME} missing.`)) + log.debug(`Helm release ${HELM_INGRESS_NGINX_RELEASE_NAME} missing.`) return "missing" } } diff --git a/core/src/plugins/kubernetes/nginx/nginx-kind.ts b/core/src/plugins/kubernetes/nginx/nginx-kind.ts index c14c782ede..6d7cd1758d 100644 --- a/core/src/plugins/kubernetes/nginx/nginx-kind.ts +++ b/core/src/plugins/kubernetes/nginx/nginx-kind.ts @@ -10,7 +10,6 @@ import type { Log } from "../../../logger/log-entry.js" import type { KubernetesPluginContext } from "../config.js" import { KubeApi } from "../api.js" import { checkResourceStatus, waitForResources } from "../status/status.js" -import chalk from "chalk" import { apply, deleteResources } from "../kubectl.js" import type { DeployState } from "../../../types/service.js" import { kindNginxGetManifests } from "./nginx-kind-manifests.js" @@ -61,7 +60,7 @@ export class KindGardenIngressController extends GardenIngressComponent { const deploymentStatus = await checkResourceStatus({ api, namespace, manifest: nginxKindMainResource, log }) - log.debug(chalk.yellow(`Status of ingress controller: ${deploymentStatus.state}`)) + log.debug(`Status of ingress controller: ${deploymentStatus.state}`) return deploymentStatus.state } diff --git a/core/src/plugins/kubernetes/nginx/nginx-microk8s.ts b/core/src/plugins/kubernetes/nginx/nginx-microk8s.ts index d9b73f30a2..11f6a4ae3d 100644 --- a/core/src/plugins/kubernetes/nginx/nginx-microk8s.ts +++ b/core/src/plugins/kubernetes/nginx/nginx-microk8s.ts @@ -8,7 +8,6 @@ import type { Log } from "../../../logger/log-entry.js" import { exec } from "../../../util/util.js" -import chalk from "chalk" import type { KubernetesPluginContext } from "../config.js" import { type DeployState } from "../../../types/service.js" import { configureMicrok8sAddons } from "../local/microk8s.js" @@ -50,7 +49,7 @@ export class Microk8sGardenIngressController extends GardenIngressComponent { const statusCommandResult = await exec("microk8s", ["status", "--format", "short"]) const status = statusCommandResult.stdout const addonEnabled = status.includes("core/ingress: enabled") - log.debug(chalk.yellow(`Status of microk8s ingress controller addon: ${addonEnabled ? "enabled" : "disabled"}`)) + log.debug(`Status of microk8s ingress controller addon: ${addonEnabled ? "enabled" : "disabled"}`) return addonEnabled ? "ready" : "missing" } diff --git a/core/src/plugins/kubernetes/nginx/nginx-minikube.ts b/core/src/plugins/kubernetes/nginx/nginx-minikube.ts index 1411ba621d..dbd3869f87 100644 --- a/core/src/plugins/kubernetes/nginx/nginx-minikube.ts +++ b/core/src/plugins/kubernetes/nginx/nginx-minikube.ts @@ -9,7 +9,6 @@ import type { Log } from "../../../logger/log-entry.js" import type { DeployState } from "../../../types/service.js" import { exec } from "../../../util/util.js" -import chalk from "chalk" import type { KubernetesPluginContext } from "../config.js" import { KubeApi } from "../api.js" import { checkResourceStatus, waitForResources } from "../status/status.js" @@ -45,7 +44,7 @@ export class MinikubeGardenIngressController extends GardenIngressComponent { const addonEnabled = minikubeAddons.ingress.Status === "enabled" if (!addonEnabled) { - log.debug(chalk.yellow("Status of minikube ingress controller addon: missing")) + log.debug("Status of minikube ingress controller addon: missing") return "missing" } //check if ingress controller deployment is ready @@ -55,7 +54,7 @@ export class MinikubeGardenIngressController extends GardenIngressComponent { manifest: nginxKindMainResource, log, }) - log.debug(chalk.yellow(`Status of minikube ingress controller addon: ${deploymentStatus.state}`)) + log.debug(`Status of minikube ingress controller addon: ${deploymentStatus.state}`) return deploymentStatus.state } diff --git a/core/src/tasks/base.ts b/core/src/tasks/base.ts index 6b96ea435b..ad83f924a4 100644 --- a/core/src/tasks/base.ts +++ b/core/src/tasks/base.ts @@ -34,6 +34,7 @@ import { makeActionProcessingPayload, makeActionGetStatusPayload, } from "../events/util.js" +import { styles } from "../logger/styles.js" export function makeBaseKey(type: string, name: string) { return `${type}.${name}` @@ -376,13 +377,56 @@ const actionKindToEventNameMap = { run: "runStatus", } satisfies { [key in ExecuteActionTaskType]: ActionStatusEventName } +const displayStates = { + failed: "in a failed state", + unknown: "in an unknown state", +} + +/** + * Just to make action states look nicer in print. + */ +function displayState(state: ActionState): string { + return displayStates[state] || state.replace("-", " ") +} + +/*+ + * Map of log strings used for logging the action lifecycle. + */ +const actionLogStrings = { + Build: { + ready: "built", + notReady: "will be built", + force: "rebuild", + running: "Building", + }, + Deploy: { + ready: "deployed", + notReady: "will be deployed", + force: "redeploy", + running: "Deploying", + }, + Test: { + ready: "run", + notReady: "test will be run", + force: "rerun test", + running: "Testing", + }, + Run: { + ready: "run", + notReady: "will be run", + force: "rerun", + running: "Running", + }, +} + /** * Decorator function for emitting status events to Cloud when calling the - * getStatus method on ExecutionAction tasks. + * getStatus method on ExecutionAction tasks and for logging the operation lifecycle + * to the terminal. * * The wrapper emits the appropriate events before and after the inner function execution. */ -export function emitGetStatusEvents< +export function logAndEmitGetStatusEvents< A extends Action, R extends ValidExecutionActionResultType = { state: ActionState @@ -403,16 +447,20 @@ export function emitGetStatusEvents< descriptor.value = async function (this: ExecuteActionTask, ...args: [ActionTaskStatusParams]) { const statusOnly = args[0].statusOnly - // We don't emit events when just checking the status if (statusOnly) { const result = (await method.apply(this, args)) as R & ExecuteActionOutputs return result } - const actionKind = this.action.kind.toLowerCase() as Lowercase - const eventName = actionKindToEventNameMap[actionKind] + const log = this.log.createLog() + const actionKindLowercased = this.action.kind.toLowerCase() as Lowercase + const eventName = actionKindToEventNameMap[actionKindLowercased] const startedAt = new Date().toISOString() + const styledName = styles.highlight(this.action.name) + const logStrings = actionLogStrings[this.action.kind] + + log.info(`Getting status for ${this.action.kind} ${styledName} (type ${styles.highlight(this.action.type)})...`) // First we emit the "getting-status" event this.garden.events.emit( @@ -428,6 +476,16 @@ export function emitGetStatusEvents< try { const result = (await method.apply(this, args)) as R & ExecuteActionOutputs + const willRerun = this.force && !statusOnly + if (result.state === "ready" && !willRerun) { + log.success({ msg: `Already ${logStrings.ready}`, showDuration: false }) + } else if (result.state === "ready" && willRerun) { + log.info(`${styledName} is already ${logStrings.ready}, will force ${logStrings.force}`) + } else { + const stateStr = result.detail?.state || displayState(result.state) + log.warn(`Status is '${stateStr}', ${styledName} ${logStrings.notReady}`) + } + // Then an event with the results if the status was successfully retrieved... const donePayload = makeActionCompletePayload({ result, @@ -443,6 +501,9 @@ export function emitGetStatusEvents< return result } catch (err) { // ...otherwise we emit a "failed" event + + // The error proper is logged downstream + log.error("Failed") this.garden.events.emit( eventName, makeActionFailedPayload({ @@ -463,11 +524,12 @@ export function emitGetStatusEvents< /** * Decorator function for emitting status events to Cloud when calling the - * process method on ExecutionAction tasks. + * process method on ExecutionAction tasks and for logging the operation lifecycle + * to the terminal. * * The wrapper emits the appropriate events before and after the inner function execution. */ -export function emitProcessingEvents< +export function logAndEmitProcessingEvents< A extends Action, R extends ValidExecutionActionResultType = { state: ActionState @@ -492,6 +554,14 @@ export function emitProcessingEvents< const actionKind = this.action.kind.toLowerCase() as Lowercase const eventName = actionKindToEventNameMap[actionKind] const startedAt = new Date().toISOString() + const log = this.log.createLog() + const version = this.action.versionString() + const logStrings = actionLogStrings[this.action.kind] + log.info( + `${logStrings.running} ${styles.highlight(this.action.name)} (type ${styles.highlight( + this.action.type + )}) at version ${styles.highlight(version)}...` + ) // First we emit the "processing" event this.garden.events.emit( @@ -518,10 +588,14 @@ export function emitProcessingEvents< }) as Events[typeof eventName] this.garden.events.emit(eventName, donePayload) + log.success("Done") return result } catch (err) { // ...otherwise we emit a "failed" event + + // The error proper is logged downstream + log.error("Failed") this.garden.events.emit( eventName, makeActionFailedPayload({ diff --git a/core/src/tasks/build.ts b/core/src/tasks/build.ts index 46f11a1ac7..945d6eb0e5 100644 --- a/core/src/tasks/build.ts +++ b/core/src/tasks/build.ts @@ -7,7 +7,7 @@ */ import type { BaseActionTaskParams, ActionTaskProcessParams, ActionTaskStatusParams } from "../tasks/base.js" -import { ExecuteActionTask, emitGetStatusEvents, emitProcessingEvents } from "../tasks/base.js" +import { ExecuteActionTask, logAndEmitGetStatusEvents, logAndEmitProcessingEvents } from "../tasks/base.js" import { Profile } from "../util/profiling.js" import type { BuildAction, BuildActionConfig, ResolvedBuildAction } from "../actions/build.js" import pluralize from "pluralize" @@ -38,7 +38,7 @@ export class BuildTask extends ExecuteActionTask { } }, }) - @(emitGetStatusEvents) + @(logAndEmitGetStatusEvents) async getStatus({ statusOnly, dependencyResults }: ActionTaskStatusParams) { const router = await this.garden.getActionRouter() const action = this.getResolvedAction(this.action, dependencyResults) @@ -47,7 +47,6 @@ export class BuildTask extends ExecuteActionTask { const status = output.result if (status.state === "ready" && !statusOnly && !this.force) { - this.log.info(`Already built`) await this.ensureBuildContext(action) } @@ -65,7 +64,7 @@ export class BuildTask extends ExecuteActionTask { } }, }) - @(emitProcessingEvents) + @(logAndEmitProcessingEvents) async process({ dependencyResults }: ActionTaskProcessParams) { const router = await this.garden.getActionRouter() const action = this.getResolvedAction(this.action, dependencyResults) @@ -87,7 +86,6 @@ export class BuildTask extends ExecuteActionTask { log, }) ) - log.success(`Done`) return { ...result, @@ -95,8 +93,6 @@ export class BuildTask extends ExecuteActionTask { executedAction: resolvedActionToExecuted(action, { status: result }), } } catch (err) { - log.error(`Build failed`) - throw err } } diff --git a/core/src/tasks/deploy.ts b/core/src/tasks/deploy.ts index 0dc99ff344..e83be4d4e1 100644 --- a/core/src/tasks/deploy.ts +++ b/core/src/tasks/deploy.ts @@ -7,12 +7,12 @@ */ import type { BaseActionTaskParams, BaseTask, ActionTaskProcessParams, ActionTaskStatusParams } from "./base.js" -import { ExecuteActionTask, emitGetStatusEvents, emitProcessingEvents } from "./base.js" +import { ExecuteActionTask, logAndEmitGetStatusEvents, logAndEmitProcessingEvents } from "./base.js" import { getLinkUrl } from "../types/service.js" import { Profile } from "../util/profiling.js" import type { DeployAction } from "../actions/deploy.js" import type { DeployStatus } from "../plugin/handlers/Deploy/get-status.js" -import { displayState, resolvedActionToExecuted } from "../actions/helpers.js" +import { resolvedActionToExecuted } from "../actions/helpers.js" import type { PluginEventBroker } from "../plugin-context.js" import type { ActionLog } from "../logger/log-entry.js" import { OtelTraced } from "../util/open-telemetry/decorators.js" @@ -65,7 +65,7 @@ export class DeployTask extends ExecuteActionTask { } }, }) - @(emitGetStatusEvents) + @(logAndEmitGetStatusEvents) async getStatus({ statusOnly, dependencyResults }: ActionTaskStatusParams) { const log = this.log.createLog() const action = this.getResolvedAction(this.action, dependencyResults) @@ -82,14 +82,8 @@ export class DeployTask extends ExecuteActionTask { status.state = "not-ready" } - if (!statusOnly && !this.force) { - if (status.state === "ready") { - log.success({ msg: `Already deployed`, showDuration: false }) - printIngresses(status, log) - } else { - const state = status.detail?.state || displayState(status.state) - log.info(state) - } + if (!statusOnly && !this.force && status.state === "ready") { + printIngresses(status, log) } const executedAction = resolvedActionToExecuted(action, { status }) @@ -113,7 +107,7 @@ export class DeployTask extends ExecuteActionTask { } }, }) - @(emitProcessingEvents) + @(logAndEmitProcessingEvents) async process({ dependencyResults, status }: ActionTaskProcessParams) { const action = this.getResolvedAction(this.action, dependencyResults) const version = action.versionString() @@ -121,7 +115,6 @@ export class DeployTask extends ExecuteActionTask { const router = await this.garden.getActionRouter() const log = this.log.createLog() - log.info(`Deploying version ${version}...`) try { const output = await router.deploy.deploy({ @@ -133,12 +126,9 @@ export class DeployTask extends ExecuteActionTask { }) status = output.result } catch (err) { - log.error(`Failed`) throw err } - log.success(`Done`) - const executedAction = resolvedActionToExecuted(action, { status }) printIngresses(status, log) diff --git a/core/src/tasks/run.ts b/core/src/tasks/run.ts index 455e7f0a43..2a290eaaa8 100644 --- a/core/src/tasks/run.ts +++ b/core/src/tasks/run.ts @@ -7,7 +7,7 @@ */ import type { BaseActionTaskParams, ActionTaskProcessParams, ActionTaskStatusParams } from "./base.js" -import { ExecuteActionTask, emitGetStatusEvents, emitProcessingEvents } from "./base.js" +import { ExecuteActionTask, logAndEmitGetStatusEvents, logAndEmitProcessingEvents } from "./base.js" import { Profile } from "../util/profiling.js" import type { RunAction } from "../actions/run.js" import type { GetRunResult } from "../plugin/handlers/Run/get-result.js" @@ -46,9 +46,8 @@ export class RunTask extends ExecuteActionTask { } }, }) - @(emitGetStatusEvents) - async getStatus({ statusOnly, dependencyResults }: ActionTaskStatusParams) { - this.log.verbose("Checking status...") + @(logAndEmitGetStatusEvents) + async getStatus({ dependencyResults }: ActionTaskStatusParams) { const router = await this.garden.getActionRouter() const action = this.getResolvedAction(this.action, dependencyResults) @@ -60,8 +59,6 @@ export class RunTask extends ExecuteActionTask { log: this.log, }) - this.log.verbose(`Status check complete`) - if (status.detail === null) { return { ...status, @@ -71,18 +68,12 @@ export class RunTask extends ExecuteActionTask { } } - if (status.state === "ready" && !statusOnly && !this.force) { - this.log.success("Already complete") - } - return { ...status, version: action.versionString(), executedAction: resolvedActionToExecuted(action, { status }), } } catch (err) { - this.log.error(`Failed getting status`) - throw err } } @@ -98,12 +89,10 @@ export class RunTask extends ExecuteActionTask { } }, }) - @(emitProcessingEvents) + @(logAndEmitProcessingEvents) async process({ dependencyResults }: ActionTaskProcessParams) { const action = this.getResolvedAction(this.action, dependencyResults) - - const taskLog = this.log.createLog().info("Running...") - + const taskLog = this.log.createLog() const actions = await this.garden.getActionRouter() let status: GetRunResult @@ -117,14 +106,9 @@ export class RunTask extends ExecuteActionTask { }) status = output.result } catch (err) { - taskLog.error(`Failed running ${action.name}`) - throw err } - if (status.state === "ready") { - taskLog.success(`Done`) - } else { - taskLog.error(`Failed!`) + if (status.state !== "ready") { if (status.detail?.diagnosticErrorMsg) { this.log.debug(`Additional context for the error:\n\n${status.detail.diagnosticErrorMsg}`) } diff --git a/core/src/tasks/test.ts b/core/src/tasks/test.ts index 3132accb9a..0c34c46c89 100644 --- a/core/src/tasks/test.ts +++ b/core/src/tasks/test.ts @@ -7,7 +7,7 @@ */ import type { BaseActionTaskParams, ActionTaskProcessParams, ActionTaskStatusParams } from "../tasks/base.js" -import { ExecuteActionTask, emitGetStatusEvents, emitProcessingEvents } from "../tasks/base.js" +import { ExecuteActionTask, logAndEmitGetStatusEvents, logAndEmitProcessingEvents } from "../tasks/base.js" import { Profile } from "../util/profiling.js" import { resolvedActionToExecuted } from "../actions/helpers.js" import type { TestAction } from "../actions/test.js" @@ -64,9 +64,8 @@ export class TestTask extends ExecuteActionTask { } }, }) - @(emitGetStatusEvents) + @(logAndEmitGetStatusEvents) async getStatus({ dependencyResults }: ActionTaskStatusParams) { - this.log.verbose("Checking status...") const action = this.getResolvedAction(this.action, dependencyResults) const router = await this.garden.getActionRouter() @@ -76,17 +75,12 @@ export class TestTask extends ExecuteActionTask { action, }) - this.log.verbose("Status check complete") - const testResult = status?.detail const version = action.versionString() const executedAction = resolvedActionToExecuted(action, { status }) if (testResult && testResult.success) { - if (!this.force) { - this.log.success("Already passed") - } return { ...status, version, @@ -111,12 +105,10 @@ export class TestTask extends ExecuteActionTask { } }, }) - @(emitProcessingEvents) + @(logAndEmitProcessingEvents) async process({ dependencyResults }: ActionTaskProcessParams) { const action = this.getResolvedAction(this.action, dependencyResults) - this.log.info(`Running...`) - const router = await this.garden.getActionRouter() let status: GetTestResult @@ -130,12 +122,9 @@ export class TestTask extends ExecuteActionTask { }) status = output.result } catch (err) { - this.log.error(`Failed running test`) - throw err } if (status.detail?.success) { - this.log.success(`Success`) } else { const exitCode = status.detail?.exitCode const failedMsg = !!exitCode ? `Failed with code ${exitCode}!` : `Failed!` diff --git a/core/test/unit/src/logger/renderers.ts b/core/test/unit/src/logger/renderers.ts index 3cdcb365e5..590cffdbb8 100644 --- a/core/test/unit/src/logger/renderers.ts +++ b/core/test/unit/src/logger/renderers.ts @@ -17,7 +17,6 @@ import { formatForJson, SECTION_PADDING, renderData, - padSection, renderSection, } from "../../../../src/logger/renderers.js" import { GenericGardenError } from "../../../../src/exceptions.js" @@ -121,17 +120,7 @@ describe("renderers", () => { it("should print the log level if it's higher then 'info'", () => { const entry = logger.createLog().debug({ msg: "hello world" }).getLatestEntry() - expect(formatForTerminal(entry, logger)).to.equal( - `${styles.primary("[debug]")} ${styles.primary("hello world")}\n` - ) - }) - it("should print the log level if it's higher then 'info' after the section if there is one", () => { - const entry = logger.createLog({ name: "foo" }).debug("hello world").getLatestEntry() - - const section = `foo ${styles.primary("[debug]")}` - expect(formatForTerminal(entry, logger)).to.equal( - `${logSymbols["info"]} ${styles.highlight.italic(padSection(section))} → ${styles.primary("hello world")}\n` - ) + expect(formatForTerminal(entry, logger)).to.equal(`${styles.secondary("[debug] hello world")}\n`) }) context("basic", () => { before(() => { @@ -142,7 +131,7 @@ describe("renderers", () => { const entry = logger.createLog().info("hello world").getLatestEntry() expect(formatForTerminal(entry, logger)).to.equal( - `${styles.primary(format(now, "HH:mm:ss"))} ${styles.primary("hello world")}\n` + `${styles.secondary(format(now, "HH:mm:ss"))} ${styles.primary("hello world")}\n` ) }) after(() => {