From fe2432a40c7d4e2247c4d5b8117cf6b7c9d52c19 Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Tue, 26 Sep 2017 11:01:24 -0400 Subject: [PATCH 1/2] Fix traceIDs query params clobbering form search (#83) --- src/components/SearchTracePage/TraceSearchForm.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/SearchTracePage/TraceSearchForm.js b/src/components/SearchTracePage/TraceSearchForm.js index 393cb34bac..4766aad656 100644 --- a/src/components/SearchTracePage/TraceSearchForm.js +++ b/src/components/SearchTracePage/TraceSearchForm.js @@ -285,8 +285,8 @@ const mapDispatchToProps = dispatch => { minDuration, maxDuration, lookback, - traceIDs, } = fields; + // Note: traceID is ignored when the form is submitted store.set('lastSearch', { service, operation }); @@ -317,7 +317,6 @@ const mapDispatchToProps = dispatch => { tag: tagsToQuery(tags) || undefined, minDuration: minDuration || null, maxDuration: maxDuration || null, - traceID: traceIDsToQuery(traceIDs) || undefined, }); }, }; From 5139c7f0f486737a873a7559acd80f9305ecb468 Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Tue, 26 Sep 2017 11:23:05 -0400 Subject: [PATCH 2/2] Fix Google Analytics tracking (#81) * Fix Google Analytics tracking * Google Analytics tracking moved to a higher level component * Misc css tweak --- .flowconfig | 1 + flow-typed/npm/react-router-dom_v4.x.x.js | 148 ++++++++++++++++++ package.json | 2 +- src/components/App/Page.js | 60 +++++-- src/components/App/index.js | 8 +- .../TraceTimelineViewer/SpanBarRow.css | 2 +- src/index.js | 6 +- src/utils/metrics.js | 47 ++---- 8 files changed, 211 insertions(+), 63 deletions(-) create mode 100644 flow-typed/npm/react-router-dom_v4.x.x.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..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,26 +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({ children }) { - 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 4916ecce44..3e27b5d5fb 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'; @@ -35,13 +34,10 @@ 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(); export default class JaegerUIApp extends Component { @@ -70,7 +66,7 @@ export default class JaegerUIApp extends Component { return ( - + @@ -80,7 +76,7 @@ export default class JaegerUIApp extends Component { - + ); 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 { 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); +}