-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Logs Explorer] Implement Flyout content header #169832
Merged
Merged
Changes from 23 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
9b8104d
feat(discover): enhance flyout customization to update content
7977fa0
Merge branch 'main' into 169394-enhance-flyout-extension-point
tonyghiani 0b8514a
Merge branch 'main' into 169394-enhance-flyout-extension-point
tonyghiani e48d26a
refactor(discover): add request changes
faa0c3e
refactor(discover): update test
ca1accf
feat(discover): begin flyout header implementation
1e0f740
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine fdd9edf
feat(discover): remove trailing comma
98b7b8b
feat(discover): fix conditional rendering
4c481f8
Merge branch 'main' into 169501-log-flyout-header
achyutjhunjhunwala 2c9a014
feat(log-explorer): minor changes
3b8b680
Merge branch '169501-log-flyout-header' of github.com:tonyghiani/kiba…
5f8e53a
Merge branch 'main' into 169501-log-flyout-header
tonyghiani e72c32c
test(log-explorer): wip test flyout
8919efb
Merge branch '169501-log-flyout-header' of github.com:tonyghiani/kiba…
dca7add
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine 4a888e6
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine e84a21c
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine f180439
test(log-explorer): finish stateful tests
cdfc9b9
Merge branch '169501-log-flyout-header' of github.com:tonyghiani/kiba…
fc25832
Merge branch 'main' into 169501-log-flyout-header
tonyghiani 9e31b51
test(log-explorer): copy serverless test suite
9761534
test(log-explorer): small page object refactor
c0cf9dc
refactor(log-explorer): request changes
31b9f81
Merge branch 'main' into 169501-log-flyout-header
tonyghiani c1f97d6
Merge branch 'main' into 169501-log-flyout-header
tonyghiani cabb230
Merge branch 'main' into 169501-log-flyout-header
tonyghiani 09e3cf6
Merge branch 'main' into 169501-log-flyout-header
tonyghiani bb11af7
Merge branch 'main' into 169501-log-flyout-header
tonyghiani 223a490
feat(log-explorer): comment content customization application
9b980d8
tests(log-explorer): skip tests
aa8bf51
Merge branch 'main' into 169501-log-flyout-header
tonyghiani c0be1bb
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine 46507aa
feat(log-explorer): re-enable flyout customization
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
"data", | ||
"dataViews", | ||
"discover", | ||
"fieldFormats", | ||
"fleet", | ||
"kibanaReact", | ||
"kibanaUtils", | ||
|
51 changes: 51 additions & 0 deletions
51
x-pack/plugins/log_explorer/public/components/flyout_detail/flyout_detail.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; | ||
import { FlyoutContentProps } from '@kbn/discover-plugin/public'; | ||
import { LogLevel } from './sub_components/log_level'; | ||
import { Timestamp } from './sub_components/timestamp'; | ||
import { LogDocument } from './types'; | ||
import { getDocDetailRenderFlags, useDocDetail } from './use_doc_detail'; | ||
import { Message } from './sub_components/message'; | ||
|
||
export function FlyoutDetail({ | ||
dataView, | ||
doc, | ||
}: Pick<FlyoutContentProps, 'dataView' | 'doc' | 'actions'>) { | ||
const parsedDoc = useDocDetail(doc as LogDocument, { dataView }); | ||
|
||
const { hasTimestamp, hasLogLevel, hasMessage, hasBadges, hasFlyoutHeader } = | ||
getDocDetailRenderFlags(parsedDoc); | ||
|
||
return hasFlyoutHeader ? ( | ||
<EuiFlexGroup direction="column" gutterSize="m" data-test-subj="logExplorerFlyoutDetail"> | ||
<EuiFlexItem grow={false}> | ||
{hasBadges && ( | ||
<EuiFlexGroup responsive={false} gutterSize="m"> | ||
{hasLogLevel && ( | ||
<EuiFlexItem grow={false}> | ||
<LogLevel level={parsedDoc['log.level']} /> | ||
</EuiFlexItem> | ||
)} | ||
{hasTimestamp && ( | ||
<EuiFlexItem grow={false}> | ||
<Timestamp timestamp={parsedDoc['@timestamp']} /> | ||
</EuiFlexItem> | ||
)} | ||
</EuiFlexGroup> | ||
)} | ||
</EuiFlexItem> | ||
{hasMessage && ( | ||
<EuiFlexItem grow={false}> | ||
<Message message={parsedDoc.message} /> | ||
</EuiFlexItem> | ||
)} | ||
</EuiFlexGroup> | ||
) : null; | ||
} |
9 changes: 9 additions & 0 deletions
9
x-pack/plugins/log_explorer/public/components/flyout_detail/index.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
export * from './flyout_detail'; | ||
export * from './types'; |
33 changes: 33 additions & 0 deletions
33
x-pack/plugins/log_explorer/public/components/flyout_detail/sub_components/log_level.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { EuiBadge, type EuiBadgeProps } from '@elastic/eui'; | ||
import { FlyoutDoc } from '../types'; | ||
|
||
const LEVEL_DICT: Record<string, EuiBadgeProps['color']> = { | ||
error: 'danger', | ||
warn: 'warning', | ||
info: 'primary', | ||
default: 'default', | ||
}; | ||
|
||
interface LogLevelProps { | ||
level: FlyoutDoc['log.level']; | ||
} | ||
|
||
export function LogLevel({ level }: LogLevelProps) { | ||
if (!level) return null; | ||
|
||
const levelColor = LEVEL_DICT[level] ?? LEVEL_DICT.default; | ||
|
||
return ( | ||
<EuiBadge color={levelColor} data-test-subj="logExplorerFlyoutLogLevel"> | ||
{level} | ||
</EuiBadge> | ||
); | ||
} |
34 changes: 34 additions & 0 deletions
34
x-pack/plugins/log_explorer/public/components/flyout_detail/sub_components/message.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { EuiCodeBlock, EuiFlexGroup, EuiFlexItem, EuiText } from '@elastic/eui'; | ||
import { FlyoutDoc } from '../types'; | ||
import { flyoutMessageLabel } from '../translations'; | ||
|
||
interface MessageProps { | ||
message: FlyoutDoc['message']; | ||
} | ||
|
||
export function Message({ message }: MessageProps) { | ||
if (!message) return null; | ||
|
||
return ( | ||
<EuiFlexGroup direction="column" gutterSize="xs" data-test-subj="logExplorerFlyoutLogMessage"> | ||
<EuiFlexItem> | ||
<EuiText color="subdued" size="xs"> | ||
{flyoutMessageLabel} | ||
</EuiText> | ||
</EuiFlexItem> | ||
<EuiFlexItem> | ||
<EuiCodeBlock overflowHeight={100} paddingSize="m" isCopyable language="txt" fontSize="m"> | ||
{message} | ||
</EuiCodeBlock> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
); | ||
} |
24 changes: 24 additions & 0 deletions
24
x-pack/plugins/log_explorer/public/components/flyout_detail/sub_components/timestamp.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { EuiBadge } from '@elastic/eui'; | ||
import { FlyoutDoc } from '../types'; | ||
|
||
interface TimestampProps { | ||
timestamp: FlyoutDoc['@timestamp']; | ||
} | ||
|
||
export function Timestamp({ timestamp }: TimestampProps) { | ||
if (!timestamp) return null; | ||
|
||
return ( | ||
<EuiBadge color="hollow" data-test-subj="logExplorerFlyoutLogTimestamp"> | ||
{timestamp} | ||
</EuiBadge> | ||
); | ||
} |
12 changes: 12 additions & 0 deletions
12
x-pack/plugins/log_explorer/public/components/flyout_detail/translations.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { i18n } from '@kbn/i18n'; | ||
|
||
export const flyoutMessageLabel = i18n.translate('xpack.logExplorer.flyoutDetail.label.message', { | ||
defaultMessage: 'Message', | ||
}); |
29 changes: 29 additions & 0 deletions
29
x-pack/plugins/log_explorer/public/components/flyout_detail/types.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import type { EuiIconType } from '@elastic/eui/src/components/icon/icon'; | ||
import { DataTableRecord } from '@kbn/discover-utils/types'; | ||
|
||
export interface LogDocument extends DataTableRecord { | ||
flattened: { | ||
'@timestamp': string; | ||
'log.level'?: string; | ||
message?: string; | ||
}; | ||
} | ||
|
||
export interface FlyoutDoc { | ||
'@timestamp': string; | ||
'log.level'?: string; | ||
message?: string; | ||
} | ||
|
||
export interface FlyoutHighlightField { | ||
label: string; | ||
value: string; | ||
iconType?: EuiIconType; | ||
} |
62 changes: 62 additions & 0 deletions
62
x-pack/plugins/log_explorer/public/components/flyout_detail/use_doc_detail.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { FlyoutContentProps } from '@kbn/discover-plugin/public'; | ||
import { formatFieldValue } from '@kbn/discover-utils'; | ||
import { LOG_LEVEL_FIELD, MESSAGE_FIELD, TIMESTAMP_FIELD } from '../../../common/constants'; | ||
import { useKibanaContextForPlugin } from '../../utils/use_kibana'; | ||
import { FlyoutDoc, LogDocument } from './types'; | ||
|
||
export function useDocDetail( | ||
doc: LogDocument, | ||
{ dataView }: Pick<FlyoutContentProps, 'dataView'> | ||
): FlyoutDoc { | ||
const { services } = useKibanaContextForPlugin(); | ||
|
||
const formatField = <F extends keyof LogDocument['flattened']>( | ||
field: F | ||
): LogDocument['flattened'][F] => { | ||
return ( | ||
doc.flattened[field] && | ||
formatFieldValue( | ||
doc.flattened[field], | ||
doc.raw, | ||
services.fieldFormats, | ||
dataView, | ||
dataView.fields.getByName(field) | ||
) | ||
); | ||
}; | ||
|
||
const level = formatField(LOG_LEVEL_FIELD)?.toLowerCase(); | ||
const timestamp = formatField(TIMESTAMP_FIELD); | ||
const message = formatField(MESSAGE_FIELD); | ||
|
||
return { | ||
[LOG_LEVEL_FIELD]: level, | ||
[TIMESTAMP_FIELD]: timestamp, | ||
[MESSAGE_FIELD]: message, | ||
}; | ||
} | ||
|
||
export const getDocDetailRenderFlags = (doc: FlyoutDoc) => { | ||
const hasTimestamp = Boolean(doc['@timestamp']); | ||
const hasLogLevel = Boolean(doc['log.level']); | ||
const hasMessage = Boolean(doc.message); | ||
|
||
const hasBadges = hasTimestamp || hasLogLevel; | ||
|
||
const hasFlyoutHeader = hasBadges || hasMessage; | ||
|
||
return { | ||
hasTimestamp, | ||
hasLogLevel, | ||
hasMessage, | ||
hasBadges, | ||
hasFlyoutHeader, | ||
}; | ||
}; |
32 changes: 32 additions & 0 deletions
32
x-pack/plugins/log_explorer/public/customizations/custom_flyout_content.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { FlyoutContentProps } from '@kbn/discover-plugin/public'; | ||
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; | ||
import { FlyoutDetail } from '../components/flyout_detail/flyout_detail'; | ||
|
||
export const CustomFlyoutContent = ({ | ||
actions, | ||
dataView, | ||
doc, | ||
renderDefaultContent, | ||
}: FlyoutContentProps) => { | ||
return ( | ||
<EuiFlexGroup direction="column"> | ||
{/* Apply custom Log Explorer detail */} | ||
<EuiFlexItem> | ||
<FlyoutDetail actions={actions} dataView={dataView} doc={doc} /> | ||
</EuiFlexItem> | ||
{/* Restore default content */} | ||
<EuiFlexItem>{renderDefaultContent()}</EuiFlexItem> | ||
</EuiFlexGroup> | ||
); | ||
}; | ||
|
||
// eslint-disable-next-line import/no-default-export | ||
export default CustomFlyoutContent; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding
dataView
toFlyoutContentProps
, could we instead retrieve it from the state container directly and pass it to your custom component? I'd prefer to keep a single source of truth for customizations to access the shared state if possible.On a side note, requiring
useObservable
to access our state in customization components feels a bit inconvenient IMO. Internally we use state selector functions instead, and we could likely give customizations access to the same selectors by passing them to theCustomizationCallback
. Just curious if it would be useful for your team to have access to these selectors too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Davis, our team is working on extracting the persistent system on top of the state container, so that each consumer of the log explorer won't depend on the URL sync. I think we'll consume the internal state reacting to the observable changes anyway, I can't say if we'll need those selectors.
Regarding your suggestion I see your point, I'll apply your suggestion and revert the changes on the customization contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes applied in c0cf9dc