You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by cc...@apache.org on 2018/06/22 00:54:30 UTC
[incubator-superset] 15/26: [dashboard v2] logging updates (#5087)
This is an automated email from the ASF dual-hosted git repository.
ccwilliams pushed a commit to branch dashboard-builder
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit 69ab5f7af569dc2a5851526aaef8e68baa15e0cd
Author: Chris Williams <wi...@users.noreply.github.com>
AuthorDate: Thu May 31 19:02:11 2018 -0700
[dashboard v2] logging updates (#5087)
* [dashboard v2] initial logging refactor
* [dashboard v2] clean up logger
* [logger] update explore with new log events, add refresh dashboard + refresh dashboard chart actions
* [logging] add logger_spec.js, fix reducers/dashboardState_spec + gridComponents/Chart_spec
* [dashboard v2][logging] refactor for bulk logging in python
* [logging] tweak python, fix and remove dup start_offset entries
* [dashboard v2][logging] add dashboard_first_load event
* [dashboard v2][logging] add slice_ids to dashboard pane load event
* [tests] fix npm test script
---
superset/assets/package.json | 2 +-
.../dashboard/components/Dashboard_spec.jsx | 1 +
.../components/gridComponents/Chart_spec.jsx | 10 +-
.../dashboard/reducers/dashboardState_spec.js | 52 +++----
superset/assets/spec/javascripts/logger_spec.js | 143 +++++++++++++++++++
superset/assets/src/chart/Chart.jsx | 80 +++++------
superset/assets/src/chart/chartAction.js | 27 ++--
.../assets/src/dashboard/actions/dashboardState.js | 28 ++--
.../assets/src/dashboard/components/Dashboard.jsx | 71 +++++++---
.../src/dashboard/components/DashboardBuilder.jsx | 2 -
.../dashboard/components/SliceHeaderControls.jsx | 50 ++++++-
.../dashboard/components/gridComponents/Chart.jsx | 9 +-
superset/assets/src/dashboard/containers/Chart.jsx | 4 +-
.../assets/src/dashboard/containers/Dashboard.jsx | 8 +-
.../assets/src/dashboard/containers/SliceAdder.jsx | 28 ++++
.../src/dashboard/reducers/dashboardState.js | 49 ++++---
.../dashboard/util/logging/childChartsDidLoad.js | 21 +++
.../util/logging/findNonTabChildChartIds.js | 45 ++++++
.../util/logging/findTopLevelComponentIds.js | 74 ++++++++++
.../logging/getLoadStatsPerTopLevelComponent.js | 26 ++++
superset/assets/src/dashboard/util/propShapes.jsx | 13 ++
.../explore/components/ExploreViewContainer.jsx | 121 ++++++++--------
superset/assets/src/logger.js | 153 ++++++++++++++-------
superset/models/core.py | 48 ++++---
24 files changed, 768 insertions(+), 297 deletions(-)
diff --git a/superset/assets/package.json b/superset/assets/package.json
index 6d116a4..21abd17 100644
--- a/superset/assets/package.json
+++ b/superset/assets/package.json
@@ -8,7 +8,7 @@
"test": "spec"
},
"scripts": {
- "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js --recursive spec/**/*_spec.*",
+ "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js --recursive spec/**/**/*_spec.*",
"cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --require ignore-styles spec/helpers/browser.js --recursive spec/**/*_spec.*",
"dev": "NODE_ENV=dev webpack --watch --colors --progress --debug --output-pathinfo --devtool eval-cheap-source-map",
"dev-slow": "NODE_ENV=dev webpack --watch --colors --progress --debug --output-pathinfo --devtool inline-source-map",
diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
index 545b890..2094040 100644
--- a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
+++ b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx
@@ -35,6 +35,7 @@ describe('Dashboard', () => {
timeout: 60,
userId: dashboardInfo.userId,
impressionId: 'id',
+ loadStats: {},
};
function setup(overrideProps) {
diff --git a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
index 05756f4..5fff313 100644
--- a/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
+++ b/superset/assets/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
@@ -35,9 +35,10 @@ describe('Chart', () => {
refreshChart() {},
toggleExpandSlice() {},
addFilter() {},
- removeFilter() {},
editMode: false,
isExpanded: false,
+ supersetCanExplore: false,
+ sliceCanEdit: false,
};
function setup(overrideProps) {
@@ -77,13 +78,6 @@ describe('Chart', () => {
expect(addFilter.callCount).to.equal(1);
});
- it('should call removeFilter when ChartContainer calls removeFilter', () => {
- const removeFilter = sinon.spy();
- const wrapper = setup({ removeFilter });
- wrapper.instance().removeFilter();
- expect(removeFilter.callCount).to.equal(1);
- });
-
it('should return props.filters when its getFilters method is called', () => {
const filters = { column: ['value'] };
const wrapper = setup({ filters });
diff --git a/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js b/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js
index 078019d..89c4ffe 100644
--- a/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js
+++ b/superset/assets/spec/javascripts/dashboard/reducers/dashboardState_spec.js
@@ -3,11 +3,10 @@ import { expect } from 'chai';
import {
ADD_SLICE,
- ADD_FILTER,
+ CHANGE_FILTER,
ON_CHANGE,
ON_SAVE,
REMOVE_SLICE,
- REMOVE_FILTER,
SET_EDIT_MODE,
SET_MAX_UNDO_HISTORY_EXCEEDED,
SET_UNSAVED_CHANGES,
@@ -138,7 +137,7 @@ describe('dashboardState reducer', () => {
});
});
- describe('add filter', () => {
+ describe('change filter', () => {
it('should add a new filter if it does not exist', () => {
expect(
dashboardStateReducer(
@@ -147,7 +146,7 @@ describe('dashboardState reducer', () => {
sliceIds: [1],
},
{
- type: ADD_FILTER,
+ type: CHANGE_FILTER,
chart: { id: 1, formData: { groupby: 'column' } },
col: 'column',
vals: ['b', 'a'],
@@ -172,7 +171,7 @@ describe('dashboardState reducer', () => {
sliceIds: [1],
},
{
- type: ADD_FILTER,
+ type: CHANGE_FILTER,
chart: { id: 1, formData: { groupby: 'column' } },
col: 'column',
vals: ['b', 'a'],
@@ -197,7 +196,7 @@ describe('dashboardState reducer', () => {
sliceIds: [1],
},
{
- type: ADD_FILTER,
+ type: CHANGE_FILTER,
chart: { id: 1, formData: { groupby: 'column' } },
col: 'column',
vals: ['b', 'a'],
@@ -211,29 +210,30 @@ describe('dashboardState reducer', () => {
sliceIds: [1],
});
});
- });
- it('should remove a filter', () => {
- expect(
- dashboardStateReducer(
- {
- filters: {
- 1: {
- column: ['a', 'b', 'c'],
+ it('should remove the filter if values are empty', () => {
+ expect(
+ dashboardStateReducer(
+ {
+ filters: {
+ 1: { column: ['z'] },
},
+ sliceIds: [1],
},
- },
- {
- type: REMOVE_FILTER,
- sliceId: 1,
- col: 'column',
- vals: ['b', 'a'], // these are removed
- refresh: true,
- },
- ),
- ).to.deep.equal({
- filters: { 1: { column: ['c'] } },
- refresh: true,
+ {
+ type: CHANGE_FILTER,
+ chart: { id: 1, formData: { groupby: 'column' } },
+ col: 'column',
+ vals: [],
+ refresh: true,
+ merge: false,
+ },
+ ),
+ ).to.deep.equal({
+ filters: {},
+ refresh: true,
+ sliceIds: [1],
+ });
});
});
});
diff --git a/superset/assets/spec/javascripts/logger_spec.js b/superset/assets/spec/javascripts/logger_spec.js
new file mode 100644
index 0000000..64580b4
--- /dev/null
+++ b/superset/assets/spec/javascripts/logger_spec.js
@@ -0,0 +1,143 @@
+import $ from 'jquery';
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import sinon from 'sinon';
+
+import { Logger, ActionLog } from '../../src/logger';
+
+describe('ActionLog', () => {
+ it('should be a constructor', () => {
+ const newLogger = new ActionLog({});
+ expect(newLogger instanceof ActionLog).to.equal(true);
+ });
+
+ it('should set the eventNames, impressionId, source, sourceId, and sendNow init parameters', () => {
+ const eventNames = [];
+ const impressionId = 'impressionId';
+ const source = 'source';
+ const sourceId = 'sourceId';
+ const sendNow = true;
+
+ const log = new ActionLog({ eventNames, impressionId, source, sourceId, sendNow });
+ expect(log.eventNames).to.equal(eventNames);
+ expect(log.impressionId).to.equal(impressionId);
+ expect(log.source).to.equal(source);
+ expect(log.sourceId).to.equal(sourceId);
+ expect(log.sendNow).to.equal(sendNow);
+ });
+
+ it('should set attributes with the setAttribute method', () => {
+ const log = new ActionLog({});
+ expect(log.test).to.equal(undefined);
+ log.setAttribute('test', 'testValue');
+ expect(log.test).to.equal('testValue');
+ });
+
+ it('should track added events', () => {
+ const log = new ActionLog({});
+ const eventName = 'myEventName';
+ const eventBody = { test: 'event' };
+ expect(log.events[eventName]).to.equal(undefined);
+
+ log.addEvent(eventName, eventBody);
+ expect(log.events[eventName]).to.have.length(1);
+ expect(log.events[eventName][0]).to.deep.include(eventBody);
+ });
+});
+
+describe('Logger', () => {
+ it('should add events when .append(eventName, eventBody) is called', () => {
+ const eventName = 'testEvent';
+ const eventBody = { test: 'event' };
+ const log = new ActionLog({ eventNames: [eventName] });
+ Logger.start(log);
+ Logger.append(eventName, eventBody);
+ expect(log.events[eventName]).to.have.length(1);
+ expect(log.events[eventName][0]).to.deep.include(eventBody);
+ Logger.end(log);
+ });
+
+ describe('.send()', () => {
+ beforeEach(() => {
+ sinon.spy($, 'ajax');
+ });
+ afterEach(() => {
+ $.ajax.restore();
+ });
+
+ const eventNames = ['test'];
+
+ function setup(overrides = {}) {
+ const log = new ActionLog({ eventNames, ...overrides });
+ return log;
+ }
+
+ it('should POST an event to /superset/log/ when called', () => {
+ const log = setup();
+ Logger.start(log);
+ Logger.append(eventNames[0], { test: 'event' });
+ expect(log.events[eventNames[0]]).to.have.length(1);
+ Logger.end(log);
+ expect($.ajax.calledOnce).to.equal(true);
+ const args = $.ajax.getCall(0).args[0];
+ expect(args.url).to.equal('/superset/log/');
+ expect(args.method).to.equal('POST');
+ });
+
+ it("should flush the log's events", () => {
+ const log = setup();
+ Logger.start(log);
+ Logger.append(eventNames[0], { test: 'event' });
+ const event = log.events[eventNames[0]][0];
+ expect(event).to.deep.include({ test: 'event' });
+ Logger.end(log);
+ expect(log.events).to.deep.equal({});
+ });
+
+ it('should include ts, start_offset, event_name, impression_id, source, and source_id in every event', () => {
+ const config = {
+ eventNames: ['event1', 'event2'],
+ impressionId: 'impress_me',
+ source: 'superset',
+ sourceId: 'lolz',
+ };
+ const log = setup(config);
+
+ Logger.start(log);
+ Logger.append('event1', { key: 'value' });
+ Logger.append('event2', { foo: 'bar' });
+ Logger.end(log);
+
+ const args = $.ajax.getCall(0).args[0];
+ const events = JSON.parse(args.data.events);
+
+ expect(events).to.have.length(2);
+ expect(events[0]).to.deep.include({
+ key: 'value',
+ event_name: 'event1',
+ impression_id: config.impressionId,
+ source: config.source,
+ source_id: config.sourceId,
+ });
+ expect(events[1]).to.deep.include({
+ foo: 'bar',
+ event_name: 'event2',
+ impression_id: config.impressionId,
+ source: config.source,
+ source_id: config.sourceId,
+ });
+ expect(typeof events[0].ts).to.equal('number');
+ expect(typeof events[1].ts).to.equal('number');
+ expect(typeof events[0].start_offset).to.equal('number');
+ expect(typeof events[1].start_offset).to.equal('number');
+ });
+
+ it('should send() a log immediately if .append() is called with sendNow=true', () => {
+ const log = setup();
+ Logger.start(log);
+ Logger.append(eventNames[0], { test: 'event' }, true);
+ expect($.ajax.calledOnce).to.equal(true);
+ Logger.end(log);
+ });
+ });
+});
diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
index 4a471e8..060249f 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/superset/assets/src/chart/Chart.jsx
@@ -7,7 +7,7 @@ import { Tooltip } from 'react-bootstrap';
import { d3format } from '../modules/utils';
import ChartBody from './ChartBody';
import Loading from '../components/Loading';
-import { Logger, LOG_ACTIONS_RENDER_EVENT } from '../logger';
+import { Logger, LOG_ACTIONS_RENDER_CHART } from '../logger';
import StackTraceMessage from '../components/StackTraceMessage';
import RefreshChartOverlay from '../components/RefreshChartOverlay';
import visMap from '../visualizations';
@@ -42,7 +42,6 @@ const propTypes = {
// dashboard callbacks
addFilter: PropTypes.func,
getFilters: PropTypes.func,
- removeFilter: PropTypes.func,
onQuery: PropTypes.func,
onDismissRefreshOverlay: PropTypes.func,
};
@@ -50,7 +49,6 @@ const propTypes = {
const defaultProps = {
addFilter: () => ({}),
getFilters: () => ({}),
- removeFilter: () => ({}),
};
class Chart extends React.PureComponent {
@@ -65,7 +63,6 @@ class Chart extends React.PureComponent {
this.datasource = props.datasource;
this.addFilter = this.addFilter.bind(this);
this.getFilters = this.getFilters.bind(this);
- this.removeFilter = this.removeFilter.bind(this);
this.headerHeight = this.headerHeight.bind(this);
this.height = this.height.bind(this);
this.width = this.width.bind(this);
@@ -74,12 +71,7 @@ class Chart extends React.PureComponent {
componentDidMount() {
if (this.props.triggerQuery) {
const { formData } = this.props;
- this.props.actions.runQuery(
- formData,
- false,
- this.props.timeout,
- this.props.chartId,
- );
+ this.props.actions.runQuery(formData, false, this.props.timeout, this.props.chartId);
} else {
// when drag/dropping in a dashboard, a chart may be unmounted/remounted but still have data
this.renderViz();
@@ -98,13 +90,12 @@ class Chart extends React.PureComponent {
if (
this.props.queryResponse &&
['success', 'rendered'].indexOf(this.props.chartStatus) > -1 &&
- !this.props.queryResponse.error && (
- prevProps.annotationData !== this.props.annotationData ||
+ !this.props.queryResponse.error &&
+ (prevProps.annotationData !== this.props.annotationData ||
prevProps.queryResponse !== this.props.queryResponse ||
prevProps.height !== this.props.height ||
prevProps.width !== this.props.width ||
- prevProps.lastRendered !== this.props.lastRendered
- )
+ prevProps.lastRendered !== this.props.lastRendered)
) {
this.renderViz();
}
@@ -122,17 +113,14 @@ class Chart extends React.PureComponent {
this.props.addFilter(col, vals, merge, refresh);
}
- removeFilter(col, vals, refresh = true) {
- this.props.removeFilter(col, vals, refresh);
- }
-
clearError() {
this.setState({ errorMsg: null });
}
width() {
- return this.props.width ||
- (this.container && this.container.el && this.container.el.offsetWidth);
+ return (
+ this.props.width || (this.container && this.container.el && this.container.el.offsetWidth)
+ );
}
headerHeight() {
@@ -140,8 +128,9 @@ class Chart extends React.PureComponent {
}
height() {
- return this.props.height
- || (this.container && this.container.el && this.container.el.offsetHeight);
+ return (
+ this.props.height || (this.container && this.container.el && this.container.el.offsetHeight)
+ );
}
d3format(col, number) {
@@ -200,7 +189,7 @@ class Chart extends React.PureComponent {
if (chartStatus !== 'rendered') {
this.props.actions.chartRenderingSucceeded(chartId);
}
- Logger.append(LOG_ACTIONS_RENDER_EVENT, {
+ Logger.append(LOG_ACTIONS_RENDER_CHART, {
label: 'slice_' + chartId,
vis_type: vizType,
start_offset: renderStart,
@@ -222,36 +211,39 @@ class Chart extends React.PureComponent {
<div className={`chart-container ${isLoading ? 'is-loading' : ''}`} style={containerStyles}>
{this.renderTooltip()}
{isLoading && <Loading size={75} />}
- {this.props.chartAlert &&
+ {this.props.chartAlert && (
<StackTraceMessage
message={this.props.chartAlert}
queryResponse={this.props.queryResponse}
- />}
+ />
+ )}
{!isLoading &&
!this.props.chartAlert &&
this.props.refreshOverlayVisible &&
!this.props.errorMessage &&
- this.container &&
- <RefreshChartOverlay
- height={this.height()}
- width={this.width()}
- onQuery={this.props.onQuery}
- onDismiss={this.props.onDismissRefreshOverlay}
- />}
+ this.container && (
+ <RefreshChartOverlay
+ height={this.height()}
+ width={this.width()}
+ onQuery={this.props.onQuery}
+ onDismiss={this.props.onDismissRefreshOverlay}
+ />
+ )}
- {!isLoading && !this.props.chartAlert &&
- <ChartBody
- containerId={this.containerId}
- vizType={this.props.vizType}
- height={this.height}
- width={this.width}
- faded={this.props.refreshOverlayVisible && !this.props.errorMessage}
- ref={(inner) => {
- this.container = inner;
- }}
- />
- }
+ {!isLoading &&
+ !this.props.chartAlert && (
+ <ChartBody
+ containerId={this.containerId}
+ vizType={this.props.vizType}
+ height={this.height}
+ width={this.width}
+ faded={this.props.refreshOverlayVisible && !this.props.errorMessage}
+ ref={(inner) => {
+ this.container = inner;
+ }}
+ />
+ )}
</div>
);
}
diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js
index 529d295..82c4250 100644
--- a/superset/assets/src/chart/chartAction.js
+++ b/superset/assets/src/chart/chartAction.js
@@ -1,10 +1,10 @@
import { getExploreUrlAndPayload, getAnnotationJsonUrl } from '../explore/exploreUtils';
import { requiresQuery, ANNOTATION_SOURCE_TYPES } from '../modules/AnnotationTypes';
-import { Logger, LOG_ACTIONS_LOAD_EVENT } from '../logger';
+import { Logger, LOG_ACTIONS_LOAD_CHART } from '../logger';
import { COMMON_ERR_MESSAGES } from '../common';
import { t } from '../locales';
-const $ = window.$ = require('jquery');
+const $ = (window.$ = require('jquery'));
export const CHART_UPDATE_STARTED = 'CHART_UPDATE_STARTED';
export function chartUpdateStarted(queryRequest, latestQueryFormData, key) {
@@ -74,11 +74,13 @@ export function runAnnotationQuery(annotation, timeout = 60, formData = null, ke
fd.time_grain_sqla = granularity;
fd.granularity = granularity;
- const sliceFormData = Object.keys(annotation.overrides)
- .reduce((d, k) => ({
+ const sliceFormData = Object.keys(annotation.overrides).reduce(
+ (d, k) => ({
...d,
[k]: annotation.overrides[k] || fd[k],
- }), {});
+ }),
+ {},
+ );
const isNative = annotation.sourceType === ANNOTATION_SOURCE_TYPES.NATIVE;
const url = getAnnotationJsonUrl(annotation.value, sliceFormData, isNative);
const queryRequest = $.ajax({
@@ -143,19 +145,22 @@ export function runQuery(formData, force = false, timeout = 60, key) {
const queryPromise = Promise.resolve(dispatch(chartUpdateStarted(queryRequest, payload, key)))
.then(() => queryRequest)
.then((queryResponse) => {
- Logger.append(LOG_ACTIONS_LOAD_EVENT, {
- label: 'slice_' + key,
+ Logger.append(LOG_ACTIONS_LOAD_CHART, {
+ slice_id: 'slice_' + key,
is_cached: queryResponse.is_cached,
+ force_refresh: force,
row_count: queryResponse.rowcount,
datasource: formData.datasource,
start_offset: logStart,
duration: Logger.getTimestamp() - logStart,
+ has_extra_filters: formData.extra_filters && formData.extra_filters.length > 0,
+ viz_type: formData.viz_type,
});
return dispatch(chartUpdateSucceeded(queryResponse, key));
})
.catch((err) => {
- Logger.append(LOG_ACTIONS_LOAD_EVENT, {
- label: key,
+ Logger.append(LOG_ACTIONS_LOAD_CHART, {
+ slice_id: 'slice_' + key,
has_err: true,
datasource: formData.datasource,
start_offset: logStart,
@@ -197,7 +202,5 @@ export function runQuery(formData, force = false, timeout = 60, key) {
}
export function refreshChart(chart, force, timeout) {
- return dispatch => (
- dispatch(runQuery(chart.latestQueryFormData, force, timeout, chart.id))
- );
+ return dispatch => dispatch(runQuery(chart.latestQueryFormData, force, timeout, chart.id));
}
diff --git a/superset/assets/src/dashboard/actions/dashboardState.js b/superset/assets/src/dashboard/actions/dashboardState.js
index aac4f98..688f5b0 100644
--- a/superset/assets/src/dashboard/actions/dashboardState.js
+++ b/superset/assets/src/dashboard/actions/dashboardState.js
@@ -7,6 +7,11 @@ import { chart as initChart } from '../../chart/chartReducer';
import { fetchDatasourceMetadata } from '../../dashboard/actions/datasources';
import { applyDefaultFormData } from '../../explore/store';
import { getAjaxErrorMsg } from '../../modules/utils';
+import {
+ Logger,
+ LOG_ACTIONS_CHANGE_DASHBOARD_FILTER,
+ LOG_ACTIONS_REFRESH_DASHBOARD,
+} from '../../logger';
import { SAVE_TYPE_OVERWRITE } from '../util/constants';
import { t } from '../../locales';
@@ -21,14 +26,16 @@ export function setUnsavedChanges(hasUnsavedChanges) {
return { type: SET_UNSAVED_CHANGES, payload: { hasUnsavedChanges } };
}
-export const ADD_FILTER = 'ADD_FILTER';
-export function addFilter(chart, col, vals, merge = true, refresh = true) {
- return { type: ADD_FILTER, chart, col, vals, merge, refresh };
-}
-
-export const REMOVE_FILTER = 'REMOVE_FILTER';
-export function removeFilter(sliceId, col, vals, refresh = true) {
- return { type: REMOVE_FILTER, sliceId, col, vals, refresh };
+export const CHANGE_FILTER = 'CHANGE_FILTER';
+export function changeFilter(chart, col, vals, merge = true, refresh = true) {
+ Logger.append(LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, {
+ id: chart.id,
+ column: col,
+ value_count: Array.isArray(vals) ? vals.length : (vals && 1) || 0,
+ merge,
+ refresh,
+ });
+ return { type: CHANGE_FILTER, chart, col, vals, merge, refresh };
}
export const ADD_SLICE = 'ADD_SLICE';
@@ -130,6 +137,11 @@ export function saveDashboardRequest(data, id, saveType) {
export function fetchCharts(chartList = [], force = false, interval = 0) {
return (dispatch, getState) => {
+ Logger.append(LOG_ACTIONS_REFRESH_DASHBOARD, {
+ force,
+ interval,
+ chartCount: chartList.length,
+ });
const timeout = getState().dashboardInfo.common.conf
.SUPERSET_WEBSERVER_TIMEOUT;
if (!interval) {
diff --git a/superset/assets/src/dashboard/components/Dashboard.jsx b/superset/assets/src/dashboard/components/Dashboard.jsx
index 644ddf0..76f4b54 100644
--- a/superset/assets/src/dashboard/components/Dashboard.jsx
+++ b/superset/assets/src/dashboard/components/Dashboard.jsx
@@ -10,16 +10,19 @@ import {
slicePropShape,
dashboardInfoPropShape,
dashboardStatePropShape,
+ loadStatsPropShape,
} from '../util/propShapes';
import { areObjectsEqual } from '../../reduxUtils';
import getFormDataWithExtraFilters from '../util/charts/getFormDataWithExtraFilters';
import {
Logger,
ActionLog,
- LOG_ACTIONS_PAGE_LOAD,
- LOG_ACTIONS_LOAD_EVENT,
- LOG_ACTIONS_RENDER_EVENT,
+ DASHBOARD_EVENT_NAMES,
+ LOG_ACTIONS_MOUNT_DASHBOARD,
+ LOG_ACTIONS_LOAD_DASHBOARD_PANE,
+ LOG_ACTIONS_FIRST_DASHBOARD_LOAD,
} from '../../logger';
+
import { t } from '../../locales';
import '../stylesheets/index.less';
@@ -35,6 +38,7 @@ const propTypes = {
charts: PropTypes.objectOf(chartPropShape).isRequired,
slices: PropTypes.objectOf(slicePropShape).isRequired,
datasources: PropTypes.object.isRequired,
+ loadStats: loadStatsPropShape.isRequired,
layout: PropTypes.object.isRequired,
impressionId: PropTypes.string.isRequired,
initMessages: PropTypes.array,
@@ -65,28 +69,59 @@ class Dashboard extends React.PureComponent {
constructor(props) {
super(props);
-
- this.firstLoad = true;
- this.loadingLog = new ActionLog({
+ this.isFirstLoad = true;
+ this.actionLog = new ActionLog({
impressionId: props.impressionId,
- actionType: LOG_ACTIONS_PAGE_LOAD,
source: 'dashboard',
sourceId: props.dashboardInfo.id,
- eventNames: [LOG_ACTIONS_LOAD_EVENT, LOG_ACTIONS_RENDER_EVENT],
+ eventNames: DASHBOARD_EVENT_NAMES,
});
- Logger.start(this.loadingLog);
+ Logger.start(this.actionLog);
+ }
+
+ componentDidMount() {
+ this.ts_mount = new Date().getTime();
+ Logger.append(LOG_ACTIONS_MOUNT_DASHBOARD);
}
componentWillReceiveProps(nextProps) {
- if (
- this.firstLoad &&
- Object.values(nextProps.charts).every(
- chart =>
- ['rendered', 'failed', 'stopped'].indexOf(chart.chartStatus) > -1,
- )
- ) {
- Logger.end(this.loadingLog);
- this.firstLoad = false;
+ if (!nextProps.dashboardState.editMode) {
+ // log pane loads
+ const loadedPaneIds = [];
+ const allPanesDidLoad = Object.entries(nextProps.loadStats).every(
+ ([paneId, stats]) => {
+ const { didLoad, minQueryStartTime, ...restStats } = stats;
+
+ if (
+ didLoad &&
+ this.props.loadStats[paneId] &&
+ !this.props.loadStats[paneId].didLoad
+ ) {
+ const duration = new Date().getTime() - minQueryStartTime;
+ Logger.append(LOG_ACTIONS_LOAD_DASHBOARD_PANE, {
+ ...restStats,
+ duration,
+ });
+
+ if (!this.isFirstLoad) {
+ Logger.send(this.actionLog);
+ }
+ }
+ if (this.isFirstLoad && didLoad && stats.slice_ids.length > 0) {
+ loadedPaneIds.push(paneId);
+ }
+ return didLoad || stats.index !== 0;
+ },
+ );
+
+ if (allPanesDidLoad && this.isFirstLoad) {
+ Logger.append(LOG_ACTIONS_FIRST_DASHBOARD_LOAD, {
+ pane_ids: loadedPaneIds,
+ duration: new Date().getTime() - this.ts_mount,
+ });
+ Logger.send(this.actionLog);
+ this.isFirstLoad = false;
+ }
}
const currentChartIds = getChartIdsFromLayout(this.props.layout);
diff --git a/superset/assets/src/dashboard/components/DashboardBuilder.jsx b/superset/assets/src/dashboard/components/DashboardBuilder.jsx
index 0f42f1b..30e2e78 100644
--- a/superset/assets/src/dashboard/components/DashboardBuilder.jsx
+++ b/superset/assets/src/dashboard/components/DashboardBuilder.jsx
@@ -159,8 +159,6 @@ class DashboardBuilder extends React.Component {
id={DASHBOARD_GRID_ID}
activeKey={tabIndex}
onSelect={this.handleChangeTab}
- // these are important for performant loading of tabs. also, there is a
- // react-bootstrap bug where mountOnEnter has no effect unless animation=true
animation
mountOnEnter
unmountOnExit={false}
diff --git a/superset/assets/src/dashboard/components/SliceHeaderControls.jsx b/superset/assets/src/dashboard/components/SliceHeaderControls.jsx
index 6729e57..7d25ab0 100644
--- a/superset/assets/src/dashboard/components/SliceHeaderControls.jsx
+++ b/superset/assets/src/dashboard/components/SliceHeaderControls.jsx
@@ -3,6 +3,12 @@ import PropTypes from 'prop-types';
import cx from 'classnames';
import moment from 'moment';
import { Dropdown, MenuItem } from 'react-bootstrap';
+import {
+ Logger,
+ LOG_ACTIONS_EXPLORE_DASHBOARD_CHART,
+ LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART,
+ LOG_ACTIONS_REFRESH_CHART,
+} from '../../logger';
import { t } from '../../locales';
@@ -42,22 +48,52 @@ const VerticalDotsTrigger = () => (
class SliceHeaderControls extends React.PureComponent {
constructor(props) {
super(props);
- this.exportCSV = this.props.exportCSV.bind(this, this.props.slice.slice_id);
- this.exploreChart = this.props.exploreChart.bind(
- this,
- this.props.slice.slice_id,
- );
+ this.exportCSV = this.exportCSV.bind(this);
+ this.exploreChart = this.exploreChart.bind(this);
+ this.toggleControls = this.toggleControls.bind(this);
+ this.refreshChart = this.refreshChart.bind(this);
this.toggleExpandSlice = this.props.toggleExpandSlice.bind(
this,
this.props.slice.slice_id,
);
- this.toggleControls = this.toggleControls.bind(this);
this.state = {
showControls: false,
};
}
+ exportCSV() {
+ this.props.exportCSV(this.props.slice.slice_id);
+ Logger.append(
+ LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART,
+ {
+ slice_id: this.props.slice.slice_id,
+ is_cached: this.props.isCached,
+ },
+ true,
+ );
+ }
+
+ exploreChart() {
+ this.props.exploreChart(this.props.slice.slice_id);
+ Logger.append(
+ LOG_ACTIONS_EXPLORE_DASHBOARD_CHART,
+ {
+ slice_id: this.props.slice.slice_id,
+ is_cached: this.props.isCached,
+ },
+ true,
+ );
+ }
+
+ refreshChart() {
+ this.props.forceRefresh(this.props.slice.slice_id);
+ Logger.append(LOG_ACTIONS_REFRESH_CHART, {
+ slice_id: this.props.slice.slice_id,
+ is_cached: this.props.isCached,
+ });
+ }
+
toggleControls() {
this.setState({
showControls: !this.state.showControls,
@@ -84,7 +120,7 @@ class SliceHeaderControls extends React.PureComponent {
</Dropdown.Toggle>
<Dropdown.Menu>
- <MenuItem onClick={this.props.forceRefresh}>
+ <MenuItem onClick={this.refreshChart}>
{isCached && <span className="dot" />}
{t('Force refresh')}
{isCached && (
diff --git a/superset/assets/src/dashboard/components/gridComponents/Chart.jsx b/superset/assets/src/dashboard/components/gridComponents/Chart.jsx
index 1ace51d..39c1e81 100644
--- a/superset/assets/src/dashboard/components/gridComponents/Chart.jsx
+++ b/superset/assets/src/dashboard/components/gridComponents/Chart.jsx
@@ -26,7 +26,6 @@ const propTypes = {
refreshChart: PropTypes.func.isRequired,
toggleExpandSlice: PropTypes.func.isRequired,
addFilter: PropTypes.func.isRequired,
- removeFilter: PropTypes.func.isRequired,
editMode: PropTypes.bool.isRequired,
isExpanded: PropTypes.bool.isRequired,
supersetCanExplore: PropTypes.bool.isRequired,
@@ -54,7 +53,6 @@ class Chart extends React.Component {
this.exportCSV = this.exportCSV.bind(this);
this.forceRefresh = this.forceRefresh.bind(this);
this.getFilters = this.getFilters.bind(this);
- this.removeFilter = this.removeFilter.bind(this);
this.resize = this.resize.bind(this);
this.setDescriptionRef = this.setDescriptionRef.bind(this);
this.setHeaderRef = this.setHeaderRef.bind(this);
@@ -140,10 +138,6 @@ class Chart extends React.Component {
return this.props.refreshChart(this.props.chart, true, this.props.timeout);
}
- removeFilter(...args) {
- this.props.removeFilter(this.props.id, ...args);
- }
-
render() {
const {
id,
@@ -161,6 +155,8 @@ class Chart extends React.Component {
sliceCanEdit,
} = this.props;
+ // this should never happen but prevents throwing in the case that a gridComponent
+ // references a chart that is not associated with the dashboard
if (!chart || !slice) return null;
const { width } = this.state;
@@ -224,7 +220,6 @@ class Chart extends React.Component {
vizType={slice.viz_type}
addFilter={this.addFilter}
getFilters={this.getFilters}
- removeFilter={this.removeFilter}
annotationData={chart.annotationData}
chartAlert={chart.chartAlert}
chartStatus={chart.chartStatus}
diff --git a/superset/assets/src/dashboard/containers/Chart.jsx b/superset/assets/src/dashboard/containers/Chart.jsx
index 4ad1270..5631a25 100644
--- a/superset/assets/src/dashboard/containers/Chart.jsx
+++ b/superset/assets/src/dashboard/containers/Chart.jsx
@@ -2,8 +2,7 @@ import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import {
- addFilter,
- removeFilter,
+ changeFilter as addFilter,
toggleExpandSlice,
} from '../actions/dashboardState';
import { refreshChart } from '../../chart/chartAction';
@@ -54,7 +53,6 @@ function mapDispatchToProps(dispatch) {
toggleExpandSlice,
addFilter,
refreshChart,
- removeFilter,
},
dispatch,
);
diff --git a/superset/assets/src/dashboard/containers/Dashboard.jsx b/superset/assets/src/dashboard/containers/Dashboard.jsx
index bcf2ace..3252af3 100644
--- a/superset/assets/src/dashboard/containers/Dashboard.jsx
+++ b/superset/assets/src/dashboard/containers/Dashboard.jsx
@@ -1,12 +1,14 @@
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
+import Dashboard from '../components/Dashboard';
+
import {
addSliceToDashboard,
removeSliceFromDashboard,
} from '../actions/dashboardState';
import { runQuery } from '../../chart/chartAction';
-import Dashboard from '../components/Dashboard';
+import getLoadStatsPerTopLevelComponent from '../util/logging/getLoadStatsPerTopLevelComponent';
function mapStateToProps({
datasources,
@@ -28,6 +30,10 @@ function mapStateToProps({
slices: sliceEntities.slices,
layout: dashboardLayout.present,
impressionId,
+ loadStats: getLoadStatsPerTopLevelComponent({
+ layout: dashboardLayout.present,
+ chartQueries: charts,
+ }),
};
}
diff --git a/superset/assets/src/dashboard/containers/SliceAdder.jsx b/superset/assets/src/dashboard/containers/SliceAdder.jsx
new file mode 100644
index 0000000..e3d931d
--- /dev/null
+++ b/superset/assets/src/dashboard/containers/SliceAdder.jsx
@@ -0,0 +1,28 @@
+import { bindActionCreators } from 'redux';
+import { connect } from 'react-redux';
+
+import { fetchAllSlices } from '../actions/sliceEntities';
+import SliceAdder from '../components/SliceAdder';
+
+function mapStateToProps({ sliceEntities, dashboardInfo, dashboardState }) {
+ return {
+ userId: dashboardInfo.userId,
+ selectedSliceIds: dashboardState.sliceIds,
+ slices: sliceEntities.slices,
+ isLoading: sliceEntities.isLoading,
+ errorMessage: sliceEntities.errorMessage,
+ lastUpdated: sliceEntities.lastUpdated,
+ editMode: dashboardState.editMode,
+ };
+}
+
+function mapDispatchToProps(dispatch) {
+ return bindActionCreators(
+ {
+ fetchAllSlices,
+ },
+ dispatch,
+ );
+}
+
+export default connect(mapStateToProps, mapDispatchToProps)(SliceAdder);
diff --git a/superset/assets/src/dashboard/reducers/dashboardState.js b/superset/assets/src/dashboard/reducers/dashboardState.js
index c7f2277..410ecc0 100644
--- a/superset/assets/src/dashboard/reducers/dashboardState.js
+++ b/superset/assets/src/dashboard/reducers/dashboardState.js
@@ -1,11 +1,10 @@
/* eslint-disable camelcase */
import {
ADD_SLICE,
- ADD_FILTER,
+ CHANGE_FILTER,
ON_CHANGE,
ON_SAVE,
REMOVE_SLICE,
- REMOVE_FILTER,
SET_EDIT_MODE,
SET_MAX_UNDO_HISTORY_EXCEEDED,
SET_UNSAVED_CHANGES,
@@ -86,15 +85,14 @@ export default function dashboardStateReducer(state = {}, action) {
};
},
- // filters
- [ADD_FILTER]() {
+ [CHANGE_FILTER]() {
const hasSelectedFilter = state.sliceIds.includes(action.chart.id);
if (!hasSelectedFilter) {
return state;
}
let filters = state.filters;
- const { chart, col, vals, merge, refresh } = action;
+ const { chart, col, vals: nextVals, merge, refresh } = action;
const sliceId = chart.id;
const filterKeys = [
'__from',
@@ -110,33 +108,32 @@ export default function dashboardStateReducer(state = {}, action) {
) {
let newFilter = {};
if (!(sliceId in filters)) {
- // Straight up set the filters if none existed for the slice
- newFilter = { [col]: vals };
+ // if no filters existed for the slice, set them
+ newFilter = { [col]: nextVals };
} else if ((filters[sliceId] && !(col in filters[sliceId])) || !merge) {
- newFilter = { ...filters[sliceId], [col]: vals };
- // d3.merge pass in array of arrays while some value form filter components
- // from and to filter box require string to be process and return
+ // If no filters exist for this column, or we are overwriting them
+ newFilter = { ...filters[sliceId], [col]: nextVals };
} else if (filters[sliceId][col] instanceof Array) {
- newFilter[col] = [...filters[sliceId][col], ...vals];
+ newFilter[col] = [...filters[sliceId][col], ...nextVals];
} else {
- newFilter[col] = [filters[sliceId][col], ...vals];
+ newFilter[col] = [filters[sliceId][col], ...nextVals];
}
filters = { ...filters, [sliceId]: newFilter };
- }
- return { ...state, filters, refresh };
- },
- [REMOVE_FILTER]() {
- const { sliceId, col, vals, refresh } = action;
- const excluded = new Set(vals);
- let filters = state.filters;
- // Have to be careful not to modify the dashboard state so that
- // the render actually triggers
- if (sliceId in state.filters && col in state.filters[sliceId]) {
- const newFilter = filters[sliceId][col].filter(
- val => !excluded.has(val),
- );
- filters = { ...filters, [sliceId]: { [col]: newFilter } };
+ // remove any empty filters so they don't pollute the logs
+ Object.keys(filters).forEach(chartId => {
+ Object.keys(filters[chartId]).forEach(column => {
+ if (
+ !filters[chartId][column] ||
+ filters[chartId][column].length === 0
+ ) {
+ delete filters[chartId][column];
+ }
+ });
+ if (Object.keys(filters[chartId]).length === 0) {
+ delete filters[chartId];
+ }
+ });
}
return { ...state, filters, refresh };
},
diff --git a/superset/assets/src/dashboard/util/logging/childChartsDidLoad.js b/superset/assets/src/dashboard/util/logging/childChartsDidLoad.js
new file mode 100644
index 0000000..58b81f9
--- /dev/null
+++ b/superset/assets/src/dashboard/util/logging/childChartsDidLoad.js
@@ -0,0 +1,21 @@
+import findNonTabChildCharIds from './findNonTabChildChartIds';
+
+export default function childChartsDidLoad({ chartQueries, layout, id }) {
+ const chartIds = findNonTabChildCharIds({ id, layout });
+
+ let minQueryStartTime = Infinity;
+ const didLoad = chartIds.every(chartId => {
+ const query = chartQueries[chartId] || {};
+
+ // filterbox's don't re-render, don't use stale update time
+ if (query.formData && query.formData.viz_type !== 'filter_box') {
+ minQueryStartTime = Math.min(
+ query.chartUpdateStartTime,
+ minQueryStartTime,
+ );
+ }
+ return ['stopped', 'failed', 'rendered'].indexOf(query.chartStatus) > -1;
+ });
+
+ return { didLoad, minQueryStartTime };
+}
diff --git a/superset/assets/src/dashboard/util/logging/findNonTabChildChartIds.js b/superset/assets/src/dashboard/util/logging/findNonTabChildChartIds.js
new file mode 100644
index 0000000..a9a51f7
--- /dev/null
+++ b/superset/assets/src/dashboard/util/logging/findNonTabChildChartIds.js
@@ -0,0 +1,45 @@
+import { TABS_TYPE, CHART_TYPE } from '../componentTypes';
+
+// This function traverses the layout from the passed id, returning an array
+// of any child chartIds NOT nested within a Tabs component. These helps us identify
+// if the charts at a given "Tabs" level are loaded
+function findNonTabChildChartIds({ id, layout }) {
+ const chartIds = [];
+ function recurseFromNode(node) {
+ if (node && node.type === CHART_TYPE) {
+ if (node.meta && node.meta.chartId) {
+ chartIds.push(node.meta.chartId);
+ }
+ } else if (
+ node &&
+ node.type !== TABS_TYPE &&
+ node.children &&
+ node.children.length
+ ) {
+ node.children.forEach(childId => {
+ const child = layout[childId];
+ if (child) {
+ recurseFromNode(child);
+ }
+ });
+ }
+ }
+
+ recurseFromNode(layout[id]);
+
+ return chartIds;
+}
+
+// This method is called frequently, so cache results
+let cachedLayout;
+let cachedIdsLookup = {};
+export default function findNonTabChildChartIdsWithCache({ id, layout }) {
+ if (cachedLayout === layout && cachedIdsLookup[id]) {
+ return cachedIdsLookup[id];
+ } else if (layout !== cachedLayout) {
+ cachedLayout = layout;
+ cachedIdsLookup = {};
+ }
+ cachedIdsLookup[id] = findNonTabChildChartIds({ layout, id });
+ return cachedIdsLookup[id];
+}
diff --git a/superset/assets/src/dashboard/util/logging/findTopLevelComponentIds.js b/superset/assets/src/dashboard/util/logging/findTopLevelComponentIds.js
new file mode 100644
index 0000000..d274d7c
--- /dev/null
+++ b/superset/assets/src/dashboard/util/logging/findTopLevelComponentIds.js
@@ -0,0 +1,74 @@
+import { TAB_TYPE, DASHBOARD_GRID_TYPE } from '../componentTypes';
+import { DASHBOARD_ROOT_ID } from '../constants';
+import findNonTabChildChartIds from './findNonTabChildChartIds';
+
+// This function traverses the layout to identify top grid + tab level components
+// for which we track load times
+function findTopLevelComponentIds(layout) {
+ const topLevelNodes = [];
+
+ function recurseFromNode({
+ node,
+ index = null,
+ depth,
+ parentType = null,
+ parentId = null,
+ }) {
+ if (!node) return;
+
+ let nextParentType = parentType;
+ let nextParentId = parentId;
+ let nextDepth = depth;
+ if (node.type === TAB_TYPE || node.type === DASHBOARD_GRID_TYPE) {
+ const chartIds = findNonTabChildChartIds({
+ layout,
+ id: node.id,
+ });
+
+ topLevelNodes.push({
+ id: node.id,
+ type: node.type,
+ parent_type: parentType,
+ parent_id: parentId,
+ index,
+ depth,
+ slice_ids: chartIds,
+ });
+
+ nextParentId = node.id;
+ nextParentType = node.type;
+ nextDepth += 1;
+ }
+ if (node.children && node.children.length) {
+ node.children.forEach((childId, childIndex) => {
+ recurseFromNode({
+ node: layout[childId],
+ index: childIndex,
+ parentType: nextParentType,
+ parentId: nextParentId,
+ depth: nextDepth,
+ });
+ });
+ }
+ }
+
+ recurseFromNode({
+ node: layout[DASHBOARD_ROOT_ID],
+ depth: 0,
+ });
+
+ return topLevelNodes;
+}
+
+// This method is called frequently, so cache results
+let cachedLayout;
+let cachedTopLevelNodes;
+export default function findTopLevelComponentIdsWithCache(layout) {
+ if (layout === cachedLayout) {
+ return cachedTopLevelNodes;
+ }
+ cachedLayout = layout;
+ cachedTopLevelNodes = findTopLevelComponentIds(layout);
+
+ return cachedTopLevelNodes;
+}
diff --git a/superset/assets/src/dashboard/util/logging/getLoadStatsPerTopLevelComponent.js b/superset/assets/src/dashboard/util/logging/getLoadStatsPerTopLevelComponent.js
new file mode 100644
index 0000000..b503746
--- /dev/null
+++ b/superset/assets/src/dashboard/util/logging/getLoadStatsPerTopLevelComponent.js
@@ -0,0 +1,26 @@
+import findTopLevelComponentIds from './findTopLevelComponentIds';
+import childChartsDidLoad from './childChartsDidLoad';
+
+export default function getLoadStatsPerTopLevelComponent({
+ layout,
+ chartQueries,
+}) {
+ const topLevelComponents = findTopLevelComponentIds(layout);
+ const stats = {};
+ topLevelComponents.forEach(({ id, ...restStats }) => {
+ const { didLoad, minQueryStartTime } = childChartsDidLoad({
+ id,
+ layout,
+ chartQueries,
+ });
+
+ stats[id] = {
+ didLoad,
+ id,
+ minQueryStartTime,
+ ...restStats,
+ };
+ });
+
+ return stats;
+}
diff --git a/superset/assets/src/dashboard/util/propShapes.jsx b/superset/assets/src/dashboard/util/propShapes.jsx
index f07497c..1242d2b 100644
--- a/superset/assets/src/dashboard/util/propShapes.jsx
+++ b/superset/assets/src/dashboard/util/propShapes.jsx
@@ -84,3 +84,16 @@ export const dashboardInfoPropShape = PropTypes.shape({
common: PropTypes.object,
userId: PropTypes.string.isRequired,
});
+
+export const loadStatsPropShape = PropTypes.objectOf(
+ PropTypes.shape({
+ didLoad: PropTypes.bool.isRequired,
+ minQueryStartTime: PropTypes.number.isRequired,
+ id: PropTypes.string.isRequired,
+ type: PropTypes.string.isRequired,
+ parent_id: PropTypes.string,
+ parent_type: PropTypes.string,
+ index: PropTypes.number.isRequired,
+ slice_ids: PropTypes.arrayOf(PropTypes.number).isRequired,
+ }),
+);
diff --git a/superset/assets/src/explore/components/ExploreViewContainer.jsx b/superset/assets/src/explore/components/ExploreViewContainer.jsx
index a648464..3eada5c 100644
--- a/superset/assets/src/explore/components/ExploreViewContainer.jsx
+++ b/superset/assets/src/explore/components/ExploreViewContainer.jsx
@@ -15,8 +15,7 @@ import { chartPropShape } from '../../dashboard/util/propShapes';
import * as exploreActions from '../actions/exploreActions';
import * as saveModalActions from '../actions/saveModalActions';
import * as chartActions from '../../chart/chartAction';
-import { Logger, ActionLog, LOG_ACTIONS_PAGE_LOAD,
- LOG_ACTIONS_LOAD_EVENT, LOG_ACTIONS_RENDER_EVENT } from '../../logger';
+import { Logger, ActionLog, EXPLORE_EVENT_NAMES, LOG_ACTIONS_MOUNT_EXPLORER } from '../../logger';
const propTypes = {
actions: PropTypes.object.isRequired,
@@ -35,13 +34,11 @@ const propTypes = {
class ExploreViewContainer extends React.Component {
constructor(props) {
super(props);
- this.firstLoad = true;
this.loadingLog = new ActionLog({
impressionId: props.impressionId,
- actionType: LOG_ACTIONS_PAGE_LOAD,
source: 'slice',
sourceId: props.slice ? props.slice.slice_id : 0,
- eventNames: [LOG_ACTIONS_LOAD_EVENT, LOG_ACTIONS_RENDER_EVENT],
+ eventNames: EXPLORE_EVENT_NAMES,
});
Logger.start(this.loadingLog);
@@ -62,34 +59,37 @@ class ExploreViewContainer extends React.Component {
window.addEventListener('resize', this.handleResize);
window.addEventListener('popstate', this.handlePopstate);
this.addHistory({ isReplace: true });
+ Logger.append(LOG_ACTIONS_MOUNT_EXPLORER);
}
- componentWillReceiveProps(np) {
- if (this.firstLoad &&
- ['rendered', 'failed', 'stopped'].indexOf(np.chart.chartStatus) > -1) {
- Logger.end(this.loadingLog);
- this.firstLoad = false;
+ componentWillReceiveProps(nextProps) {
+ const wasRendered =
+ ['rendered', 'failed', 'stopped'].indexOf(this.props.chart.chartStatus) > -1;
+ const isRendered = ['rendered', 'failed', 'stopped'].indexOf(nextProps.chart.chartStatus) > -1;
+ if (!wasRendered && isRendered) {
+ Logger.send(this.loadingLog);
}
- if (np.controls.viz_type.value !== this.props.controls.viz_type.value) {
+ if (nextProps.controls.viz_type.value !== this.props.controls.viz_type.value) {
this.props.actions.resetControls();
this.props.actions.triggerQuery(true, this.props.chart.id);
}
if (
- np.controls.datasource && (
- this.props.controls.datasource == null ||
- np.controls.datasource.value !== this.props.controls.datasource.value
- )
+ nextProps.controls.datasource &&
+ (this.props.controls.datasource == null ||
+ nextProps.controls.datasource.value !== this.props.controls.datasource.value)
) {
- this.props.actions.fetchDatasourceMetadata(np.form_data.datasource, true);
+ this.props.actions.fetchDatasourceMetadata(nextProps.form_data.datasource, true);
}
- const changedControlKeys = this.findChangedControlKeys(this.props.controls, np.controls);
- if (this.hasDisplayControlChanged(changedControlKeys, np.controls)) {
+ const changedControlKeys = this.findChangedControlKeys(this.props.controls, nextProps.controls);
+ if (this.hasDisplayControlChanged(changedControlKeys, nextProps.controls)) {
this.props.actions.updateQueryFormData(
- getFormDataFromControls(np.controls), this.props.chart.id);
+ getFormDataFromControls(nextProps.controls),
+ this.props.chart.id,
+ );
this.props.actions.renderTriggered(new Date().getTime(), this.props.chart.id);
}
- if (this.hasQueryControlChanged(changedControlKeys, np.controls)) {
+ if (this.hasQueryControlChanged(changedControlKeys, nextProps.controls)) {
this.setState({ chartIsStale: true, refreshOverlayVisible: true });
}
}
@@ -139,26 +139,31 @@ class ExploreViewContainer extends React.Component {
}
findChangedControlKeys(prevControls, currentControls) {
- return Object.keys(currentControls).filter(key => (
- typeof prevControls[key] !== 'undefined' &&
- !areObjectsEqual(currentControls[key].value, prevControls[key].value)
- ));
+ return Object.keys(currentControls).filter(
+ key =>
+ typeof prevControls[key] !== 'undefined' &&
+ !areObjectsEqual(currentControls[key].value, prevControls[key].value),
+ );
}
hasDisplayControlChanged(changedControlKeys, currentControls) {
- return changedControlKeys.some(key => (currentControls[key].renderTrigger));
+ return changedControlKeys.some(key => currentControls[key].renderTrigger);
}
hasQueryControlChanged(changedControlKeys, currentControls) {
- return changedControlKeys.some(key => (
- !currentControls[key].renderTrigger && !currentControls[key].dontRefreshOnChange
- ));
+ return changedControlKeys.some(
+ key => !currentControls[key].renderTrigger && !currentControls[key].dontRefreshOnChange,
+ );
}
triggerQueryIfNeeded() {
if (this.props.chart.triggerQuery && !this.hasErrors()) {
- this.props.actions.runQuery(this.props.form_data, false,
- this.props.timeout, this.props.chart.id);
+ this.props.actions.runQuery(
+ this.props.form_data,
+ false,
+ this.props.timeout,
+ this.props.chart.id,
+ );
}
}
@@ -166,15 +171,9 @@ class ExploreViewContainer extends React.Component {
const { payload } = getExploreUrlAndPayload({ formData: this.props.form_data });
const longUrl = getExploreLongUrl(this.props.form_data);
if (isReplace) {
- history.replaceState(
- payload,
- title,
- longUrl);
+ history.replaceState(payload, title, longUrl);
} else {
- history.pushState(
- payload,
- title,
- longUrl);
+ history.pushState(payload, title, longUrl);
}
// it seems some browsers don't support pushState title attribute
@@ -194,12 +193,7 @@ class ExploreViewContainer extends React.Component {
const formData = history.state;
if (formData && Object.keys(formData).length) {
this.props.actions.setExploreControls(formData);
- this.props.actions.runQuery(
- formData,
- false,
- this.props.timeout,
- this.props.chart.id,
- );
+ this.props.actions.runQuery(formData, false, this.props.timeout, this.props.chart.id);
}
}
@@ -209,7 +203,8 @@ class ExploreViewContainer extends React.Component {
hasErrors() {
const ctrls = this.props.controls;
return Object.keys(ctrls).some(
- k => ctrls[k].validationErrors && ctrls[k].validationErrors.length > 0);
+ k => ctrls[k].validationErrors && ctrls[k].validationErrors.length > 0,
+ );
}
renderErrorMessage() {
// Returns an error message as a node if any errors are in the store
@@ -227,9 +222,7 @@ class ExploreViewContainer extends React.Component {
}
let errorMessage;
if (errors.length > 0) {
- errorMessage = (
- <div style={{ textAlign: 'left' }}>{errors}</div>
- );
+ errorMessage = <div style={{ textAlign: 'left' }}>{errors}</div>;
}
return errorMessage;
}
@@ -244,7 +237,8 @@ class ExploreViewContainer extends React.Component {
addHistory={this.addHistory}
onQuery={this.onQuery.bind(this)}
onDismissRefreshOverlay={this.onDismissRefreshOverlay.bind(this)}
- />);
+ />
+ );
}
render() {
@@ -260,13 +254,13 @@ class ExploreViewContainer extends React.Component {
overflow: 'hidden',
}}
>
- {this.state.showModal &&
- <SaveModal
- onHide={this.toggleModal.bind(this)}
- actions={this.props.actions}
- form_data={this.props.form_data}
- />
- }
+ {this.state.showModal && (
+ <SaveModal
+ onHide={this.toggleModal.bind(this)}
+ actions={this.props.actions}
+ form_data={this.props.form_data}
+ />
+ )}
<div className="row">
<div className="col-sm-4">
<QueryAndSaveBtns
@@ -287,9 +281,7 @@ class ExploreViewContainer extends React.Component {
isDatasourceMetaLoading={this.props.isDatasourceMetaLoading}
/>
</div>
- <div className="col-sm-8">
- {this.renderChartContainer()}
- </div>
+ <div className="col-sm-8">{this.renderChartContainer()}</div>
</div>
</div>
);
@@ -301,12 +293,11 @@ ExploreViewContainer.propTypes = propTypes;
function mapStateToProps({ explore, charts, impressionId }) {
const form_data = getFormDataFromControls(explore.controls);
// fill in additional params stored in form_data but not used by control
- Object.keys(explore.rawFormData)
- .forEach((key) => {
- if (form_data[key] === undefined) {
- form_data[key] = explore.rawFormData[key];
- }
- });
+ Object.keys(explore.rawFormData).forEach((key) => {
+ if (form_data[key] === undefined) {
+ form_data[key] = explore.rawFormData[key];
+ }
+ });
const chartKey = Object.keys(charts)[0];
const chart = charts[chartKey];
return {
diff --git a/superset/assets/src/logger.js b/superset/assets/src/logger.js
index 65c81b5..06059b2 100644
--- a/superset/assets/src/logger.js
+++ b/superset/assets/src/logger.js
@@ -1,64 +1,61 @@
import $ from 'jquery';
-export const LOG_ACTIONS_PAGE_LOAD = 'page_load_perf';
-export const LOG_ACTIONS_LOAD_EVENT = 'load_events';
-export const LOG_ACTIONS_RENDER_EVENT = 'render_events';
-
-const handlers = {};
+// This creates an association between an eventName and the ActionLog instance so that
+// Logger.append calls do not have to know about the appropriate ActionLog instance
+const addEventHandlers = {};
export const Logger = {
start(log) {
- log.setAttribute('startAt', new Date().getTime() - this.getTimestamp());
+ // create a handler to handle adding each event type
log.eventNames.forEach((eventName) => {
- if (!handlers[eventName]) {
- handlers[eventName] = [];
+ if (!addEventHandlers[eventName]) {
+ addEventHandlers[eventName] = log.addEvent.bind(log);
+ } else {
+ console.warn(`Duplicate event handler for event '${eventName}'`);
}
- handlers[eventName].push(log.addEvent.bind(log));
});
},
- append(eventName, eventBody) {
- return (
- (handlers[eventName] || {}).length &&
- handlers[eventName].forEach(handler => handler(eventName, eventBody))
- );
+ append(eventName, eventBody, sendNow) {
+ if (addEventHandlers[eventName]) {
+ addEventHandlers[eventName](eventName, eventBody, sendNow);
+ } else {
+ console.warn(`No event handler for event '${eventName}'`);
+ }
},
end(log) {
- log.setAttribute('duration', new Date().getTime() - log.startAt);
this.send(log);
+ // remove handlers
log.eventNames.forEach((eventName) => {
- if (handlers[eventName].length) {
- const index = handlers[eventName].findIndex(handler => handler === log.addEvent);
- handlers[eventName].splice(index, 1);
+ if (addEventHandlers[eventName]) {
+ delete addEventHandlers[eventName];
}
});
},
send(log) {
- const { impressionId, actionType, source, sourceId, events, startAt, duration } = log;
- const requestPrams = [];
- requestPrams.push(['impression_id', impressionId]);
- switch (source) {
- case 'dashboard':
- requestPrams.push(['dashboard_id', sourceId]);
- break;
- case 'slice':
- requestPrams.push(['slice_id', sourceId]);
- break;
- default:
- break;
- }
+ const { impressionId, source, sourceId, events } = log;
let url = '/superset/log/';
- if (requestPrams.length) {
- url += '?' + requestPrams.map(([k, v]) => k + '=' + v).join('&');
+
+ // backend logs treat these request params as first-class citizens
+ if (source === 'dashboard') {
+ url += `?dashboard_id=${sourceId}`;
+ } else if (source === 'slice') {
+ url += `?slice_id=${sourceId}`;
}
- const eventData = {};
+
+ const eventData = [];
for (const eventName in events) {
- eventData[eventName] = [];
events[eventName].forEach((event) => {
- eventData[eventName].push(event);
+ eventData.push({
+ source,
+ source_id: sourceId,
+ event_name: eventName,
+ impression_id: impressionId,
+ ...event,
+ });
});
}
@@ -67,30 +64,28 @@ export const Logger = {
method: 'POST',
dataType: 'json',
data: {
- source: 'client',
- type: actionType,
- started_time: startAt,
- duration,
+ explode: 'events',
events: JSON.stringify(eventData),
},
});
+
+ // flush events for this logger
+ log.events = {}; // eslint-disable-line no-param-reassign
},
+ // note that this returns ms since page load, NOT ms since epoc
getTimestamp() {
return Math.round(window.performance.now());
},
};
export class ActionLog {
- constructor({ impressionId, actionType, source, sourceId, eventNames, sendNow }) {
+ constructor({ impressionId, source, sourceId, sendNow, eventNames }) {
this.impressionId = impressionId;
this.source = source;
this.sourceId = sourceId;
- this.actionType = actionType;
this.eventNames = eventNames;
this.sendNow = sendNow || false;
- this.startAt = 0;
- this.duration = 0;
this.events = {};
this.addEvent = this.addEvent.bind(this);
@@ -100,16 +95,68 @@ export class ActionLog {
this[name] = value;
}
- addEvent(eventName, eventBody) {
- if (!this.events[eventName]) {
- this.events[eventName] = [];
- }
- this.events[eventName].push(eventBody);
+ addEvent(eventName, eventBody, sendNow) {
+ if (sendNow) {
+ Logger.send({
+ ...this,
+ // overwrite events so that Logger.send doesn't clear this.events
+ events: {
+ [eventName]: [
+ {
+ ts: new Date().getTime(),
+ start_offset: Logger.getTimestamp(),
+ ...eventBody,
+ },
+ ],
+ },
+ });
+ } else {
+ this.events[eventName] = this.events[eventName] || [];
- if (this.sendNow) {
- this.setAttribute('duration', new Date().getTime() - this.startAt);
- Logger.send(this);
- this.events = {};
+ this.events[eventName].push({
+ ts: new Date().getTime(),
+ start_offset: Logger.getTimestamp(),
+ ...eventBody,
+ });
+
+ if (this.sendNow) {
+ Logger.send(this);
+ }
}
}
}
+
+// Log event types ------------------------------------------------------------
+export const LOG_ACTIONS_MOUNT_DASHBOARD = 'mount_dashboard';
+export const LOG_ACTIONS_MOUNT_EXPLORER = 'mount_explorer';
+
+export const LOG_ACTIONS_FIRST_DASHBOARD_LOAD = 'first_dashboard_load';
+export const LOG_ACTIONS_LOAD_DASHBOARD_PANE = 'load_dashboard_pane';
+export const LOG_ACTIONS_LOAD_CHART = 'load_chart_data';
+export const LOG_ACTIONS_RENDER_CHART = 'render_chart';
+export const LOG_ACTIONS_REFRESH_CHART = 'force_refresh_chart';
+
+export const LOG_ACTIONS_REFRESH_DASHBOARD = 'force_refresh_dashboard';
+export const LOG_ACTIONS_EXPLORE_DASHBOARD_CHART = 'explore_dashboard_chart';
+export const LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART = 'export_csv_dashboard_chart';
+export const LOG_ACTIONS_CHANGE_DASHBOARD_FILTER = 'change_dashboard_filter';
+
+export const DASHBOARD_EVENT_NAMES = [
+ LOG_ACTIONS_MOUNT_DASHBOARD,
+ LOG_ACTIONS_FIRST_DASHBOARD_LOAD,
+ LOG_ACTIONS_LOAD_DASHBOARD_PANE,
+ LOG_ACTIONS_LOAD_CHART,
+ LOG_ACTIONS_RENDER_CHART,
+ LOG_ACTIONS_EXPLORE_DASHBOARD_CHART,
+ LOG_ACTIONS_REFRESH_CHART,
+ LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART,
+ LOG_ACTIONS_CHANGE_DASHBOARD_FILTER,
+ LOG_ACTIONS_REFRESH_DASHBOARD,
+];
+
+export const EXPLORE_EVENT_NAMES = [
+ LOG_ACTIONS_MOUNT_EXPLORER,
+ LOG_ACTIONS_LOAD_CHART,
+ LOG_ACTIONS_RENDER_CHART,
+ LOG_ACTIONS_REFRESH_CHART,
+];
diff --git a/superset/models/core.py b/superset/models/core.py
index 0ef3331..3b663af 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -883,16 +883,18 @@ class Log(Model):
"""Decorator to log user actions"""
@functools.wraps(f)
def wrapper(*args, **kwargs):
- start_dttm = datetime.now()
user_id = None
if g.user:
user_id = g.user.get_id()
d = request.form.to_dict() or {}
+
# request parameters can overwrite post body
request_params = request.args.to_dict()
d.update(request_params)
d.update(kwargs)
+
slice_id = d.get('slice_id')
+ dashboard_id = d.get('dashboard_id')
try:
slice_id = int(
@@ -900,26 +902,40 @@ class Log(Model):
except (ValueError, TypeError):
slice_id = 0
- params = ''
- try:
- params = json.dumps(d)
- except Exception:
- pass
stats_logger.incr(f.__name__)
+ start_dttm = datetime.now()
value = f(*args, **kwargs)
+ duration_ms = (datetime.now() - start_dttm).total_seconds() * 1000
+
+ # bulk insert
+ try:
+ explode_by = d.get('explode')
+ records = json.loads(d.get(explode_by))
+ except Exception:
+ records = [d]
+
+ referrer = request.referrer[:1000] if request.referrer else None
+ logs = []
+ for record in records:
+ try:
+ json_string = json.dumps(record)
+ except Exception:
+ json_string = None
+ log = cls(
+ action=f.__name__,
+ json=json_string,
+ dashboard_id=dashboard_id,
+ slice_id=slice_id,
+ duration_ms=duration_ms,
+ referrer=referrer,
+ user_id=user_id)
+ logs.append(log)
+
sesh = db.session()
- log = cls(
- action=f.__name__,
- json=params,
- dashboard_id=d.get('dashboard_id'),
- slice_id=slice_id,
- duration_ms=(
- datetime.now() - start_dttm).total_seconds() * 1000,
- referrer=request.referrer[:1000] if request.referrer else None,
- user_id=user_id)
- sesh.add(log)
+ sesh.bulk_save_objects(logs)
sesh.commit()
return value
+
return wrapper