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

improvement(core): better action lifecycle logs #5428

Merged
merged 2 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 0 additions & 12 deletions core/src/actions/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
46 changes: 11 additions & 35 deletions core/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Copy link
Collaborator Author

@eysi09 eysi09 Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the log level name to the log line proper.

So now it's:

section => [level name] message

as opposed to:

section [level name] => message

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 {
Expand All @@ -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 ""
}
Expand Down
49 changes: 14 additions & 35 deletions core/src/logger/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just plain wrong so I removed it.

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,
}

/**
Expand All @@ -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,
}
6 changes: 3 additions & 3 deletions core/src/logger/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -74,7 +74,7 @@ interface DividerOpts {
width?: number
char?: string
titlePadding?: number
color?: Styles
color?: Chalk
title?: string
padding?: number
}
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions core/src/plugins/kubernetes/container/build/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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 }
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/plugins/kubernetes/nginx/default-backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 2 additions & 3 deletions core/src/plugins/kubernetes/nginx/nginx-helm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
}

Expand All @@ -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"
}
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/plugins/kubernetes/nginx/nginx-kind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 1 addition & 2 deletions core/src/plugins/kubernetes/nginx/nginx-microk8s.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
}

Expand Down
5 changes: 2 additions & 3 deletions core/src/plugins/kubernetes/nginx/nginx-minikube.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down