From 104f0ae40af4656139ca8ec312878b5e28aca847 Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Thu, 21 Sep 2017 20:13:54 -0400 Subject: [PATCH 1/3] Fix Google Analytics tracking --- .flowconfig | 1 + flow-typed/npm/react-router-dom_v4.x.x.js | 148 ++++++++++++++++++ package.json | 2 +- src/components/App/Page.js | 3 +- src/components/App/index.js | 19 +-- .../App/tracked-component-enhancer.js | 60 +++++++ src/index.js | 6 +- src/utils/metrics.js | 47 ++---- 8 files changed, 235 insertions(+), 51 deletions(-) create mode 100644 flow-typed/npm/react-router-dom_v4.x.x.js create mode 100644 src/components/App/tracked-component-enhancer.js diff --git a/.flowconfig b/.flowconfig index d928c88e81..3aecbf5a94 100644 --- a/.flowconfig +++ b/.flowconfig @@ -8,6 +8,7 @@ [include] [libs] +./flow-typed/npm [options] diff --git a/flow-typed/npm/react-router-dom_v4.x.x.js b/flow-typed/npm/react-router-dom_v4.x.x.js new file mode 100644 index 0000000000..be0f5399a5 --- /dev/null +++ b/flow-typed/npm/react-router-dom_v4.x.x.js @@ -0,0 +1,148 @@ +// flow-typed signature: 4d8e947f2e396ef2f26ecbd1ed7f04ab +// flow-typed version: 97d98ab83e/react-router-dom_v4.x.x/flow_>=v0.53.x + +declare module 'react-router-dom' { + declare export class BrowserRouter extends React$Component<{ + basename?: string, + forceRefresh?: boolean, + getUserConfirmation?: GetUserConfirmation, + keyLength?: number, + children?: React$Node, + }> {} + + declare export class HashRouter extends React$Component<{ + basename?: string, + getUserConfirmation?: GetUserConfirmation, + hashType?: 'slash' | 'noslash' | 'hashbang', + children?: React$Node, + }> {} + + declare export class Link extends React$Component<{ + to: string | LocationShape, + replace?: boolean, + children?: React$Node, + }> {} + + declare export class NavLink extends React$Component<{ + to: string | LocationShape, + activeClassName?: string, + className?: string, + activeStyle?: Object, + style?: Object, + isActive?: (match: Match, location: Location) => boolean, + children?: React$Node, + exact?: boolean, + strict?: boolean, + }> {} + + // NOTE: Below are duplicated from react-router. If updating these, please + // update the react-router and react-router-native types as well. + declare export type Location = { + pathname: string, + search: string, + hash: string, + state?: any, + key?: string, + }; + + declare export type LocationShape = { + pathname?: string, + search?: string, + hash?: string, + state?: any, + }; + + declare export type HistoryAction = 'PUSH' | 'REPLACE' | 'POP'; + + declare export type RouterHistory = { + length: number, + location: Location, + action: HistoryAction, + listen(callback: (location: Location, action: HistoryAction) => void): () => void, + push(path: string | LocationShape, state?: any): void, + replace(path: string | LocationShape, state?: any): void, + go(n: number): void, + goBack(): void, + goForward(): void, + canGo?: (n: number) => boolean, + block(callback: (location: Location, action: HistoryAction) => boolean): void, + // createMemoryHistory + index?: number, + entries?: Array, + }; + + declare export type Match = { + params: { [key: string]: ?string }, + isExact: boolean, + path: string, + url: string, + }; + + declare export type ContextRouter = {| + history: RouterHistory, + location: Location, + match: Match, + |}; + + declare export type GetUserConfirmation = (message: string, callback: (confirmed: boolean) => void) => void; + + declare type StaticRouterContext = { + url?: string, + }; + + declare export class StaticRouter extends React$Component<{ + basename?: string, + location?: string | Location, + context: StaticRouterContext, + children?: React$Node, + }> {} + + declare export class MemoryRouter extends React$Component<{ + initialEntries?: Array, + initialIndex?: number, + getUserConfirmation?: GetUserConfirmation, + keyLength?: number, + children?: React$Node, + }> {} + + declare export class Router extends React$Component<{ + history: RouterHistory, + children?: React$Node, + }> {} + + declare export class Prompt extends React$Component<{ + message: string | ((location: Location) => string | true), + when?: boolean, + }> {} + + declare export class Redirect extends React$Component<{ + to: string | LocationShape, + push?: boolean, + }> {} + + declare export class Route extends React$Component<{ + component?: React$ComponentType<*>, + render?: (router: ContextRouter) => React$Node, + children?: React$ComponentType | React$Node, + path?: string, + exact?: boolean, + strict?: boolean, + }> {} + + declare export class Switch extends React$Component<{ + children?: React$Node, + }> {} + + declare export function withRouter

( + Component: React$ComponentType<{| ...ContextRouter, ...P |}> + ): React$ComponentType

; + + declare type MatchPathOptions = { + path?: string, + exact?: boolean, + sensitive?: boolean, + strict?: boolean, + }; + + declare export function matchPath(pathname: string, options?: MatchPathOptions | string): null | Match; +} diff --git a/package.json b/package.json index a6b079d43e..e17e689989 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "test": "react-scripts test --env=jsdom", "coverage": "npm run test -- --coverage", "lint": "npm run eslint && npm run prettier && npm run flow", - "eslint": "eslint .", + "eslint": "eslint src", "prettier": "prettier --single-quote --trailing-comma es5 --print-width 110 --write \"src/**/*.js\"", "flow": "flow; test $? -eq 0 -o $? -eq 2", "set-homepage": "json -I -f package.json -e 'this.homepage=process.env.HOMEPAGE'", diff --git a/src/components/App/Page.js b/src/components/App/Page.js index a5ae110838..4ec00fd4b6 100644 --- a/src/components/App/Page.js +++ b/src/components/App/Page.js @@ -26,7 +26,8 @@ import TopNav from './TopNav'; import './Page.css'; -export default function JaegerUIPage({ children }) { +export default function JaegerUIPage(props) { + const { children } = props; return (

diff --git a/src/components/App/index.js b/src/components/App/index.js index 4916ecce44..7a2ff2e59a 100644 --- a/src/components/App/index.js +++ b/src/components/App/index.js @@ -21,7 +21,6 @@ import React, { Component } from 'react'; import createHistory from 'history/createBrowserHistory'; import PropTypes from 'prop-types'; -import { metrics } from 'react-metrics'; import { Provider } from 'react-redux'; import { Route, Redirect, Switch } from 'react-router-dom'; import { ConnectedRouter } from 'react-router-redux'; @@ -30,20 +29,22 @@ import 'semantic-ui-css/semantic.min.css'; import Page from './Page'; import NotFound from './NotFound'; +import trackedComponentEnahncer from './tracked-component-enhancer'; import { ConnectedDependencyGraphPage } from '../DependencyGraph'; import { ConnectedSearchTracePage } from '../SearchTracePage'; import { ConnectedTracePage } from '../TracePage'; import JaegerAPI, { DEFAULT_API_ROOT } from '../../api/jaeger'; import configureStore from '../../utils/configure-store'; -import metricConfig from '../../utils/metrics'; import prefixUrl from '../../utils/prefix-url'; import './App.css'; -const PageWithMetrics = metrics(metricConfig)(Page); - const defaultHistory = createHistory(); +const TrackedSearchPage = trackedComponentEnahncer(ConnectedSearchTracePage); +const TrackedTracePage = trackedComponentEnahncer(ConnectedTracePage); +const TrackedDependencyPage = trackedComponentEnahncer(ConnectedDependencyGraphPage); + export default class JaegerUIApp extends Component { static get propTypes() { return { @@ -70,17 +71,17 @@ export default class JaegerUIApp extends Component { return ( - + - - - + + + - + ); diff --git a/src/components/App/tracked-component-enhancer.js b/src/components/App/tracked-component-enhancer.js new file mode 100644 index 0000000000..a94fb6eb74 --- /dev/null +++ b/src/components/App/tracked-component-enhancer.js @@ -0,0 +1,60 @@ +// @flow + +// Copyright (c) 2017 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +import * as React from 'react'; +import type { Location } from 'react-router-dom'; + +import { trackPageView } from '../../utils/metrics'; + +type TrackedComponentProps = { + location: Location, +}; + +/** + * Wrap `Component` to add tracking based on changes in + * `props.location.pathname` and `props.location.search`. If either change, a + * page-view is tracked. + */ +export default function trackedComponentEnhancer(Component: any) { + class TrackedComponent extends React.Component { + props: TrackedComponentProps; + + componentDidMount() { + const { pathname, search } = this.props.location; + trackPageView(pathname, search); + } + + componentWillReceiveProps(nextProps: TrackedComponentProps) { + const { pathname, search } = this.props.location; + const { pathname: nextPathname, search: nextSearch } = nextProps.location; + if (pathname !== nextPathname || search !== nextSearch) { + trackPageView(nextPathname, nextSearch); + } + } + + render() { + return ; + } + } + + return TrackedComponent; +} diff --git a/src/index.js b/src/index.js index 66eb95124c..8419feb452 100644 --- a/src/index.js +++ b/src/index.js @@ -25,17 +25,19 @@ import { document } from 'global'; import 'basscss/css/basscss.css'; import JaegerUIApp from './components/App'; +import { init as initTracking } from './utils/metrics'; /* istanbul ignore if */ if (process.env.NODE_ENV === 'development') { require.ensure(['global/window', 'react-addons-perf'], require => { const window = require('global/window'); - /* eslint-disable import/no-extraneous-dependencies */ + // eslint-disable-next-line import/no-extraneous-dependencies window.Perf = require('react-addons-perf'); - /* eslint-enable import/no-extraneous-dependencies */ }); } +initTracking(); + const UI_ROOT_ID = 'jaeger-ui-root'; /* istanbul ignore if */ diff --git a/src/utils/metrics.js b/src/utils/metrics.js index 1e367df016..7af26b9bee 100644 --- a/src/utils/metrics.js +++ b/src/utils/metrics.js @@ -20,43 +20,14 @@ import ReactGA from 'react-ga'; -if (process.env.NODE_ENV === 'production' && process.env.REACT_APP_GA_TRACKING_ID) { - const GA_CODE = process.env.REACT_APP_GA_TRACKING_ID; - ReactGA.initialize(GA_CODE); +export function init() { + if (process.env.NODE_ENV === 'production' && process.env.REACT_APP_GA_TRACKING_ID) { + const GA_CODE = process.env.REACT_APP_GA_TRACKING_ID; + ReactGA.initialize(GA_CODE); + } } -const GoogleAnalytics = { - pageView(event, fields) { - ReactGA.set({ - page: fields.page, - }); - ReactGA.pageview(fields.pathname); - }, - track(eventName, params) { - ReactGA.event({ - category: params.page, - action: eventName, - label: params.label, - value: params.value, - }); - }, -}; - -export default { - enabled: process.env.NODE_ENV === 'production', - debug: process.env.NODE_ENV === 'development', - vendors: [ - { - name: 'Google Analytics', - api: GoogleAnalytics, - }, - ], - pageDefaults(routeState) { - const paths = routeState.pathname.substr(1).split('/'); - return { - pathname: routeState.pathname, - query: routeState.query, - page: !paths[0] ? 'landing' : paths[0], - }; - }, -}; +export function trackPageView(pathname, search) { + const pagePath = search ? `${pathname}?${search}` : pathname; + ReactGA.pageview(pagePath); +} From a9eb89b9e54043d99d7b2b7d56595344ad406379 Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Fri, 22 Sep 2017 16:15:29 -0400 Subject: [PATCH 2/3] Google Analytics tracking moved to a higher level component --- src/components/App/Page.js | 61 ++++++++++++++----- src/components/App/index.js | 11 +--- .../App/tracked-component-enhancer.js | 60 ------------------ 3 files changed, 48 insertions(+), 84 deletions(-) delete mode 100644 src/components/App/tracked-component-enhancer.js diff --git a/src/components/App/Page.js b/src/components/App/Page.js index 4ec00fd4b6..8a28a5bc25 100644 --- a/src/components/App/Page.js +++ b/src/components/App/Page.js @@ -1,3 +1,5 @@ +// @flow + // Copyright (c) 2017 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy @@ -18,27 +20,54 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -import PropTypes from 'prop-types'; -import React from 'react'; +import * as React from 'react'; import Helmet from 'react-helmet'; +import { connect } from 'react-redux'; +import type { Location } from 'react-router-dom'; import TopNav from './TopNav'; +import { trackPageView } from '../../utils/metrics'; import './Page.css'; -export default function JaegerUIPage(props) { - const { children } = props; - return ( -
- - -
- {children} -
-
- ); +type PageProps = { + location: Location, + children: React.Node, +}; + +class Page extends React.Component { + props: PageProps; + + componentDidMount() { + const { pathname, search } = this.props.location; + trackPageView(pathname, search); + } + + componentWillReceiveProps(nextProps: PageProps) { + const { pathname, search } = this.props.location; + const { pathname: nextPathname, search: nextSearch } = nextProps.location; + if (pathname !== nextPathname || search !== nextSearch) { + trackPageView(nextPathname, nextSearch); + } + } + + render() { + const { children } = this.props; + return ( +
+ + +
+ {children} +
+
+ ); + } } -JaegerUIPage.propTypes = { - children: PropTypes.node, -}; +function mapStateToProps(state, ownProps) { + const { location } = state.routing; + return { ...ownProps, location }; +} + +export default connect(mapStateToProps)(Page); diff --git a/src/components/App/index.js b/src/components/App/index.js index 7a2ff2e59a..3e27b5d5fb 100644 --- a/src/components/App/index.js +++ b/src/components/App/index.js @@ -29,7 +29,6 @@ import 'semantic-ui-css/semantic.min.css'; import Page from './Page'; import NotFound from './NotFound'; -import trackedComponentEnahncer from './tracked-component-enhancer'; import { ConnectedDependencyGraphPage } from '../DependencyGraph'; import { ConnectedSearchTracePage } from '../SearchTracePage'; import { ConnectedTracePage } from '../TracePage'; @@ -41,10 +40,6 @@ import './App.css'; const defaultHistory = createHistory(); -const TrackedSearchPage = trackedComponentEnahncer(ConnectedSearchTracePage); -const TrackedTracePage = trackedComponentEnahncer(ConnectedTracePage); -const TrackedDependencyPage = trackedComponentEnahncer(ConnectedDependencyGraphPage); - export default class JaegerUIApp extends Component { static get propTypes() { return { @@ -73,9 +68,9 @@ export default class JaegerUIApp extends Component { - - - + + + diff --git a/src/components/App/tracked-component-enhancer.js b/src/components/App/tracked-component-enhancer.js deleted file mode 100644 index a94fb6eb74..0000000000 --- a/src/components/App/tracked-component-enhancer.js +++ /dev/null @@ -1,60 +0,0 @@ -// @flow - -// Copyright (c) 2017 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -import * as React from 'react'; -import type { Location } from 'react-router-dom'; - -import { trackPageView } from '../../utils/metrics'; - -type TrackedComponentProps = { - location: Location, -}; - -/** - * Wrap `Component` to add tracking based on changes in - * `props.location.pathname` and `props.location.search`. If either change, a - * page-view is tracked. - */ -export default function trackedComponentEnhancer(Component: any) { - class TrackedComponent extends React.Component { - props: TrackedComponentProps; - - componentDidMount() { - const { pathname, search } = this.props.location; - trackPageView(pathname, search); - } - - componentWillReceiveProps(nextProps: TrackedComponentProps) { - const { pathname, search } = this.props.location; - const { pathname: nextPathname, search: nextSearch } = nextProps.location; - if (pathname !== nextPathname || search !== nextSearch) { - trackPageView(nextPathname, nextSearch); - } - } - - render() { - return ; - } - } - - return TrackedComponent; -} From 8f0b60a63845db1c36fe110fc110b859a306da06 Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Sun, 24 Sep 2017 00:12:37 -0400 Subject: [PATCH 3/3] Misc css tweak --- src/components/TracePage/TraceTimelineViewer/SpanBarRow.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/TracePage/TraceTimelineViewer/SpanBarRow.css b/src/components/TracePage/TraceTimelineViewer/SpanBarRow.css index 972a9fab27..5d2896656f 100644 --- a/src/components/TracePage/TraceTimelineViewer/SpanBarRow.css +++ b/src/components/TracePage/TraceTimelineViewer/SpanBarRow.css @@ -38,7 +38,7 @@ THE SOFTWARE. width: 6px; background-image: linear-gradient(to right, rgba(25, 25, 25, 0.25), rgba(32, 32, 32, 0.0)); left: 100%; - z-index: 1; + z-index: -1; } .span-name-wrapper {