From d5ac05caf8c6a22937b43a429a25464fd6f8f0d0 Mon Sep 17 00:00:00 2001 From: Nikhil Nandagopal Date: Thu, 24 Sep 2020 21:35:18 +0530 Subject: [PATCH] Feature/instrumentation (#708) * bumped sentry version modified performance monitor to use a queue internally to make synchronous logging easy added logging for api pane close / open and various different methods * added use effect to stop tracking on mount of entity properties * removed open api tracking * fixed stop tracking to pop from the end instead of the beginning added tracking for editor mount and sidebar mount * added tracking for entity explorer * moved from app route to sentry app route because passing JSX to app route causes re-renders * Fixing theme and route change issues. Co-authored-by: Nikhil Nandagopal Co-authored-by: Satbir Singh --- app/client/src/AppRouter.tsx | 86 +++++++++++-------- .../components/editorComponents/Sidebar.tsx | 9 +- app/client/src/pages/AppViewer/index.tsx | 8 +- .../src/pages/Editor/Explorer/index.tsx | 10 ++- app/client/src/pages/Editor/MainContainer.tsx | 15 ++-- app/client/src/pages/Editor/WidgetsEditor.tsx | 6 +- app/client/src/pages/Editor/routes.tsx | 42 ++++----- app/client/src/pages/UserAuth/index.tsx | 28 ++---- app/client/src/pages/common/AppRoute.tsx | 33 +++---- app/client/src/pages/organization/index.tsx | 14 ++- .../src/pages/organization/settings.tsx | 17 ++-- app/client/src/utils/DynamicBindingUtils.ts | 4 + app/client/src/utils/PerformanceTracker.ts | 46 ++++++---- 13 files changed, 163 insertions(+), 155 deletions(-) diff --git a/app/client/src/AppRouter.tsx b/app/client/src/AppRouter.tsx index d31948a59a..d539b60e35 100644 --- a/app/client/src/AppRouter.tsx +++ b/app/client/src/AppRouter.tsx @@ -1,8 +1,7 @@ import React, { Suspense } from "react"; import history from "utils/history"; import AppHeader from "pages/common/AppHeader"; -import { Redirect, Router, Switch } from "react-router-dom"; -import AppRoute from "pages/common/AppRoute"; +import { Redirect, Route, Router, Switch } from "react-router-dom"; import { APP_VIEW_URL, APPLICATIONS_URL, @@ -28,70 +27,81 @@ import Users from "pages/users"; import PageNotFound from "pages/common/PageNotFound"; import PageLoadingBar from "pages/common/PageLoadingBar"; import ServerUnavailable from "pages/common/ServerUnavailable"; +import { getThemeDetails } from "selectors/themeSelectors"; +import { ThemeMode } from "reducers/uiReducers/themeReducer"; +import { AppState } from "reducers"; +import { setThemeMode } from "actions/themeActions"; +import { connect } from "react-redux"; + +import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); const loadingIndicator = ; +function changeAppBackground(currentTheme: any) { + if ( + window.location.pathname === "/applications" || + window.location.pathname.indexOf("/settings/") !== -1 + ) { + document.body.style.backgroundColor = + currentTheme.colors.homepageBackground; + } else { + document.body.style.backgroundColor = currentTheme.colors.appBackground; + } +} + class AppRouter extends React.Component { render() { + const { currentTheme } = this.props; + // This is needed for the theme switch. + changeAppBackground(currentTheme); + // This is needed for the route switch. + history.listen(() => { + changeAppBackground(currentTheme); + }); + return ( - + - - - - + + + - - - + + - - + ); } } +const mapStateToProps = (state: AppState) => ({ + currentTheme: getThemeDetails(state).theme, +}); +const mapDispatchToProps = (dispatch: any) => ({ + setTheme: (mode: ThemeMode) => { + dispatch(setThemeMode(mode)); + }, +}); -export default AppRouter; +export default connect(mapStateToProps, mapDispatchToProps)(AppRouter); diff --git a/app/client/src/components/editorComponents/Sidebar.tsx b/app/client/src/components/editorComponents/Sidebar.tsx index 25692ae48a..ad8e3a9322 100644 --- a/app/client/src/components/editorComponents/Sidebar.tsx +++ b/app/client/src/components/editorComponents/Sidebar.tsx @@ -1,9 +1,12 @@ -import React, { memo } from "react"; +import React, { memo, useEffect } from "react"; import styled from "styled-components"; import ExplorerSidebar from "pages/Editor/Explorer"; import { PanelStack, Classes } from "@blueprintjs/core"; import { Colors } from "constants/Colors"; import * as Sentry from "@sentry/react"; +import PerformanceTracker, { + PerformanceTransactionName, +} from "utils/PerformanceTracker"; const SidebarWrapper = styled.div` background-color: ${Colors.MINE_SHAFT}; @@ -24,6 +27,10 @@ const SidebarWrapper = styled.div` const initialPanel = { component: ExplorerSidebar }; export const Sidebar = memo(() => { + PerformanceTracker.startTracking(PerformanceTransactionName.SIDE_BAR_MOUNT); + useEffect(() => { + PerformanceTracker.stopTracking(); + }); return ( diff --git a/app/client/src/pages/AppViewer/index.tsx b/app/client/src/pages/AppViewer/index.tsx index 5134c091bd..1e27718a2a 100644 --- a/app/client/src/pages/AppViewer/index.tsx +++ b/app/client/src/pages/AppViewer/index.tsx @@ -1,7 +1,7 @@ import React, { Component } from "react"; import styled from "styled-components"; import { connect } from "react-redux"; -import { withRouter, RouteComponentProps } from "react-router"; +import { withRouter, RouteComponentProps, Route } from "react-router"; import { Switch } from "react-router-dom"; import { AppState } from "reducers"; import { @@ -21,9 +21,9 @@ import { resetChildrenMetaProperty, updateWidgetMetaProperty, } from "actions/metaActions"; -import AppRoute from "pages/common/AppRoute"; import { editorInitializer } from "utils/EditorUtils"; import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); const AppViewerBody = styled.section` display: flex; @@ -84,12 +84,10 @@ class AppViewer extends Component< {isInitialized && this.state.registered && ( - )} diff --git a/app/client/src/pages/Editor/Explorer/index.tsx b/app/client/src/pages/Editor/Explorer/index.tsx index 38299d667e..2c24b53bb7 100644 --- a/app/client/src/pages/Editor/Explorer/index.tsx +++ b/app/client/src/pages/Editor/Explorer/index.tsx @@ -1,4 +1,4 @@ -import React, { useRef, MutableRefObject, useCallback } from "react"; +import React, { useRef, MutableRefObject, useCallback, useEffect } from "react"; import styled from "styled-components"; import Divider from "components/editorComponents/Divider"; import { @@ -18,6 +18,9 @@ import history from "utils/history"; import { useParams } from "react-router"; import { ExplorerURLParams } from "./helpers"; import JSDependencies from "./JSDependencies"; +import PerformanceTracker, { + PerformanceTransactionName, +} from "utils/PerformanceTracker"; const Wrapper = styled.div` height: 100%; @@ -40,7 +43,10 @@ const EntityExplorer = (props: IPanelProps) => { const searchInputRef: MutableRefObject = useRef( null, ); - + PerformanceTracker.startTracking(PerformanceTransactionName.ENTITY_EXPLORER); + useEffect(() => { + PerformanceTracker.stopTracking(); + }); const explorerRef = useRef(null); const { searchKeyword, clearSearch } = useFilteredEntities(searchInputRef); const datasources = useFilteredDatasources(searchKeyword); diff --git a/app/client/src/pages/Editor/MainContainer.tsx b/app/client/src/pages/Editor/MainContainer.tsx index 93e09a0429..890fc72248 100644 --- a/app/client/src/pages/Editor/MainContainer.tsx +++ b/app/client/src/pages/Editor/MainContainer.tsx @@ -3,10 +3,12 @@ import EditorsRouter from "./routes"; import WidgetsEditor from "./WidgetsEditor"; import styled from "styled-components"; import Sidebar from "components/editorComponents/Sidebar"; -import { Switch } from "react-router"; -import AppRoute from "../common/AppRoute"; +import { Route, Switch } from "react-router"; import { BUILDER_URL } from "constants/routes"; +import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); + const Container = styled.div` display: flex; height: calc(100vh - ${props => props.theme.headerHeight}); @@ -23,13 +25,8 @@ const MainContainer = () => { - - + + diff --git a/app/client/src/pages/Editor/WidgetsEditor.tsx b/app/client/src/pages/Editor/WidgetsEditor.tsx index 9b73236392..abd8bb85e5 100644 --- a/app/client/src/pages/Editor/WidgetsEditor.tsx +++ b/app/client/src/pages/Editor/WidgetsEditor.tsx @@ -49,9 +49,7 @@ const CanvasContainer = styled.section` /* eslint-disable react/display-name */ const WidgetsEditor = () => { - PerformanceTracker.startTracking( - PerformanceTransactionName.GENERATE_WIDGET_EDITOR_PROPS, - ); + PerformanceTracker.startTracking(PerformanceTransactionName.EDITOR_MOUNT); const { focusWidget, selectWidget } = useWidgetSelection(); const params = useParams<{ applicationId: string; pageId: string }>(); const dispatch = useDispatch(); @@ -62,6 +60,7 @@ const WidgetsEditor = () => { const currentPageName = useSelector(getCurrentPageName); useEffect(() => { + PerformanceTracker.stopTracking(PerformanceTransactionName.EDITOR_MOUNT); PerformanceTracker.stopTracking(PerformanceTransactionName.CLOSE_API); }); @@ -111,7 +110,6 @@ const WidgetsEditor = () => { node = ; } log.debug("Canvas rendered"); - PerformanceTracker.stopTracking(); return ( diff --git a/app/client/src/pages/Editor/routes.tsx b/app/client/src/pages/Editor/routes.tsx index 5b962bccc8..e11560552d 100644 --- a/app/client/src/pages/Editor/routes.tsx +++ b/app/client/src/pages/Editor/routes.tsx @@ -1,5 +1,10 @@ import React, { useEffect, ReactNode } from "react"; -import { Switch, withRouter, RouteComponentProps } from "react-router-dom"; +import { + Switch, + withRouter, + RouteComponentProps, + Route, +} from "react-router-dom"; import ApiEditor from "./APIEditor"; import QueryEditor from "./QueryEditor"; import DataSourceEditor from "./DataSourceEditor"; @@ -21,7 +26,6 @@ import { getProviderTemplatesURL, } from "constants/routes"; import styled from "styled-components"; -import AppRoute from "pages/common/AppRoute"; import { useShowPropertyPane, useWidgetSelection, @@ -32,6 +36,9 @@ import PerformanceTracker, { PerformanceTransactionName, } from "utils/PerformanceTracker"; +import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); + const Wrapper = styled.div<{ isVisible: boolean }>` position: absolute; top: 0; @@ -102,60 +109,47 @@ class EditorsRouter extends React.Component< onClick={this.preventClose} > - - + - - - - - - - diff --git a/app/client/src/pages/UserAuth/index.tsx b/app/client/src/pages/UserAuth/index.tsx index 27deeb86a3..80633c5248 100644 --- a/app/client/src/pages/UserAuth/index.tsx +++ b/app/client/src/pages/UserAuth/index.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { Switch, useRouteMatch, useLocation } from "react-router-dom"; +import { Switch, useRouteMatch, useLocation, Route } from "react-router-dom"; import Login from "./Login"; import Centered from "components/designSystems/appsmith/CenteredWrapper"; import { animated, useTransition } from "react-spring"; @@ -7,8 +7,10 @@ import { AuthContainer, AuthCard } from "./StyledComponents"; import SignUp from "./SignUp"; import ForgotPassword from "./ForgotPassword"; import ResetPassword from "./ResetPassword"; -import AppRoute from "pages/common/AppRoute"; import PageNotFound from "pages/common/PageNotFound"; +import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); + const AnimatedAuthCard = animated(AuthContainer); export const UserAuth = () => { const { path } = useRouteMatch(); @@ -25,31 +27,19 @@ export const UserAuth = () => { - - - + + - - + diff --git a/app/client/src/pages/common/AppRoute.tsx b/app/client/src/pages/common/AppRoute.tsx index 4bef0352d7..3ce4dbaecb 100644 --- a/app/client/src/pages/common/AppRoute.tsx +++ b/app/client/src/pages/common/AppRoute.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { Route } from "react-router-dom"; +import { Route, RouteComponentProps } from "react-router-dom"; import AnalyticsUtil from "utils/AnalyticsUtil"; import * as Sentry from "@sentry/react"; import { connect } from "react-redux"; @@ -7,29 +7,29 @@ import { getThemeDetails } from "selectors/themeSelectors"; import { AppState } from "reducers"; import { ThemeMode } from "reducers/uiReducers/themeReducer"; import { setThemeMode } from "actions/themeActions"; +import equal from "fast-deep-equal/es6"; const SentryRoute = Sentry.withSentryRouting(Route); -class AppRouteWithoutProps extends React.Component<{ +interface AppRouteProps { currentTheme: any; path?: string; - component: React.ReactType; + component: + | React.ComponentType> + | React.ComponentType; exact?: boolean; logDisable?: boolean; name: string; location?: any; setTheme: Function; -}> { - componentDidUpdate() { - if (!this.props.logDisable) { - AnalyticsUtil.logEvent("NAVIGATE_EDITOR", { - page: this.props.name, - path: this.props.location.pathname, - }); - } +} + +class AppRouteWithoutProps extends React.Component { + shouldComponentUpdate(prevProps: AppRouteProps, nextProps: AppRouteProps) { + return !equal(prevProps?.location, nextProps?.location); } + render() { const { component: Component, currentTheme, ...rest } = this.props; - if ( window.location.pathname === "/applications" || window.location.pathname.indexOf("/settings/") !== -1 @@ -39,14 +39,7 @@ class AppRouteWithoutProps extends React.Component<{ } else { document.body.style.backgroundColor = currentTheme.colors.appBackground; } - return ( - { - return ; - }} - /> - ); + return ; } } const mapStateToProps = (state: AppState) => ({ diff --git a/app/client/src/pages/organization/index.tsx b/app/client/src/pages/organization/index.tsx index 282a054dea..7b731c4009 100644 --- a/app/client/src/pages/organization/index.tsx +++ b/app/client/src/pages/organization/index.tsx @@ -1,21 +1,19 @@ import React from "react"; -import { Switch, useRouteMatch, useLocation } from "react-router-dom"; +import { Switch, useRouteMatch, useLocation, Route } from "react-router-dom"; import PageWrapper from "pages/common/PageWrapper"; import DefaultOrgPage from "./defaultOrgPage"; -import AppRoute from "pages/common/AppRoute"; import Settings from "./settings"; +import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); + export const Organization = () => { const { path } = useRouteMatch(); const location = useLocation(); return ( - - + + ); diff --git a/app/client/src/pages/organization/settings.tsx b/app/client/src/pages/organization/settings.tsx index dac4ffb5ce..3dfee288ad 100644 --- a/app/client/src/pages/organization/settings.tsx +++ b/app/client/src/pages/organization/settings.tsx @@ -1,6 +1,11 @@ import React, { useEffect } from "react"; -import { useRouteMatch, useLocation, useParams, Link } from "react-router-dom"; -import AppRoute from "pages/common/AppRoute"; +import { + useRouteMatch, + useLocation, + useParams, + Link, + Route, +} from "react-router-dom"; import { getCurrentOrg } from "selectors/organizationSelectors"; import { useSelector, useDispatch } from "react-redux"; import { TabComponent, TabProp } from "components/ads/Tabs"; @@ -12,6 +17,8 @@ import MemberSettings from "./Members"; import IconComponent from "components/designSystems/appsmith/IconComponent"; import { fetchOrg } from "actions/orgActions"; import { GeneralSettings } from "./General"; +import * as Sentry from "@sentry/react"; +const SentryRoute = Sentry.withSentryRouting(Route); const LinkToApplications = styled(Link)` margin-top: 30px; @@ -41,17 +48,15 @@ export default function Settings() { const SettingsRenderer = (
- -
); diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index 76236fd896..62f3505da7 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -660,6 +660,7 @@ export function dependencySortedEvaluateDataTree( try { return sortedDependencies.reduce( (currentTree: DataTree, propertyPath: string) => { + // PerformanceTracker.startTracking(PerformanceTransactionName.EVALUATE_BINDING, { binding: propertyPath }, true) const entityName = propertyPath.split(".")[0]; const entity: DataTreeEntity = currentTree[entityName]; const unEvalPropertyValue = _.get(currentTree as any, propertyPath); @@ -719,10 +720,13 @@ export function dependencySortedEvaluateDataTree( widgetEntity, ); } + // PerformanceTracker.stopTracking(); return _.set(currentTree, propertyPath, parsedValue); } + // PerformanceTracker.stopTracking(); return _.set(currentTree, propertyPath, evalPropertyValue); } else { + // PerformanceTracker.stopTracking(); return _.set(currentTree, propertyPath, evalPropertyValue); } }, diff --git a/app/client/src/utils/PerformanceTracker.ts b/app/client/src/utils/PerformanceTracker.ts index 1042534d81..bc6975bd8a 100644 --- a/app/client/src/utils/PerformanceTracker.ts +++ b/app/client/src/utils/PerformanceTracker.ts @@ -27,8 +27,11 @@ export enum PerformanceTransactionName { GENERATE_VIEW_MODE_PROPS = "GENERATE_VIEW_MODE_PROPS", GENERATE_WIDGET_EDITOR_PROPS = "GENERATE_WIDGET_EDITOR_PROPS", ENTITY_EXPLORER_ENTITY = "ENTITY_EXPLORER_ENTITY", + ENTITY_EXPLORER = "ENTITY_EXPLORER", CLOSE_API = "CLOSE_API", OPEN_API = "OPEN_API", + EDITOR_MOUNT = "EDITOR_MOUNT", + SIDE_BAR_MOUNT = "SIDE_BAR_MOUNT", CANVAS_MOUNT = "CANVAS_MOUNT", GENERATE_API_PROPS = "GENERATE_API_PROPS", CHANGE_API_SAGA = "CHANGE_API_SAGA", @@ -118,27 +121,32 @@ class PerformanceTracker { eventName?: PerformanceTransactionName, ) => { if (eventName) { - let index = -1; - _.forEach(PerformanceTracker.perfLogQueue, (perfLog, i) => { - if (perfLog.eventName === eventName) { - index = i; - } - if (index !== -1 && i >= index) { - const currentSpan = perfLog.sentrySpan; - currentSpan.finish(); - if (!perfLog?.skipLog) { - PerformanceTracker.printDuration( - perfLog.eventName, - PerformanceTracker.perfLogQueue.length + 1, - currentSpan.startTimestamp, - currentSpan.endTimestamp, - ); + const index = _.findLastIndex( + PerformanceTracker.perfLogQueue, + (perfLog, i) => { + return perfLog.eventName === eventName; + }, + ); + if (index !== -1) { + for (let i = PerformanceTracker.perfLogQueue.length - 1; i >= 0; i--) { + const perfLog = PerformanceTracker.perfLogQueue[i]; + if (i >= index) { + const currentSpan = perfLog.sentrySpan; + currentSpan.finish(); + if (!perfLog?.skipLog) { + PerformanceTracker.printDuration( + perfLog.eventName, + i + 1, + currentSpan.startTimestamp, + currentSpan.endTimestamp, + ); + } } } - }); - PerformanceTracker.perfLogQueue = PerformanceTracker.perfLogQueue.splice( - index, - ); + PerformanceTracker.perfLogQueue = PerformanceTracker.perfLogQueue.splice( + index, + ); + } } else { const perfLog = PerformanceTracker.perfLogQueue.pop(); if (perfLog) {