-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[Streams] Use TS instead of FROM for TSDB mode in Discover links #250554
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
Changes from 3 commits
9dfbb72
222b14b
2b74894
ef7ebc5
3c118eb
66bc64e
990a70d
9addbaf
c0d16b0
4dca2c9
07230a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,20 +167,26 @@ export function LifecycleBadge({ | |
| export function DiscoverBadgeButton({ | ||
| definition, | ||
| isWiredStream, | ||
| isTSDBMode: isTSDBModeProp, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot This prop is not used anywhere, please remove it |
||
| }: { | ||
| definition: Streams.ingest.all.GetResponse; | ||
| isWiredStream: boolean; | ||
| /** When provided from listing data, uses this instead of definition.index_mode */ | ||
| isTSDBMode?: boolean; | ||
| }) { | ||
| const { | ||
| dependencies: { | ||
| start: { share }, | ||
| }, | ||
| } = useKibana(); | ||
| // Use prop if provided (from listing data), otherwise use index_mode from definition (API response) | ||
| const isTSDBMode = isTSDBModeProp ?? definition.index_mode === 'time_series'; | ||
| const dataStreamExists = | ||
| Streams.WiredStream.GetResponse.is(definition) || definition.data_stream_exists; | ||
| const indexPatterns = getIndexPatternsForStream(definition.stream); | ||
| const sourceCommand = isTSDBMode ? 'TS' : 'FROM'; | ||
| const esqlQuery = indexPatterns | ||
| ? `FROM ${indexPatterns.join(', ')}${isWiredStream ? ' METADATA _source' : ''}` | ||
| ? `${sourceCommand} ${indexPatterns.join(', ')}${isWiredStream ? ' METADATA _source' : ''}` | ||
| : undefined; | ||
| const useUrl = share.url.locators.useUrl; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,40 +27,50 @@ import { | |
| isDescendantOf, | ||
| isRootStreamDefinition, | ||
| } from '@kbn/streams-schema'; | ||
| import type { estypes } from '@elastic/elasticsearch'; | ||
| import { useStreamsAppRouter } from '../../hooks/use_streams_app_router'; | ||
| import { NestedView } from '../nested_view'; | ||
| import { useKibana } from '../../hooks/use_kibana'; | ||
|
|
||
| export interface StreamListItem { | ||
| stream: Streams.all.Definition; | ||
| data_stream?: estypes.IndicesDataStream; | ||
| } | ||
|
|
||
| export interface StreamTree { | ||
| name: string; | ||
| type: 'wired' | 'root' | 'classic'; | ||
| stream: Streams.all.Definition; | ||
| data_stream?: estypes.IndicesDataStream; | ||
| children: StreamTree[]; | ||
| } | ||
|
|
||
| export function asTrees(streams: Streams.all.Definition[]) { | ||
| export function asTrees(items: StreamListItem[]) { | ||
| const trees: StreamTree[] = []; | ||
| const sortedStreams = streams | ||
| const sortedItems = items | ||
| .slice() | ||
| .sort((a, b) => getSegments(a.name).length - getSegments(b.name).length); | ||
| .sort((a, b) => getSegments(a.stream.name).length - getSegments(b.stream.name).length); | ||
|
|
||
| sortedStreams.forEach((stream) => { | ||
| sortedItems.forEach((item) => { | ||
| let currentTree = trees; | ||
| let existingNode: StreamTree | undefined; | ||
| // traverse the tree following the prefix of the current name. | ||
| // once we reach the leaf, the current name is added as child - this works because the ids are sorted by depth | ||
| while ((existingNode = currentTree.find((node) => isDescendantOf(node.name, stream.name)))) { | ||
| while ( | ||
| (existingNode = currentTree.find((node) => isDescendantOf(node.name, item.stream.name))) | ||
| ) { | ||
| currentTree = existingNode.children; | ||
| } | ||
|
|
||
| if (!existingNode) { | ||
| const newNode: StreamTree = { | ||
| name: stream.name, | ||
| name: item.stream.name, | ||
| children: [], | ||
| stream, | ||
| type: Streams.ClassicStream.Definition.is(stream) | ||
| stream: item.stream, | ||
| data_stream: item.data_stream, | ||
| type: Streams.ClassicStream.Definition.is(item.stream) | ||
| ? 'classic' | ||
| : isRootStreamDefinition(stream) | ||
| : isRootStreamDefinition(item.stream) | ||
| ? 'root' | ||
| : 'wired', | ||
| }; | ||
|
|
@@ -76,7 +86,7 @@ export function StreamsList({ | |
| query, | ||
| showControls, | ||
| }: { | ||
| streams: Streams.all.Definition[] | undefined; | ||
| streams: StreamListItem[] | undefined; | ||
| query?: string; | ||
| showControls: boolean; | ||
| }) { | ||
|
|
@@ -88,8 +98,8 @@ export function StreamsList({ | |
|
|
||
| const filteredItems = useMemo(() => { | ||
| return items | ||
| .filter((item) => showClassic || Streams.WiredStream.Definition.is(item)) | ||
| .filter((item) => !query || item.name.toLowerCase().includes(query.toLowerCase())); | ||
| .filter((item) => showClassic || Streams.WiredStream.Definition.is(item.stream)) | ||
| .filter((item) => !query || item.stream.name.toLowerCase().includes(query.toLowerCase())); | ||
| }, [query, items, showClassic]); | ||
|
|
||
| const treeView = useMemo(() => asTrees(filteredItems), [filteredItems]); | ||
|
|
@@ -113,7 +123,7 @@ export function StreamsList({ | |
| iconType="fold" | ||
| size="s" | ||
| onClick={() => | ||
| setCollapsed(Object.fromEntries(items.map((item) => [item.name, true]))) | ||
| setCollapsed(Object.fromEntries(items.map((item) => [item.stream.name, true]))) | ||
| } | ||
| > | ||
| {i18n.translate('xpack.streams.streamsTable.collapseAll', { | ||
|
|
@@ -178,19 +188,23 @@ function StreamNode({ | |
| [share.url.locators] | ||
| ); | ||
|
|
||
| // Use TSDB mode from data_stream.index_mode directly from listing data | ||
| const isTSDBMode = node.data_stream?.index_mode === 'time_series'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen this about 4 times in the PR and it will probably used more, can we expose a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, we should centralize. We should even go a bit further and centralize the whole base ESQL query generation (definition -> query) instead of having that logic in multiple places, there is no good reason for that. |
||
|
|
||
| const discoverUrl = useMemo(() => { | ||
| const indexPatterns = getIndexPatternsForStream(node.stream); | ||
|
|
||
| if (!discoverLocator || !indexPatterns) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const sourceCommand = isTSDBMode ? 'TS' : 'FROM'; | ||
| return discoverLocator.getRedirectUrl({ | ||
| query: { | ||
| esql: `FROM ${indexPatterns.join(', ')}`, | ||
| esql: `${sourceCommand} ${indexPatterns.join(', ')}`, | ||
| }, | ||
| }); | ||
| }, [discoverLocator, node]); | ||
| }, [discoverLocator, node, isTSDBMode]); | ||
|
|
||
| return ( | ||
| <EuiFlexGroup | ||
|
|
||
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.
nit: why is this type assertion necessary? It shouldn't be harmful, but it's always nice to prevent assertions as you won't be protected if index_mode changes or is removed.
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.
Good call, we should change that.