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 34b07fe commit e76a5d9
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 19 deletions.
9 changes: 7 additions & 2 deletions core/src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ ${renderCommands(commands)}
} = parsedOpts

const parsedCliVars = parseCliVarFlags(cliVars)
const gardenLog = log.createLog({ name: "garden" })
const gardenLog = log.createLog({ name: "garden", showDuration: true })
const initLog = command.getFullName() !== "dev" ? log.createLog({ name: "garden", showDuration: true }) : null

// Some commands may set their own logger type so we update the logger config here,
// once we've resolved the command.
Expand All @@ -242,7 +243,10 @@ ${renderCommands(commands)}
// Init Cloud API (if applicable)
let cloudApi: CloudApi | undefined

gardenLog.info("Initializing...")
if (initLog) {
// Print a Garden log before the Cloud logs, just for setting context (skipped in dev console)
initLog.info("Initializing...")
}
if (!command.noProject) {
const config = await this.getProjectConfig(log, workingDir)
const cloudDomain = getGardenCloudDomain(config?.domain)
Expand Down Expand Up @@ -280,6 +284,7 @@ ${renderCommands(commands)}
commandInfo,
environmentString: environmentName,
log,
initLog: initLog || undefined,
forceRefresh,
variableOverrides: parsedCliVars,
plugins: this.plugins,
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
25 changes: 19 additions & 6 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,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 @@ -179,6 +180,7 @@ export interface GardenOpts {
globalConfigStore?: GlobalConfigStore
legacyBuildSync?: boolean
log?: Log
initLog?: Log
monitors?: MonitorManager
noEnterprise?: boolean
persistent?: boolean
Expand All @@ -203,6 +205,7 @@ export interface GardenParams {
globalConfigStore?: GlobalConfigStore
localConfigStore?: LocalConfigStore
log: Log
initLog?: Log
moduleIncludePatterns?: string[]
moduleExcludePatterns?: string[]
monitors?: MonitorManager
Expand Down Expand Up @@ -234,6 +237,7 @@ interface GardenInstanceState {
@Profile()
export class Garden {
public log: Log
private initLog?: Log
private loadedPlugins?: GardenPluginSpec[]
protected actionConfigs: ActionConfigMap
protected moduleConfigs: ModuleConfigMap
Expand Down Expand Up @@ -311,6 +315,7 @@ export class Garden {
this.namespace = params.namespace
this.gardenDirPath = params.gardenDirPath
this.log = params.log
this.initLog = params.initLog
this.artifactsPath = params.artifactsPath
this.vcsInfo = params.vcsInfo
this.opts = params.opts
Expand Down Expand Up @@ -849,7 +854,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 @@ -860,7 +865,7 @@ export class Garden {
providerLog.silly(() => `Resolved providers: ${providers.map((p) => p.name).join(", ")}`)

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

return keyBy(providers, "name")
Expand Down Expand Up @@ -1162,22 +1167,29 @@ 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 the initial Garden class initialization with duration (only once)
if (this.initLog) {
// We set the duration "manually" instead of using `initLog.success()` so we can add a new line at the end.
this.initLog.info(styles.success(`Finished initializing Garden ${renderDuration(this.initLog.getDuration(1))}\n`))
this.initLog = undefined
}

return graph.toConfigGraph()
}

async getResolvedConfigGraph(params: GetConfigGraphParams): Promise<ResolvedConfigGraph> {
const graph = await this.getConfigGraph(params)
const resolved = await this.resolveActions({ graph, actions: graph.getActions(), log: params.log })
return new ResolvedConfigGraph({
const resolvedGraph = new ResolvedConfigGraph({
actions: Object.values(resolved),
moduleGraph: graph.moduleGraph,
// TODO: perhaps groups should be resolved here
groups: graph.getGroups(),
})

return resolvedGraph
}

@OtelTraced({
Expand Down Expand Up @@ -1972,6 +1984,7 @@ export const resolveGardenParams = profileAsync(async function _resolveGardenPar
dotIgnoreFile: config.dotIgnoreFile,
proxy,
log,
initLog: opts.initLog,
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
19 changes: 15 additions & 4 deletions core/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ type RenderFn = (entry: LogEntry, logger: Logger) => string

export const SECTION_PADDING = 25

const symbols = process.env.GARDEN_THEME === "tokyonight" ? {
info: styles.accent('ℹ'),
success: styles.success('✔'),
warning: styles.warning('⚠'),
error: styles.error('✖'),
} : logSymbols

export function padSection(section: string, width: number = SECTION_PADDING) {
const diff = width - stringWidth(section)
return diff <= 0 ? section : section + repeat(" ", diff)
Expand Down Expand Up @@ -76,7 +83,7 @@ export function renderSymbol(entry: LogEntry): string {
symbol = "info"
}

return symbol ? `${logSymbols[symbol]} ` : ""
return symbol ? `${symbols[symbol]} ` : ""
}

export function renderTimestamp(entry: LogEntry, logger: Logger): string {
Expand Down Expand Up @@ -136,9 +143,13 @@ export function renderData(entry: LogEntry): string {
}

export function renderSection(entry: LogEntry): string {
const { msg } = entry
const { msg, data, error } = entry
const section = getSection(entry)

if (!msg) {
return ""
}

if (section && msg) {
return `${padSection(styles.section(section))} ${styles.accent.bold("→")} `
} else if (section) {
Expand All @@ -151,8 +162,8 @@ export function renderSection(entry: LogEntry): string {
* Formats entries for the terminal writer.
*/
export function formatForTerminal(entry: LogEntry, logger: Logger): string {
const { msg: msg, symbol, data, error } = entry
const empty = [msg, symbol, data, error].every((val) => val === undefined)
const { msg: msg, data, error } = entry
const empty = [msg, data, error].every((val) => val === undefined)

if (empty) {
return ""
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
4 changes: 2 additions & 2 deletions core/src/tasks/resolve-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,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.silly : undefined
return log.createLog({ name: providerName, fixLevel })
}

Expand Down

0 comments on commit e76a5d9

Please sign in to comment.