From 3c214cefad5abef567f3216b08ea230025ec71f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Fri, 25 Jan 2019 16:01:29 +0100 Subject: [PATCH] fix(dashboard): limit number of log lines that are fetched (#461) --- dashboard/src/api/index.ts | 12 +++-- dashboard/src/api/types.ts | 2 +- dashboard/src/components/card.tsx | 4 +- dashboard/src/components/graph/index.tsx | 50 +++++++++--------- dashboard/src/components/logs.tsx | 49 +++++++++++++++--- dashboard/src/components/terminal.tsx | 39 +++++++------- dashboard/src/containers/logs.tsx | 34 ++++++++++--- dashboard/src/context/data.tsx | 51 +++++++++++-------- dashboard/src/styles/variables.ts | 5 ++ docs/reference/commands.md | 3 +- garden-service/src/commands/logs.ts | 18 ++++--- garden-service/src/plugins/kubernetes/logs.ts | 10 ++-- garden-service/src/types/plugin/params.ts | 8 ++- garden-service/test/src/actions.ts | 2 +- 14 files changed, 185 insertions(+), 102 deletions(-) diff --git a/dashboard/src/api/index.ts b/dashboard/src/api/index.ts index 03cb30b92d..5cb4f3feae 100644 --- a/dashboard/src/api/index.ts +++ b/dashboard/src/api/index.ts @@ -11,11 +11,15 @@ import axios from "axios" import { FetchConfigResponse, FetchStatusResponse, - FetchLogResponse, + FetchLogsResponse, ApiRequest, FetchGraphResponse, } from "./types" +export type FetchLogsParam = string[] + +const MAX_LOG_LINES = 5000 + export async function fetchConfig(): Promise { return apiPost("get.config") } @@ -28,9 +32,9 @@ export async function fetchStatus(): Promise { return apiPost("get.status") } -export async function fetchLogs(services?: string[]): Promise { - const params = services ? { service: services } : {} - return apiPost("logs", params) +export async function fetchLogs(services: FetchLogsParam): Promise { + const tail = Math.floor(MAX_LOG_LINES / services.length) + return apiPost("logs", { services, tail }) } async function apiPost(command: string, parameters: {} = {}): Promise { diff --git a/dashboard/src/api/types.ts b/dashboard/src/api/types.ts index bf8569e0eb..8db9a62418 100644 --- a/dashboard/src/api/types.ts +++ b/dashboard/src/api/types.ts @@ -101,7 +101,7 @@ export interface ServiceLogEntry { msg: string } -export type FetchLogResponse = ServiceLogEntry[] +export type FetchLogsResponse = ServiceLogEntry[] export interface ApiRequest { command: string diff --git a/dashboard/src/components/card.tsx b/dashboard/src/components/card.tsx index d8102f9fac..185f4b2ee1 100644 --- a/dashboard/src/components/card.tsx +++ b/dashboard/src/components/card.tsx @@ -22,7 +22,7 @@ const Wrapper = styled.div` box-shadow: 0 1px 3px rgba(0,0,0,0.12), 0 1px 2px rgba(0,0,0,0.24); ` -const Title = styled.h3` +export const CardTitle = styled.h3` ${fontMedium}; font-size: 1.3rem; margin: 0; @@ -32,7 +32,7 @@ const Card: React.SFC = ({ children, title }) => { const titleEl = title ? (
- {title} + {title}
) : null diff --git a/dashboard/src/components/graph/index.tsx b/dashboard/src/components/graph/index.tsx index b1391cce0b..691b1ef6e5 100644 --- a/dashboard/src/components/graph/index.tsx +++ b/dashboard/src/components/graph/index.tsx @@ -170,8 +170,7 @@ const Status = styled.p` ` const ProcessSpinner = styled(Spinner)` - margin: 20px 0 0 20px; - font-size: 12px; + margin: 16px 0 0 20px; ` class Chart extends Component { @@ -296,34 +295,27 @@ class Chart extends Component { const chartHeightEstimate = `100vh - 15rem` let spinner = null - let status = "Ready" + let status = "" if (message && message.name !== "taskGraphComplete") { status = "Processing..." - spinner = + spinner = } return (
-

- — Ready - -- Pending - — Error -

-
- {taskTypes.map(type => ( - - ))} -
+ {taskTypes.map(type => ( + + ))}
{
- {status} - {spinner} + justify-content: space-between; + `, "ml-1 mr-1 pb-1")}> +
+ {status} + {spinner} +
+

+ — Ready + -- Pending + — Error +

diff --git a/dashboard/src/components/logs.tsx b/dashboard/src/components/logs.tsx index 5bee75874b..d8e08305a2 100644 --- a/dashboard/src/components/logs.tsx +++ b/dashboard/src/components/logs.tsx @@ -6,21 +6,40 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import styled from "@emotion/styled/macro" import { flatten, max } from "lodash" import React, { Component } from "react" import Terminal from "./terminal" -import { FetchConfigResponse, FetchLogResponse } from "../api/types" +import { FetchConfigResponse, FetchLogsResponse } from "../api/types" +import Card, { CardTitle } from "./card" +import { colors } from "../styles/variables" +import { LoadLogs } from "../context/data" interface Props { config: FetchConfigResponse - logs: FetchLogResponse + logs: FetchLogsResponse + loadLogs: LoadLogs } interface State { selectedService: string } +const Header = styled.div` + display: flex; + justify-content: space-between; +` + +const Icon = styled.i` + color: ${colors.gardenPink}; + font-size: 1.5rem; + cursor: pointer; + :active { + color: ${colors.gardenPinkLighten(0.7)} + } +` + class Logs extends Component { constructor(props) { @@ -31,12 +50,18 @@ class Logs extends Component { selectedService: "all", } this.handleChange = this.handleChange.bind(this) + this.refresh = this.refresh.bind(this) } handleChange(event) { this.setState({ selectedService: event.target.value }) } + refresh() { + const serviceNames = flatten(this.props.config.modules.map(m => m.serviceNames)) + this.props.loadLogs(serviceNames, true) + } + render() { const { config, logs } = this.props const { selectedService } = this.state @@ -58,12 +83,20 @@ class Logs extends Component { ))} - + +
+
+ {title} + +
+ +
+ ) } diff --git a/dashboard/src/components/terminal.tsx b/dashboard/src/components/terminal.tsx index 440a3e2f1f..7ce4a6ddbf 100644 --- a/dashboard/src/components/terminal.tsx +++ b/dashboard/src/components/terminal.tsx @@ -10,7 +10,6 @@ import styled from "@emotion/styled/macro" import { padEnd } from "lodash" import React from "react" -import Card from "./card" import { colors } from "../styles/variables" import { ServiceLogEntry } from "../api/types" @@ -24,7 +23,7 @@ interface Props { const Term = styled.div` background-color: ${colors.lightBlack}; border-radius: 2px; - max-height: 25rem; + max-height: 45rem; overflow-y: auto; ` @@ -44,26 +43,24 @@ const Timestamp = styled.span` // FIXME Use whitespace instead of dots for the sectinon padding. // For some reason whitespace is not rendered inside spans. -const Terminal: React.SFC = ({ entries, sectionPad, showServiceName, title }) => { +const Terminal: React.SFC = ({ entries, sectionPad, showServiceName }) => { return ( - - - - {entries.map((e, idx) => { - const service = showServiceName - ? {padEnd(e.serviceName, sectionPad + 3, ".")} - : "" - return ( -

- {service} - [{e.timestamp}] - {e.msg} -

- ) - })} - - - + + + {entries.map((e, idx) => { + const service = showServiceName + ? {padEnd(e.serviceName, sectionPad + 3, ".")} + : "" + return ( +

+ {service} + [{e.timestamp}] + {e.msg} +

+ ) + })} + + ) } diff --git a/dashboard/src/containers/logs.tsx b/dashboard/src/containers/logs.tsx index ab438f32bc..82a2765cf2 100644 --- a/dashboard/src/containers/logs.tsx +++ b/dashboard/src/containers/logs.tsx @@ -6,6 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import { flatten } from "lodash" import React, { useContext, useEffect } from "react" import PageError from "../components/page-error" @@ -15,19 +16,38 @@ import { DataContext } from "../context/data" export default () => { const { - actions: { loadLogs, loadConfig }, - store: { config, logs }, + actions: { loadConfig }, + store: { config }, } = useContext(DataContext) useEffect(loadConfig, []) - useEffect(loadLogs, []) - const isLoading = !config.data || !logs.data || config.loading || logs.loading - const error = config.error || logs.error + const isLoading = !config.data || config.loading return ( - - + + ) } + +const LogsContainer = () => { + const { + actions: { loadLogs }, + store: { config, logs }, + } = useContext(DataContext) + + const serviceNames = flatten(config.data.modules.map(m => m.serviceNames)) + useEffect(() => { + loadLogs(serviceNames) + }, []) + + const isLoading = !logs.data + + return ( + + + + ) + +} diff --git a/dashboard/src/context/data.tsx b/dashboard/src/context/data.tsx index 31d024ce73..8170533081 100644 --- a/dashboard/src/context/data.tsx +++ b/dashboard/src/context/data.tsx @@ -6,15 +6,15 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { useState, EffectCallback } from "react" +import { useState } from "react" import React from "react" -import { fetchConfig, fetchLogs, fetchStatus, fetchGraph } from "../api" +import { fetchConfig, fetchLogs, fetchStatus, fetchGraph, FetchLogsParam } from "../api" import { FetchConfigResponse, FetchStatusResponse, FetchGraphResponse, - FetchLogResponse, + FetchLogsResponse, } from "../api/types" interface StoreSlice { @@ -25,7 +25,7 @@ interface StoreSlice { interface Config extends StoreSlice { data: FetchConfigResponse | null } interface Status extends StoreSlice { data: FetchStatusResponse | null } interface Graph extends StoreSlice { data: FetchGraphResponse | null } -interface Logs extends StoreSlice { data: FetchLogResponse | null } +interface Logs extends StoreSlice { data: FetchLogsResponse | null } // This is the global data store interface Store { @@ -35,18 +35,21 @@ interface Store { logs: Logs, } +export type LoadLogs = (param: FetchLogsParam, force?: boolean) => void +type Loader = (force?: boolean) => void + interface Actions { - loadLogs: EffectCallback - loadConfig: EffectCallback - loadStatus: EffectCallback - loadGraph: EffectCallback + loadLogs: LoadLogs + loadConfig: Loader + loadStatus: Loader + loadGraph: Loader } type KeyActionPair = - ["config", () => Promise] | - ["logs", () => Promise] | - ["status", () => Promise] | - ["graph", () => Promise] + ["config", (arg0?: any) => Promise] | + ["logs", (arg0?: any) => Promise] | + ["status", (arg0?: any) => Promise] | + ["graph", (arg0?: any) => Promise] type Context = { store: Store, @@ -81,11 +84,11 @@ function updateSlice(prevState: Store, key: SliceName, sliceState: Object): Stor function useApi() { const [store, setData] = useState(initialState) - const fetch = async ([key, fetchFn]: KeyActionPair) => { + const fetch = async ([key, fetchFn]: KeyActionPair, args?: any[]) => { setData(prevState => updateSlice(prevState, key, { loading: true })) try { - const res = await fetchFn() + const res = args ? await fetchFn(...args) : await fetchFn() setData(prevState => updateSlice(prevState, key, { data: res, error: null })) } catch (error) { setData(prevState => updateSlice(prevState, key, { error })) @@ -94,19 +97,27 @@ function useApi() { setData(prevState => updateSlice(prevState, key, { loading: false })) } - const fetchOrReadFromStore = (keyActionPair: KeyActionPair, force: boolean) => { + const fetchOrReadFromStore = (keyActionPair: KeyActionPair, force: boolean, args = []) => { const key = keyActionPair[0] const { data, loading } = store[key] if (!force && (data || loading)) { return } - fetch(keyActionPair).catch(error => setData(prevState => updateSlice(prevState, key, { error }))) + fetch(keyActionPair, args).catch(error => setData(prevState => updateSlice(prevState, key, { error }))) } - const loadLogs = (force: boolean = false) => fetchOrReadFromStore(["logs", fetchLogs], force) - const loadConfig = (force: boolean = false) => fetchOrReadFromStore(["config", fetchConfig], force) - const loadGraph = (force: boolean = false) => fetchOrReadFromStore(["graph", fetchGraph], force) - const loadStatus = (force: boolean = false) => fetchOrReadFromStore(["status", fetchStatus], force) + const loadLogs: LoadLogs = (args: FetchLogsParam, force: boolean = false) => ( + fetchOrReadFromStore(["logs", fetchLogs], force, [args]) + ) + const loadConfig: Loader = (force: boolean = false) => ( + fetchOrReadFromStore(["config", fetchConfig], force) + ) + const loadGraph: Loader = (force: boolean = false) => ( + fetchOrReadFromStore(["graph", fetchGraph], force) + ) + const loadStatus: Loader = (force: boolean = false) => ( + fetchOrReadFromStore(["status", fetchStatus], force) + ) return { store, diff --git a/dashboard/src/styles/variables.ts b/dashboard/src/styles/variables.ts index 90a4ec7389..8e4207b106 100644 --- a/dashboard/src/styles/variables.ts +++ b/dashboard/src/styles/variables.ts @@ -19,6 +19,10 @@ export const fontRegular = `${fontFamily};font-weight: 400;` export const fontMedium = `${fontFamily};font-weight: 500;` export const fontItalic = `${fontFamily};font-style: italic;` +function gardenPinkLighten(pct: number) { + return `rgba(218, 69, 162, ${pct})` +} + export const colors = { azure: "#00aeef", lightWhite: "rgba(255, 255, 255, 0.15)", @@ -31,6 +35,7 @@ export const colors = { gardenGrey: "#f8faff", gardenPink: "#da45a2", gardenPinkRgba: "rgba(218, 69, 162, 0)", + gardenPinkLighten, blueyGrey: "#98a6b6", iceCold: "#a5f1e6", darkGreen: "#277379", diff --git a/docs/reference/commands.md b/docs/reference/commands.md index e0fc26b344..305de69278 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -445,7 +445,8 @@ Examples: | Argument | Alias | Type | Description | | -------- | ----- | ---- | ----------- | - | `--tail` | `-t` | boolean | Continuously stream new logs from the service(s). + | `--follow` | `-f` | boolean | Continuously stream new logs from the service(s). + | `--tail` | `-t` | number | Number of lines to show for each service. Defaults to -1, showing all log lines. ### garden publish diff --git a/garden-service/src/commands/logs.ts b/garden-service/src/commands/logs.ts index 3baa7f4000..bcef3a763c 100644 --- a/garden-service/src/commands/logs.ts +++ b/garden-service/src/commands/logs.ts @@ -7,11 +7,12 @@ */ import { - BooleanParameter, Command, CommandResult, CommandParams, StringsParameter, + IntegerParameter, + BooleanParameter, } from "./base" import chalk from "chalk" import { ServiceLogEntry } from "../types/plugin/outputs" @@ -30,11 +31,16 @@ const logsArgs = { } const logsOpts = { - tail: new BooleanParameter({ + follow: new BooleanParameter({ help: "Continuously stream new logs from the service(s).", - alias: "t", + alias: "f", cliOnly: true, }), + tail: new IntegerParameter({ + help: "Number of lines to show for each service. Defaults to -1, showing all log lines.", + alias: "t", + defaultValue: -1, + }), // TODO // since: new MomentParameter({ help: "Retrieve logs from the specified point onwards" }), } @@ -61,7 +67,7 @@ export class LogsCommand extends Command { loggerType = LoggerType.basic async action({ garden, log, args, opts }: CommandParams): Promise> { - const tail = opts.tail + const { follow, tail } = opts const services = await garden.getServices(args.services) const result: ServiceLogEntry[] = [] @@ -83,7 +89,7 @@ export class LogsCommand extends Command { msg: [timestamp, chalk.white(entry.msg)], }) - if (!tail) { + if (!follow) { result.push(entry) } }) @@ -93,7 +99,7 @@ export class LogsCommand extends Command { const status = await garden.actions.getServiceStatus({ log: voidLog, service, hotReload: false }) if (status.state === "ready" || status.state === "outdated") { - await garden.actions.getServiceLogs({ log, service, stream, tail }) + await garden.actions.getServiceLogs({ log, service, stream, follow, tail }) } else { await stream.write({ serviceName: service.name, diff --git a/garden-service/src/plugins/kubernetes/logs.ts b/garden-service/src/plugins/kubernetes/logs.ts index bcc79d9f58..00bb6acf9d 100644 --- a/garden-service/src/plugins/kubernetes/logs.ts +++ b/garden-service/src/plugins/kubernetes/logs.ts @@ -23,8 +23,8 @@ interface GetKubernetesLogsParams extends GetServiceLogsParams { export async function getKubernetesLogs(params: GetKubernetesLogsParams) { // Currently Stern doesn't support just returning the logs and exiting, it can only follow - const proc = params.tail - ? await tailLogs(params) + const proc = params.follow + ? await followLogs(params) : await getLogs(params) return new Promise((resolve, reject) => { @@ -36,13 +36,14 @@ export async function getKubernetesLogs(params: GetKubernetesLogsParams) { }) } -async function tailLogs({ context, namespace, service, selector, stream, log }: GetKubernetesLogsParams) { +async function followLogs({ context, namespace, service, selector, stream, log, tail }: GetKubernetesLogsParams) { const args = [ "--color", "never", "--context", context, "--namespace", namespace, "--output", "json", "--selector", selector, + "--tail", String(tail), "--timestamps", ] @@ -66,11 +67,12 @@ async function tailLogs({ context, namespace, service, selector, stream, log }: return proc } -async function getLogs({ context, namespace, service, selector, stream }: GetKubernetesLogsParams) { +async function getLogs({ context, namespace, service, selector, stream, tail }: GetKubernetesLogsParams) { // TODO: do this via API instead of kubectl const kubectlArgs = [ "logs", "--selector", selector, + "--tail", String(tail), "--timestamps=true", ] diff --git a/garden-service/src/types/plugin/params.ts b/garden-service/src/types/plugin/params.ts index 87dd45a418..f3473adf05 100644 --- a/garden-service/src/types/plugin/params.ts +++ b/garden-service/src/types/plugin/params.ts @@ -276,7 +276,8 @@ export interface GetServiceLogsParams { runtimeContext: RuntimeContext stream: Stream - tail: boolean + follow: boolean + tail: number startTime?: Date } export const getServiceLogsParamsSchema = serviceActionParamsSchema @@ -284,8 +285,11 @@ export const getServiceLogsParamsSchema = serviceActionParamsSchema runtimeContext: runtimeContextSchema, stream: Joi.object() .description("A Stream object, to write the logs to."), - tail: Joi.boolean() + follow: Joi.boolean() .description("Whether to keep listening for logs until aborted."), + tail: Joi.number() + .description("Number of lines to get from end of log. Defaults to -1, showing all log lines.") + .default(-1), startTime: Joi.date() .optional() .description("If set, only return logs that are as new or newer than this date."), diff --git a/garden-service/test/src/actions.ts b/garden-service/test/src/actions.ts index a093057d99..bcbab6a9cd 100644 --- a/garden-service/test/src/actions.ts +++ b/garden-service/test/src/actions.ts @@ -291,7 +291,7 @@ describe("ActionHelper", () => { describe("getServiceLogs", () => { it("should correctly call the corresponding plugin handler", async () => { const stream = new Stream() - const result = await actions.getServiceLogs({ log, service, stream, tail: false }) + const result = await actions.getServiceLogs({ log, service, stream, follow: false, tail: -1 }) expect(result).to.eql({}) }) })