superset-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] williaster closed pull request #5087: [dashboard v2] logging updates
Date Fri, 01 Jun 2018 02:02:13 GMT
williaster closed pull request #5087: [dashboard v2] logging updates
URL: https://github.com/apache/incubator-superset/pull/5087
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/package.json b/superset/assets/package.json
index fc6f7d080b..e880a9cc2a 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 545b890dea..209404092f 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 05756f467b..5fff31325d 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 078019dfa5..89c4ffea0f 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 0000000000..64580b4bdb
--- /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 4a471e8962..060249fbba 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 6ecb01fdad..351303cac9 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) {
@@ -70,11 +70,13 @@ export function runAnnotationQuery(annotation, timeout = 60, formData = null, ke
       return Promise.resolve();
     }
 
-    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({
@@ -139,19 +141,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,
@@ -193,7 +198,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 aac4f98b84..688f5b02fa 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 644ddf0c62..76f4b54add 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 0f42f1b5f0..30e2e78776 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 6729e57680..7d25ab08e6 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 1ace51df01..39c1e81dcb 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 4ad1270b7f..5631a25d1f 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 bcf2ace219..3252af3c34 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 0000000000..e3d931dc51
--- /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 c7f2277b34..410ecc0776 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 0000000000..58b81f92a1
--- /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 0000000000..a9a51f7a8a
--- /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 0000000000..d274d7cc49
--- /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 0000000000..b503746e11
--- /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 f07497c56f..1242d2bde5 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 a648464638..3eada5ce28 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 65c81b50a7..06059b28b1 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 74582723e4..d3e374b5e5 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -880,16 +880,18 @@ def log_this(cls, f):
         """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(
@@ -897,26 +899,40 @@ def wrapper(*args, **kwargs):
             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
 
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Mime
View raw message