From 239fb2a466bf5a5ba81dda965e48381926511ede Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 8 Feb 2023 15:36:28 -0800 Subject: [PATCH 1/3] feat(sqllab): Add event logger --- superset-frontend/src/SqlLab/App.jsx | 3 +- .../src/SqlLab/components/App/App.test.jsx | 49 +++++++++++++++---- .../src/SqlLab/components/App/index.jsx | 17 +++++-- superset-frontend/src/logger/LogUtils.ts | 2 + .../src/middleware/loggerMiddleware.js | 9 +++- 5 files changed, 66 insertions(+), 14 deletions(-) diff --git a/superset-frontend/src/SqlLab/App.jsx b/superset-frontend/src/SqlLab/App.jsx index a3a939aabd3b6..b75ea5c624bb6 100644 --- a/superset-frontend/src/SqlLab/App.jsx +++ b/superset-frontend/src/SqlLab/App.jsx @@ -31,6 +31,7 @@ import { } from 'src/featureFlags'; import setupExtensions from 'src/setup/setupExtensions'; import getBootstrapData from 'src/utils/getBootstrapData'; +import logger from 'src/middleware/loggerMiddleware'; import getInitialState from './reducers/getInitialState'; import rootReducer from './reducers/index'; import { initEnhancer } from '../reduxUtils'; @@ -116,7 +117,7 @@ const store = createStore( rootReducer, initialState, compose( - applyMiddleware(thunkMiddleware), + applyMiddleware(thunkMiddleware, logger), initEnhancer( !isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE), sqlLabPersistStateConfig, diff --git a/superset-frontend/src/SqlLab/components/App/App.test.jsx b/superset-frontend/src/SqlLab/components/App/App.test.jsx index 0629de27d5d6c..c06262915637a 100644 --- a/superset-frontend/src/SqlLab/components/App/App.test.jsx +++ b/superset-frontend/src/SqlLab/components/App/App.test.jsx @@ -19,21 +19,29 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; - -import { shallow } from 'enzyme'; +import { render } from 'spec/helpers/testing-library'; import App from 'src/SqlLab/components/App'; -import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors'; import sqlLabReducer from 'src/SqlLab/reducers/index'; +import { LOCALSTORAGE_MAX_USAGE_KB } from 'src/SqlLab/constants'; +import { LOG_EVENT } from 'src/logger/actions'; + +jest.mock('src/SqlLab/components/TabbedSqlEditors', () => () => ( +
+)); +jest.mock('src/SqlLab/components/QueryAutoRefresh', () => () => ( +
+)); describe('SqlLab App', () => { const middlewares = [thunk]; const mockStore = configureStore(middlewares); const store = mockStore(sqlLabReducer(undefined, {}), {}); - let wrapper; - beforeEach(() => { - wrapper = shallow().dive(); + jest.useFakeTimers(); + }); + afterEach(() => { + jest.useRealTimers(); }); it('is valid', () => { @@ -41,8 +49,31 @@ describe('SqlLab App', () => { }); it('should render', () => { - const inner = wrapper.dive(); - expect(inner.find('.SqlLab')).toHaveLength(1); - expect(inner.find(TabbedSqlEditors)).toHaveLength(1); + const { getByTestId } = render(, { useRedux: true, store }); + expect(getByTestId('SqlLabApp')).toBeInTheDocument(); + expect(getByTestId('mock-tabbed-sql-editors')).toBeInTheDocument(); + }); + + it('logs current usage warning', async () => { + const localStorageUsageInKilobytes = LOCALSTORAGE_MAX_USAGE_KB + 10; + const storeExceedLocalStorage = mockStore( + sqlLabReducer( + { + localStorageUsageInKilobytes, + }, + {}, + ), + ); + + const { rerender } = render(, { + useRedux: true, + store: storeExceedLocalStorage, + }); + rerender(); + expect(storeExceedLocalStorage.getActions()).toContainEqual( + expect.objectContaining({ + type: LOG_EVENT, + }), + ); }); }); diff --git a/superset-frontend/src/SqlLab/components/App/index.jsx b/superset-frontend/src/SqlLab/components/App/index.jsx index 47fce27db1931..4689f8ec21912 100644 --- a/superset-frontend/src/SqlLab/components/App/index.jsx +++ b/superset-frontend/src/SqlLab/components/App/index.jsx @@ -29,6 +29,8 @@ import { LOCALSTORAGE_WARNING_MESSAGE_THROTTLE_MS, } from 'src/SqlLab/constants'; import * as Actions from 'src/SqlLab/actions/sqlLab'; +import { logEvent } from 'src/logger/actions'; +import { LOG_ACTIONS_SQLLAB_WARN_LOCAL_STORAGE_USAGE } from 'src/logger/LogUtils'; import TabbedSqlEditors from '../TabbedSqlEditors'; import QueryAutoRefresh from '../QueryAutoRefresh'; @@ -125,6 +127,7 @@ class App extends React.PureComponent { ) { this.showLocalStorageUsageWarning( this.props.localStorageUsageInKilobytes, + this.props.queries?.lenghth || 0, ); } } @@ -140,7 +143,7 @@ class App extends React.PureComponent { this.setState({ hash: window.location.hash }); } - showLocalStorageUsageWarning(currentUsage) { + showLocalStorageUsageWarning(currentUsage, queryCount) { this.props.actions.addDangerToast( t( "SQL Lab uses your browser's local storage to store queries and results." + @@ -154,6 +157,14 @@ class App extends React.PureComponent { }, ), ); + const eventData = { + current_usage: currentUsage, + query_count: queryCount, + }; + this.props.actions.logEvent( + LOG_ACTIONS_SQLLAB_WARN_LOCAL_STORAGE_USAGE, + eventData, + ); } render() { @@ -162,7 +173,7 @@ class App extends React.PureComponent { return window.location.replace('/superset/sqllab/history/'); } return ( - + next => action => { return next(action); } - const { dashboardInfo, explore, impressionId, dashboardLayout } = + const { dashboardInfo, explore, impressionId, dashboardLayout, sqlLab } = store.getState(); let logMetadata = { impression_id: impressionId, @@ -90,6 +90,13 @@ const loggerMiddleware = store => next => action => { source_id: explore.slice ? explore.slice.slice_id : 0, ...logMetadata, }; + } else if (sqlLab) { + logMetadata = { + source: 'sqlLab', + source_id: sqlLab.queryEditors.find( + ({ id }) => id === sqlLab.tabHistory.slice(-1)[0], + )?.dbId, + }; } const { eventName } = action.payload; From 40254545428adf8b6613225a293d252f03b34901 Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 8 Feb 2023 16:03:47 -0800 Subject: [PATCH 2/3] add schema value on eventData --- superset-frontend/src/middleware/loggerMiddleware.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/middleware/loggerMiddleware.js b/superset-frontend/src/middleware/loggerMiddleware.js index c4746070b3fd8..99f1b36098013 100644 --- a/superset-frontend/src/middleware/loggerMiddleware.js +++ b/superset-frontend/src/middleware/loggerMiddleware.js @@ -91,11 +91,13 @@ const loggerMiddleware = store => next => action => { ...logMetadata, }; } else if (sqlLab) { + const editor = sqlLab.queryEditors.find( + ({ id }) => id === sqlLab.tabHistory.slice(-1)[0], + ); logMetadata = { source: 'sqlLab', - source_id: sqlLab.queryEditors.find( - ({ id }) => id === sqlLab.tabHistory.slice(-1)[0], - )?.dbId, + source_id: editor?.dbId, + schema: editor?.schema, }; } From e377fbc96cfe48d06121f29c61c04a6383b75615 Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 8 Feb 2023 16:06:20 -0800 Subject: [PATCH 3/3] update source_id using editor id --- superset-frontend/src/middleware/loggerMiddleware.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/middleware/loggerMiddleware.js b/superset-frontend/src/middleware/loggerMiddleware.js index 99f1b36098013..475c1784cce55 100644 --- a/superset-frontend/src/middleware/loggerMiddleware.js +++ b/superset-frontend/src/middleware/loggerMiddleware.js @@ -96,7 +96,8 @@ const loggerMiddleware = store => next => action => { ); logMetadata = { source: 'sqlLab', - source_id: editor?.dbId, + source_id: editor?.id, + db_id: editor?.dbId, schema: editor?.schema, }; }