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(logger): some minor fixes #5475

Merged
merged 1 commit into from
Nov 24, 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: 9 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,17 @@ ${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 +286,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
30 changes: 23 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,18 @@ 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 +1979,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