Skip to content
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

feat: duration & endtime in queries history #1169

Merged
merged 6 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/containers/Tenant/Query/QueriesHistory/QueriesHistory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import {TENANT_QUERY_TABS_ID} from '../../../../store/reducers/tenant/constants'
import {setQueryTab} from '../../../../store/reducers/tenant/tenant';
import type {QueryInHistory} from '../../../../types/store/executeQuery';
import {cn} from '../../../../utils/cn';
import {formatDateTime} from '../../../../utils/dataFormatters/dataFormatters';
import {useTypedDispatch, useTypedSelector} from '../../../../utils/hooks';
import {formatToMs, parseUsToMs} from '../../../../utils/timeParsers';
import {MAX_QUERY_HEIGHT, QUERY_TABLE_SETTINGS} from '../../utils/constants';
import i18n from '../i18n';

Expand Down Expand Up @@ -46,7 +48,7 @@ function QueriesHistory({changeUserInput}: QueriesHistoryProps) {
const columns: Column<QueryInHistory>[] = [
{
name: 'queryText',
header: 'Query Text',
header: i18n('history.queryText'),
render: ({row}) => {
return (
<div className={b('query')}>
Expand All @@ -57,6 +59,23 @@ function QueriesHistory({changeUserInput}: QueriesHistoryProps) {
sortable: false,
width: 600,
},
{
name: 'EndTime',
header: i18n('history.endTime'),
render: ({row}) =>
row.endTime ? formatDateTime(new Date(row.endTime as string).getTime()) : '-',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

row.endTime.toString() instead of row.endTime as string, it more error proof

align: 'right',
width: 200,
sortable: false,
},
{
name: 'Duration',
header: i18n('history.duration'),
render: ({row}) => (row.duration ? formatToMs(parseUsToMs(row.duration)) : '-'),
align: 'right',
width: 150,
sortable: false,
},
];

return (
Expand Down
2 changes: 1 addition & 1 deletion src/containers/Tenant/Query/QueryEditor/QueryEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ function QueryEditor(props: QueryEditorProps) {
if (!text) {
const {queries, currentIndex} = history;
if (query !== queries[currentIndex]?.queryText) {
props.saveQueryToHistory(input);
props.saveQueryToHistory(input, queryId);
}
}
dispatchResultVisibilityState(PaneVisibilityActionTypes.triggerExpand);
Expand Down
6 changes: 5 additions & 1 deletion src/containers/Tenant/Query/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,9 @@
"filter.text.placeholder": "Search by query text...",

"gear.tooltip": "Query execution settings have been changed for ",
"banner.query-settings.message": "Query results are displayed for "
"banner.query-settings.message": "Query results are displayed for ",

"history.queryText": "Query text",
"history.endTime": "End time",
"history.duration": "Duration"
}
69 changes: 64 additions & 5 deletions src/store/reducers/executeQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const MAXIMUM_QUERIES_IN_HISTORY = 20;

const CHANGE_USER_INPUT = 'query/CHANGE_USER_INPUT';
const SAVE_QUERY_TO_HISTORY = 'query/SAVE_QUERY_TO_HISTORY';
const UPDATE_QUERY_IN_HISTORY = 'query/UPDATE_QUERY_IN_HISTORY';
const SET_QUERY_HISTORY_FILTER = 'query/SET_QUERY_HISTORY_FILTER';
const GO_TO_PREVIOUS_QUERY = 'query/GO_TO_PREVIOUS_QUERY';
const GO_TO_NEXT_QUERY = 'query/GO_TO_NEXT_QUERY';
Expand Down Expand Up @@ -65,9 +66,9 @@ const executeQuery: Reducer<ExecuteQueryState, ExecuteQueryAction> = (
}

case SAVE_QUERY_TO_HISTORY: {
const queryText = action.data;
const {queryText, queryId} = action.data;

const newQueries = [...state.history.queries, {queryText}].slice(
const newQueries = [...state.history.queries, {queryText, queryId}].slice(
state.history.queries.length >= MAXIMUM_QUERIES_IN_HISTORY ? 1 : 0,
);
settingsManager.setUserSettingsValue(QUERIES_HISTORY_KEY, newQueries);
Expand All @@ -82,6 +83,38 @@ const executeQuery: Reducer<ExecuteQueryState, ExecuteQueryAction> = (
};
}

case UPDATE_QUERY_IN_HISTORY: {
const {queryId, stats} = action.data;

if (!stats) {
return state;
}

const index = state.history.queries.findIndex((item) => item.queryId === queryId);

if (index === -1) {
return state;
}

const newQueries = [...state.history.queries];
const {duration, endTime} = stats;
newQueries.splice(index, 1, {
...state.history.queries[index],
duration,
endTime,
});

settingsManager.setUserSettingsValue(QUERIES_HISTORY_KEY, newQueries);

return {
...state,
history: {
...state.history,
queries: newQueries,
},
};
}

case GO_TO_PREVIOUS_QUERY: {
const currentIndex = state.history.currentIndex;
if (currentIndex <= 0) {
Expand Down Expand Up @@ -150,6 +183,11 @@ interface SendQueryParams extends QueryRequestParams {
enableTracingLevel?: boolean;
}

interface QueryStats {
duration?: string | number;
endTime?: string | number;
}

export const executeQueryApi = api.injectEndpoints({
endpoints: (build) => ({
executeQuery: build.mutation<IQueryResult, SendQueryParams>({
Expand All @@ -162,7 +200,7 @@ export const executeQueryApi = api.injectEndpoints({
enableTracingLevel,
queryId,
},
{signal},
{signal, dispatch},
) => {
let action: ExecuteActions = 'execute';
let syntax: QuerySyntax = QUERY_SYNTAX.yql;
Expand All @@ -175,6 +213,7 @@ export const executeQueryApi = api.injectEndpoints({
}

try {
const timeStart = Date.now();
const response = await window.api.sendQuery(
{
schema,
Expand All @@ -201,6 +240,19 @@ export const executeQueryApi = api.injectEndpoints({
}

const data = parseQueryAPIExecuteResponse(response);

const queryStats: QueryStats = {};
if (data.stats) {
const {DurationUs, Executions: [{FinishTimeMs}] = [{}]} = data.stats;
queryStats.duration = DurationUs;
queryStats.endTime = FinishTimeMs;
} else {
const now = Date.now();
queryStats.duration = (now - timeStart) * 1000;
queryStats.endTime = now;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about saving duration as ms? us is not very common thing, it's become important not to forget all that conversions (*1000 or parseUsToMs).

In both cases I suggest adding metric to field name: durationMs or durationUs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep existing format from api. we could replace us to ms in all places, but usually I prefer to keep original data as is (if api returns us - it means there are cases where this precision is important)

}

dispatch(updateQueryInHistory(queryStats, queryId));
return {data};
} catch (error) {
return {error};
Expand All @@ -211,13 +263,20 @@ export const executeQueryApi = api.injectEndpoints({
overrideExisting: 'throw',
});

export const saveQueryToHistory = (queryText: string) => {
export const saveQueryToHistory = (queryText: string, queryId: string) => {
return {
type: SAVE_QUERY_TO_HISTORY,
data: queryText,
data: {queryText, queryId},
} as const;
};

export function updateQueryInHistory(stats: QueryStats, queryId: string) {
return {
type: UPDATE_QUERY_IN_HISTORY,
data: {queryId, stats},
} as const;
}

export const goToPreviousQuery = () => {
return {
type: GO_TO_PREVIOUS_QUERY,
Expand Down
5 changes: 5 additions & 0 deletions src/types/store/executeQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ import type {
saveQueryToHistory,
setQueryHistoryFilter,
setTenantPath,
updateQueryInHistory,
} from '../../store/reducers/executeQuery';

export interface QueryInHistory {
queryId?: string;
queryText: string;
syntax?: string;
endTime?: string | number;
duration?: string | number;
}

export interface ExecuteQueryState {
Expand All @@ -29,6 +33,7 @@ export type ExecuteQueryAction =
| ReturnType<typeof goToPreviousQuery>
| ReturnType<typeof changeUserInput>
| ReturnType<typeof saveQueryToHistory>
| ReturnType<typeof updateQueryInHistory>
| ReturnType<typeof setTenantPath>
| ReturnType<typeof setQueryHistoryFilter>;

Expand Down
Loading