Skip to content

Commit

Permalink
improvement(logger): some minor fixes
Browse files Browse the repository at this point in the history
Particularly:
- Setting logs for some built-in providers to debug because that's just
  more fitting
- Use the "correct" log context when rendering new lines for graph and
  provider logs. If we use this.log the newlines are visible in the dev
  console where they shouldn't be because that's in the context of
  "internal" commands
  • Loading branch information
eysi09 committed Nov 23, 2023
1 parent a7c7816 commit b0f3f2c
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 33 deletions.
11 changes: 8 additions & 3 deletions core/src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,6 @@ ${renderCommands(commands)}
} = parsedOpts

const parsedCliVars = parseCliVarFlags(cliVars)
const gardenLog = log.createLog({ name: "garden" })

// Some commands may set their own logger type so we update the logger config here,
// once we've resolved the command.
const commandLoggerType = command.getTerminalWriterType({ opts: parsedOpts, args: parsedArgs })
Expand All @@ -240,10 +238,16 @@ ${renderCommands(commands)}
const sessionId = uuidv4()

return withSessionContext({ sessionId }, async () => {
const gardenLog = log.createLog({ name: "garden", showDuration: true })
// Log context for printing the start and finish of Garden initialization when not using the dev console
const gardenInitLog = command.getFullName() !== "dev" ? log.createLog({ name: "garden", showDuration: true }) : null

// Init Cloud API (if applicable)
let cloudApi: CloudApi | undefined

gardenLog.info("Initializing...")
if (gardenInitLog) {
gardenInitLog.info("Initializing...")
}
if (!command.noProject) {
const config = await this.getProjectConfig(log, workingDir)
const cloudDomain = getGardenCloudDomain(config?.domain)
Expand Down Expand Up @@ -281,6 +285,7 @@ ${renderCommands(commands)}
commandInfo,
environmentString: environmentName,
log,
gardenInitLog: gardenInitLog || undefined,
forceRefresh,
variableOverrides: parsedCliVars,
plugins: this.plugins,
Expand Down
13 changes: 10 additions & 3 deletions core/src/commands/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { Garden } from "../garden.js"
import type { Log } from "../logger/log-entry.js"
import type { LoggerType, LoggerBase, LoggerConfigBase, LogLevel } from "../logger/logger.js"
import { eventLogLevel } from "../logger/logger.js"
import { printFooter } from "../logger/util.js"
import { printEmoji, printFooter } from "../logger/util.js"
import { getCloudDistributionName, getCloudLogSectionName } from "../util/cloud.js"
import { getDurationMsec, getPackageVersion, userPrompt } from "../util/util.js"
import { renderOptions, renderCommands, renderArguments, cliStyles, optionsWithAliasValues } from "../cli/helpers.js"
Expand Down Expand Up @@ -284,6 +284,7 @@ export abstract class Command<
const log = overrideLogLevel ? garden.log.createLog({ fixLevel: overrideLogLevel }) : garden.log

let cloudSession: CloudSession | undefined
let cloudLinkMsg: string | undefined

// Session registration for the `dev` and `serve` commands is handled in the `serve` command's `action` method,
// so we skip registering here to avoid duplication.
Expand Down Expand Up @@ -317,8 +318,10 @@ export abstract class Command<
shortId: cloudSession.shortId,
}).href
const cloudLog = log.createLog({ name: getCloudLogSectionName(distroName) })

cloudLog.info(`View command results at: ${styles.link(commandResultUrl)}\n`)
cloudLinkMsg = `View command results at: \n\n${printEmoji("👉", log)}${styles.link(
commandResultUrl
)} ${printEmoji("👈", log)} \n`
cloudLog.info(cloudLinkMsg)
}

let analytics: AnalyticsHandler | undefined
Expand Down Expand Up @@ -430,6 +433,10 @@ export abstract class Command<
// fire, which may be needed to e.g. capture monitors added in event handlers
await waitForOutputFlush()

if (cloudLinkMsg) {
log.info("\n" + cloudLinkMsg)
}

return result
})
)
Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class LoginCommand extends Command<{}, Opts> {
const cloudApi = await CloudApi.factory({ log, cloudDomain, skipLogging: true, globalConfigStore })

if (cloudApi) {
log.info({ msg: `You're already logged in to ${cloudDomain}.` })
log.success({ msg: `You're already logged in to ${cloudDomain}.` })
cloudApi.close()
return {}
}
Expand All @@ -114,7 +114,7 @@ export class LoginCommand extends Command<{}, Opts> {
log.info({ msg: `Logging in to ${cloudDomain}...` })
const tokenResponse = await login(log, cloudDomain, garden.events)
await CloudApi.saveAuthToken(log, globalConfigStore, tokenResponse, cloudDomain)
log.info({ msg: `Successfully logged in to ${cloudDomain}.` })
log.success({ msg: `Successfully logged in to ${cloudDomain}.`, showDuration: false })

return {}
}
Expand Down
28 changes: 21 additions & 7 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ import { configureNoOpExporter } from "./util/open-telemetry/tracing.js"
import { detectModuleOverlap, makeOverlapErrors } from "./util/module-overlap.js"
import { GotHttpError } from "./util/http.js"
import { styles } from "./logger/styles.js"
import { renderDuration } from "./logger/util.js"

const defaultLocalAddress = "localhost"

Expand All @@ -171,6 +172,11 @@ export interface GardenOpts {
globalConfigStore?: GlobalConfigStore
legacyBuildSync?: boolean
log?: Log
/**
* Log context for logging the start and finish of the Garden class
* initialization with duration.
*/
gardenInitLog?: Log
monitors?: MonitorManager
noEnterprise?: boolean
persistent?: boolean
Expand All @@ -195,6 +201,7 @@ export interface GardenParams {
globalConfigStore?: GlobalConfigStore
localConfigStore?: LocalConfigStore
log: Log
gardenInitLog?: Log
moduleIncludePatterns?: string[]
moduleExcludePatterns?: string[]
monitors?: MonitorManager
Expand Down Expand Up @@ -226,6 +233,7 @@ interface GardenInstanceState {
@Profile()
export class Garden {
public log: Log
private gardenInitLog?: Log
private loadedPlugins?: GardenPluginSpec[]
protected actionConfigs: ActionConfigMap
protected moduleConfigs: ModuleConfigMap
Expand Down Expand Up @@ -303,6 +311,7 @@ export class Garden {
this.namespace = params.namespace
this.gardenDirPath = params.gardenDirPath
this.log = params.log
this.gardenInitLog = params.gardenInitLog
this.artifactsPath = params.artifactsPath
this.vcsInfo = params.vcsInfo
this.opts = params.opts
Expand Down Expand Up @@ -841,7 +850,7 @@ export class Garden {
this.resolvedProviders[provider.name] = provider
}

providerLog.success("Finished initializing providers")
providerLog.success("Finished resolving providers")
if (someCached || allCached) {
const msg = allCached ? "All" : "Some"
providerLog.info(
Expand All @@ -850,9 +859,6 @@ export class Garden {
}

providerLog.silly(() => `Resolved providers: ${providers.map((p) => p.name).join(", ")}`)

// Print a new line after resolving providers
this.log.info("")
})

return keyBy(providers, "name")
Expand Down Expand Up @@ -1154,9 +1160,16 @@ export class Garden {
// This event is internal only, not to be streamed
this.events.emit("configGraph", { graph })

graphLog.success("Done")
// Print a new line after resolving graph
this.log.info("")
graphLog.success("Finished resolving graph")

// Log Garden class initialization exactly once with duration. This basically assumes that
// Garden is ready after the graph is resolved for the first time. May not be relevant
// for some commands.
if (this.gardenInitLog) {
// We set the duration "manually" instead of using `gardenInitLog.success()` so we can add a new line at the end.
this.gardenInitLog.info(styles.success(`Finished initializing Garden ${renderDuration(this.gardenInitLog.getDuration(1))}\n`))
this.gardenInitLog = undefined
}

return graph.toConfigGraph()
}
Expand Down Expand Up @@ -1964,6 +1977,7 @@ export const resolveGardenParams = profileAsync(async function _resolveGardenPar
dotIgnoreFile: config.dotIgnoreFile,
proxy,
log,
gardenInitLog: opts.gardenInitLog,
moduleIncludePatterns: (config.scan || {}).include,
username: _username,
forceRefresh: opts.forceRefresh,
Expand Down
2 changes: 1 addition & 1 deletion core/src/graph/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export async function resolveAction<T extends Action>({

const results = await garden.processTasks({ tasks: [task], log, throwOnError: true })

log.info(`Done!`)
log.success({ msg: `Done`, showDuration: false })

return <Resolved<T>>(<unknown>results.results.getResult(task)!.result!.outputs.resolvedAction)
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/monitors/port-forward.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class PortForwardMonitor extends Monitor {
this.log.info(
styles.primary(
`Port forward: ` +
styles.underline(proxy.localUrl) +
styles.link(proxy.localUrl) +
` → ${targetHost}:${proxy.spec.targetPort}` +
(proxy.spec.name ? ` (${proxy.spec.name})` : "")
)
Expand Down
2 changes: 1 addition & 1 deletion core/src/mutagen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ export class Mutagen {
}

const syncCount = session.successfulCycles || 0
const description = `from ${sourceDescription} to ${targetDescription}`
const description = `from ${styles.highlight(sourceDescription)} to ${styles.highlight(targetDescription)}`
const isInitialSync = activeSync.lastSyncCount === 0

// Mutagen resets the sync count to zero after resuming from a sync paused
Expand Down
22 changes: 11 additions & 11 deletions core/src/tasks/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,26 +395,26 @@ function displayState(state: ActionState): string {
const actionLogStrings = {
Build: {
ready: "built",
notReady: "will be built",
force: "rebuild",
notReady: `will be ${styles.highlight("built")}`,
force: `will ${styles.highlight("force rebuild")}`,
running: "Building",
},
Deploy: {
ready: "deployed",
notReady: "will be deployed",
force: "redeploy",
notReady: `will be ${styles.highlight("deployed")}`,
force: `will ${styles.highlight("force redeploy")}`,
running: "Deploying",
},
Test: {
ready: "run",
notReady: "test will be run",
force: "rerun test",
notReady: `test will be ${styles.highlight("run")}`,
force: `will ${styles.highlight("force rerun test")}`,
running: "Testing",
},
Run: {
ready: "run",
notReady: "will be run",
force: "rerun",
notReady: `will be ${styles.highlight("run")}`,
force: `will ${styles.highlight("force rerun")}`,
running: "Running",
},
}
Expand Down Expand Up @@ -480,10 +480,10 @@ export function logAndEmitGetStatusEvents<
if (result.state === "ready" && !willRerun) {
log.success({ msg: `Already ${logStrings.ready}`, showDuration: false })
} else if (result.state === "ready" && willRerun) {
log.warn(`${styledName} is already ${logStrings.ready}, will force ${logStrings.force}`)
log.info(`${styledName} is already ${styles.highlight(logStrings.ready)}, ${logStrings.force}`)
} else {
const stateStr = result.detail?.state || displayState(result.state)
log.warn(`Status is '${stateStr}', ${styledName} ${logStrings.notReady}`)
const stateStr = styles.highlight(result.detail?.state || displayState(result.state))
log.info(`Status is '${stateStr}', ${styledName} ${logStrings.notReady}`)
}

// Then an event with the results if the status was successfully retrieved...
Expand Down
11 changes: 7 additions & 4 deletions core/src/tasks/resolve-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { stableStringify } from "../util/string.js"
import { OtelTraced } from "../util/open-telemetry/decorators.js"
import { LogLevel } from "../logger/logger.js"
import type { Log } from "../logger/log-entry.js"
import { styles } from "../logger/styles.js"

/**
* Returns a provider log context with the provider name set.
Expand All @@ -41,8 +42,8 @@ import type { Log } from "../logger/log-entry.js"
* resolved per se. A bit hacky but this is just a cosmetic change.
*/
function getProviderLog(providerName: string, log: Log) {
const verboseLogProviders = ["templated", "container"]
const fixLevel = verboseLogProviders.includes(providerName) ? LogLevel.verbose : undefined
const debugLogProviders = ["templated", "container"]
const fixLevel = debugLogProviders.includes(providerName) ? LogLevel.debug : undefined
return log.createLog({ name: providerName, fixLevel })
}

Expand Down Expand Up @@ -413,8 +414,10 @@ export class ResolveProviderTask extends BaseTask<Provider> {
let status = await handler!({ ctx, log: providerLog })

if (this.forceInit || !status.ready) {
const statusMsg = status.ready ? "Ready, will force re-init" : "Not ready, will init"
providerLog.warn(statusMsg)
const statusMsg = status.ready
? `${styles.highlight("Ready")}, will ${styles.highlight("force re-initialize")}`
: `${styles.highlight("Not ready")}, will initialize`
providerLog.info(statusMsg)
// TODO: avoid calling the handler manually
const prepareHandler = await actions.provider["getPluginHandler"]({
handlerType: "prepareEnvironment",
Expand Down

0 comments on commit b0f3f2c

Please sign in to comment.