Skip to content

Commit

Permalink
web: add support for filtering logs by regular expression (#4661)
Browse files Browse the repository at this point in the history
* Add support for filtering logs by regexp with error message display, plus fix a bug with filter input not being synced with url bar
  • Loading branch information
lizzthabet committed Jun 22, 2021
1 parent 3e7bc2a commit a28ef3c
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 36 deletions.
117 changes: 94 additions & 23 deletions web/src/OverviewActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import { makeStyles } from "@material-ui/core/styles"
import ExpandMoreIcon from "@material-ui/icons/ExpandMore"
import { History } from "history"
import moment from "moment"
import React, { ChangeEvent, useRef, useState } from "react"
import React, { ChangeEvent, useEffect, useRef, useState } from "react"
import { useHistory, useLocation } from "react-router"
import styled from "styled-components"
import { Alert } from "./alerts"
import { incr } from "./analytics"
import { ReactComponent as AlertSvg } from "./assets/svg/alert.svg"
import { ReactComponent as CheckmarkSvg } from "./assets/svg/checkmark.svg"
import { ReactComponent as CloseSvg } from "./assets/svg/close.svg"
import { ReactComponent as CopySvg } from "./assets/svg/copy.svg"
Expand All @@ -24,7 +25,15 @@ import { ReactComponent as LinkSvg } from "./assets/svg/link.svg"
import { InstrumentedButton } from "./instrumentedComponents"
import { displayURL } from "./links"
import LogActions from "./LogActions"
import { EMPTY_TERM, FilterLevel, FilterSet, FilterSource } from "./logfilters"
import {
EMPTY_TERM,
FilterLevel,
FilterSet,
FilterSource,
FilterTerm,
isErrorTerm,
TermState,
} from "./logfilters"
import { useLogStore } from "./LogStore"
import OverviewActionBarKeyboardShortcuts from "./OverviewActionBarKeyboardShortcuts"
import { usePathBuilder } from "./PathBuilder"
Expand Down Expand Up @@ -253,36 +262,74 @@ export let ButtonRightPill = styled(ButtonRoot)`

const FilterTermTextField = styled(TextField)`
& .MuiOutlinedInput-root {
border-radius: ${SizeUnit(0.125)};
border: 1px solid ${Color.grayLighter};
background-color: ${Color.grayDark};
transition: border-color ${AnimDuration.default} ease;
width: ${SizeUnit(8.125)};
position: relative;
width: ${SizeUnit(9)};
&:hover {
border-color: ${Color.blue};
}
& fieldset {
border-color: 1px solid ${Color.grayLighter};
}
&:hover fieldset {
border: 1px solid ${Color.grayLighter};
border-radius: ${SizeUnit(0.125)};
transition: border-color ${AnimDuration.default} ease;
}
&:hover:not(.Mui-focused, .Mui-error) fieldset {
border: 1px solid ${Color.blue};
}
& .Mui-focused fieldset {
border: 1px solid ${Color.grayLighter};
outline: none;
}
& .Mui-error fieldset {
border: 1px solid ${Color.red};
}
& .MuiOutlinedInput-input {
padding: ${SizeUnit(0.2)};
}
}
& .MuiInputBase-input {
font-family: ${Font.monospace};
color: ${Color.gray7};
font-family: ${Font.monospace};
font-size: ${FontSize.small};
}
`

const FieldErrorTooltip = styled.span`
align-items: center;
background-color: ${Color.grayDark};
box-sizing: border-box;
display: flex;
font-family: ${Font.monospace};
font-size: ${FontSize.smallest};
left: 0;
line-height: 1.4;
margin: ${SizeUnit(0.25)} 0 0 0;
padding: ${SizeUnit(0.25)};
position: absolute;
right: 0;
top: 100%;
z-index: 1;
::before {
border-bottom: 8px solid ${Color.grayDark};
border-left: 8px solid transparent;
border-right: 8px solid transparent;
content: "";
height: 0;
left: 20px;
position: absolute;
top: -8px;
width: 0;
}
.Mui-error {
color: ${Color.red};
}
`

const AlertIcon = styled(AlertSvg)`
padding-right: ${SizeUnit(0.25)};
`

const ClearFilterTermTextButton = styled(InstrumentedButton)`
${mixinResetButtonStyle}
align-items: center;
Expand Down Expand Up @@ -421,15 +468,30 @@ export function FilterRadioButton(props: FilterRadioButtonProps) {
export const FILTER_INPUT_DEBOUNCE = 500 // in ms
export const FILTER_FIELD_ID = "FilterTermTextInput"

function FilterTermFieldError({ error }: { error: string }) {
return (
<FieldErrorTooltip>
<AlertIcon width="20" height="20" role="presentation" />
{error}
</FieldErrorTooltip>
)
}

const debounceFilterLogs = debounce((history: History, search: string) => {
history.push({ search })
}, FILTER_INPUT_DEBOUNCE)

export function FilterTermField(props: { initialTerm: string }) {
const [filterTerm, setFilterTerm] = useState(props.initialTerm ?? EMPTY_TERM)

export function FilterTermField({ termFromUrl }: { termFromUrl: FilterTerm }) {
const { input: initialTerm, state } = termFromUrl
const location = useLocation()
const history = useHistory()
const { location } = history

const [filterTerm, setFilterTerm] = useState(initialTerm ?? EMPTY_TERM)

// If the location changes, reset the value of the input field based on url
useEffect(() => {
setFilterTerm(initialTerm)
}, [location.pathname])

/**
* Note about term updates:
Expand Down Expand Up @@ -484,20 +546,26 @@ export function FilterTermField(props: { initialTerm: string }) {
inputProps.endAdornment = endAdornment
}

// TODO (LT): Add `aria-invalid` markup that will show if
// there's an error parsing an input string to regexp
return (
<>
<FilterTermTextField
error={state === TermState.Error}
id={FILTER_FIELD_ID}
helperText={
isErrorTerm(termFromUrl) ? (
<FilterTermFieldError error={termFromUrl.error} />
) : (
""
)
}
InputProps={inputProps}
onChange={onChange}
placeholder="Filter logs by text"
placeholder="Filter by text or /regexp/"
value={filterTerm}
variant="outlined"
/>
<SrOnly component="label" htmlFor={FILTER_FIELD_ID}>
Filter resource logs by text
Filter resource logs by text or /regexp/
</SrOnly>
</>
)
Expand Down Expand Up @@ -565,7 +633,10 @@ let ActionBarBottomRow = styled.div`
flex-wrap: wrap;
align-items: center;
border-bottom: 1px solid ${Color.grayLighter};
padding: ${SizeUnit(0.25)} ${SizeUnit(0.5)};
padding-left: ${SizeUnit(0.5)};
padding-right: ${SizeUnit(0.5)};
padding-top: ${SizeUnit(0.35)};
padding-bottom: ${SizeUnit(0.35)};
`

type ActionBarProps = {
Expand Down Expand Up @@ -739,7 +810,7 @@ export default function OverviewActionBar(props: OverviewActionBarProps) {
filterSet={filterSet}
alerts={alerts}
/>
<FilterTermField initialTerm={filterSet.term.input} />
<FilterTermField termFromUrl={filterSet.term} />
<LogActions resourceName={resourceName} isSnapshot={isSnapshot} />
</ActionBarBottomRow>
</ActionBarRoot>
Expand Down
5 changes: 5 additions & 0 deletions web/src/assets/svg/alert.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
60 changes: 54 additions & 6 deletions web/src/logfilters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,29 @@ describe("Log filters", () => {
})

it("gets set with a parsed state if a valid term is present", () => {
const location = { search: "term=docker+build" } as Location
const parsedTerm = filterSetFromLocation(location).term
expect(parsedTerm.state).toEqual(TermState.Parsed)
expect(parsedTerm.input).toEqual("docker build")
expect(parsedTerm.hasOwnProperty("regexp")).toBe(true)
const textLocation = { search: "term=docker+build" } as Location
const textParsedTerm = filterSetFromLocation(textLocation).term
expect(textParsedTerm.state).toEqual(TermState.Parsed)
expect(textParsedTerm.input).toEqual("docker build")
expect(textParsedTerm.hasOwnProperty("regexp")).toBe(true)
expect(textParsedTerm.hasOwnProperty("error")).toBe(false)

const regexpLocation = { search: "term=%2Fdocker%2F" } as Location
const regexpParsedTerm = filterSetFromLocation(regexpLocation).term
expect(regexpParsedTerm.state).toEqual(TermState.Parsed)
expect(regexpParsedTerm.input).toEqual("/docker/")
expect(regexpParsedTerm.hasOwnProperty("regexp")).toBe(true)
expect(regexpParsedTerm.hasOwnProperty("error")).toBe(false)
})

// TODO(LT): when regex input is supported in term filter field, add a test for error case
it("gets set with an error state if an invalid input is present", () => {
const location = { search: "term=%2Fdock(er%3F%2F" } as Location
const parsedTerm = filterSetFromLocation(location).term
expect(parsedTerm.state).toEqual(TermState.Error)
expect(parsedTerm.input).toEqual("/dock(er?/")
expect(parsedTerm.hasOwnProperty("regexp")).toBe(false)
expect(parsedTerm.hasOwnProperty("error")).toBe(true)
})
})

describe("term parsing", () => {
Expand Down Expand Up @@ -87,6 +102,39 @@ describe("Log filters", () => {
)
})
})

describe("for regular expressions", () => {
it("only parses strings surrounded by forward slashes as regexp", () => {
expect(
parseTermInput("/docker/").test(TestStrings.BuildInfoLine)
).toBe(true)
expect(
parseTermInput("/docker").test(TestStrings.BuildInfoLine)
).toBe(false)
})

it("matches on the expected text", () => {
expect(parseTermInput("/ab?c/").test(TestStrings.Basic)).toBe(true)
expect(
parseTermInput("/error.+main/").test(TestStrings.BuildErrorInFile)
).toBe(true)
expect(parseTermInput("/d+/").test(TestStrings.Basic)).toBe(false)
expect(parseTermInput("/d+/").test(TestStrings.BuildInfoLine)).toBe(
true
)
expect(
parseTermInput("/failed:/").test(TestStrings.BuildError404)
).toBe(true)
})

it("throws an error when input text is invalid regex", () => {
expect(() =>
parseTermInput("/(missing)? parenthesis)/")
).toThrowError(
"Invalid regular expression: /(missing)? parenthesis)/: Unmatched ')'"
)
})
})
})
})
})
32 changes: 25 additions & 7 deletions web/src/logfilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ type ParsedTerm = { state: TermState.Parsed; regexp: RegExp }

type ErrorTerm = { state: TermState.Error; error: string }

type FilterTerm = {
export function isErrorTerm(
term: FilterTerm
): term is { input: string } & ErrorTerm {
return term.state === TermState.Error
}

export type FilterTerm = {
input: string // Unmodified string input
} & (EmptyTerm | ParsedTerm | ErrorTerm)

Expand All @@ -51,14 +57,21 @@ export const EMPTY_FILTER_TERM: FilterTerm = {
input: EMPTY_TERM,
state: TermState.Empty,
}
const TERM_REGEXP_FLAGS = "gi" // Terms are case-insensitive and can match multiple instances

export function parseTermInput(input: string): RegExp {
// Escape all characters that have special meaning in RegExp,
// so term can be treated as a string literal
const escapedInput = RegexEscape(input)
// Input strings that are surrounded by `/` can be parsed as regular expressions
if (input.length > 2 && input[0] === "/" && input[input.length - 1] === "/") {
const regexpInput = input.slice(1, input.length - 1)

return new RegExp(regexpInput, TERM_REGEXP_FLAGS)
} else {
// Input strings that aren't regular expressions should have all
// special characters escaped so they can be treated literally
const escapedInput = RegexEscape(input)

// Filter terms are case-insensitive and can match multiple instances
return new RegExp(escapedInput, "gi")
return new RegExp(escapedInput, TERM_REGEXP_FLAGS)
}
}

export function createFilterTermState(input: string): FilterTerm {
Expand All @@ -73,10 +86,15 @@ export function createFilterTermState(input: string): FilterTerm {
state: TermState.Parsed,
}
} catch (error) {
let message = "Invalid regexp"
if (error.message) {
message += `: ${error.message}`
}

return {
input,
state: TermState.Error,
error: error?.message,
error: message,
}
}
}
Expand Down

0 comments on commit a28ef3c

Please sign in to comment.