Add better logging for profile page views (#239)

amundsen_application/api/metadata/v0.py

 import logging
 
 from http import HTTPStatus
-from typing import Any, Dict
+from typing import Any, Dict, Optional
 
 from flask import Response, jsonify, make_response, request
 from flask import current_app as app
@@ -80,8 +80,8 @@ def get_table_metadata() -> Response:
     """
     try:
         table_key = get_query_param(request.args, 'key')
-        list_item_index = get_query_param(request.args, 'index')
-        list_item_source = get_query_param(request.args, 'source')
+        list_item_index = request.args.get('index', None)
+        list_item_source = request.args.get('source', None)
 
         results_dict = _get_table_metadata(table_key=table_key, index=list_item_index, source=list_item_source)
         return make_response(jsonify(results_dict), results_dict.get('status_code', HTTPStatus.INTERNAL_SERVER_ERROR))
@@ -389,14 +389,16 @@ def update_table_tags() -> Response:
 def get_user() -> Response:
 
     @action_logging
-    def _log_get_user(*, user_id: str) -> None:
+    def _log_get_user(*, user_id: str, index: Optional[int], source: Optional[str]) -> None:
         pass  # pragma: no cover
 
     try:
         user_id = get_query_param(request.args, 'user_id')
-        url = '{0}{1}/{2}'.format(app.config['METADATASERVICE_BASE'], USER_ENDPOINT, user_id)
+        index = request.args.get('index', None)
+        source = request.args.get('source', None)
 
-        _log_get_user(user_id=user_id)
+        url = '{0}{1}/{2}'.format(app.config['METADATASERVICE_BASE'], USER_ENDPOINT, user_id)
+        _log_get_user(user_id=user_id, index=index, source=source)
 
         response = request_metadata(url=url)
         status_code = response.status_code

amundsen_application/static/js/components/NavBar/index.tsx

@@ -53,7 +53,7 @@ export class NavBar extends React.Component<NavBarProps> {
               {this.generateNavLinks(AppConfig.navLinks)}
               {
                 this.props.loggedInUser && AppConfig.indexUsers.enabled &&
-                <Link id="nav-bar-avatar-link" to={`/user/${this.props.loggedInUser.user_id}`}>
+                <Link id="nav-bar-avatar-link" to={`/user/${this.props.loggedInUser.user_id}?source=navbar`}>
                   <div id="nav-bar-avatar">
                     <Avatar name={this.props.loggedInUser.display_name} size={32} round={true} />
                   </div>

amundsen_application/static/js/components/NavBar/tests/index.spec.tsx

@@ -117,6 +117,10 @@ describe('NavBar', () => {
 
     it('renders a Link to the user profile if `indexUsers` is enabled', () => {
       expect(wrapper.find('#nav-bar-avatar-link').exists()).toBe(true)
+
+      expect(wrapper.find('#nav-bar-avatar-link').props()).toMatchObject({
+        to: `/user/${props.loggedInUser.user_id}?source=navbar`
+      });
     });
 
     it('does not render a Link to the user profile if `indexUsers` is disabled', () => {

amundsen_application/static/js/components/ProfilePage/index.tsx

@@ -4,6 +4,7 @@ import * as Avatar from 'react-avatar';
 import { connect } from 'react-redux';
 import { RouteComponentProps, withRouter } from 'react-router';
 import { bindActionCreators } from 'redux';
+import * as qs from 'simple-query-string';
 
 import Breadcrumb from 'components/common/Breadcrumb';
 import Flag from 'components/common/Flag';
@@ -39,7 +40,7 @@ interface StateFromProps {
 }
 
 interface DispatchFromProps {
-  getUserById: (userId: string) => GetUserRequest;
+  getUserById: (userId: string, index?: number, source?: string) => GetUserRequest;
   getUserOwn: (userId: string) => GetUserOwnRequest;
   getUserRead: (userId: string) => GetUserReadRequest;
   getBookmarksForUser: (userId: string) => GetBookmarksForUserRequest;
@@ -75,12 +76,25 @@ export class ProfilePage extends React.Component<ProfilePageProps, ProfilePageSt
   }
 
   loadUserInfo = (userId: string) => {
-    this.props.getUserById(userId);
+    const { index, source } = this.getLoggingParams(this.props.location.search);
+    this.props.getUserById(userId, index, source);
     this.props.getUserOwn(userId);
     this.props.getUserRead(userId);
     this.props.getBookmarksForUser(userId);
   };
 
+  getLoggingParams = (search: string) => {
+    const params = qs.parse(search);
+    const index = params['index'];
+    const source = params['source'];
+    // Remove logging params from URL
+    if (source !== undefined || index !== undefined) {
+      window.history.replaceState({}, '', `${window.location.origin}${window.location.pathname}`);
+    }
+    return { index, source };
+  };
+
+
   getTabContent = (resource: Resource[], source: string, label: string) => {
     // TODO: consider moving logic for empty content into Tab component
     if (resource.length === 0) {

amundsen_application/static/js/components/ProfilePage/tests/index.spec.tsx

@@ -90,6 +90,13 @@ describe('ProfilePage', () => {
 
 
   describe('loadUserInfo', () => {
+    it('calls getLoggingParams', () => {
+      const { props, wrapper } = setup();
+      const getLoggingParamsSpy = jest.spyOn(wrapper.instance(), 'getLoggingParams');
+      wrapper.instance().loadUserInfo('test')
+      expect(getLoggingParamsSpy).toHaveBeenCalledWith(props.location.search);
+    });
+
     it('calls props.getUserById', () => {
       const { props, wrapper } = setup();
       expect(props.getUserById).toHaveBeenCalled();
@@ -111,6 +118,41 @@ describe('ProfilePage', () => {
     });
   });
 
+  describe('getLoggingParams', () => {
+    let searchString;
+    let props;
+    let wrapper;
+    let replaceStateSpy;
+
+    beforeAll(() => {
+      const setupResult = setup();
+      props = setupResult.props;
+      wrapper = setupResult.wrapper;
+      replaceStateSpy = jest.spyOn(window.history, 'replaceState');
+    });
+
+    it('returns the parsed source and index in an object', () => {
+      searchString = 'source=test_source&index=10';
+      const params = wrapper.instance().getLoggingParams(searchString);
+      expect(params.source).toEqual('test_source');
+      expect(params.index).toEqual('10');
+    });
+
+    it('clears the logging params from the URL, if present', () => {
+      searchString = 'source=test_source&index=10';
+      replaceStateSpy.mockClear();
+      wrapper.instance().getLoggingParams(searchString);
+      expect(replaceStateSpy).toHaveBeenCalledWith({}, '', `${window.location.origin}${window.location.pathname}`);
+    });
+
+    it('does not clear the logging params if they do not exist', () => {
+      searchString = '';
+      replaceStateSpy.mockClear();
+      wrapper.instance().getLoggingParams(searchString);
+      expect(replaceStateSpy).not.toHaveBeenCalled()
+    });
+  });
+
   describe('getTabContent', () => {
     let props;
     let wrapper;

amundsen_application/static/js/components/TableDetail/index.tsx

@@ -138,7 +138,7 @@ export class TableDetail extends React.Component<TableDetailProps & RouteCompone
     let link = user.profile_url;
     let target = '_blank';
     if (AppConfig.indexUsers.enabled) {
-      link = `/user/${user.user_id}`;
+      link = `/user/${user.user_id}?source=frequent_users`;
       target = '';
     }
 

amundsen_application/static/js/components/common/ResourceListItem/UserListItem/index.tsx

@@ -19,7 +19,7 @@ class UserListItem extends React.Component<UserListItemProps, {}> {
 
   getLink = () => {
     const { user, logging } = this.props;
-    return `/user/${user.user_id}/?index=${logging.index}&source=${logging.source}`;
+    return `/user/${user.user_id}?index=${logging.index}&source=${logging.source}`;
   };
 
   render() {

amundsen_application/static/js/components/common/ResourceListItem/UserListItem/tests/index.spec.tsx

@@ -98,7 +98,7 @@ describe('UserListItem', () => {
     it('getLink returns correct string', () => {
       const { props, wrapper } = setup();
       const { user, logging } = props;
-      expect(wrapper.instance().getLink()).toEqual(`/user/${user.user_id}/?index=${logging.index}&source=${logging.source}`);
+      expect(wrapper.instance().getLink()).toEqual(`/user/${user.user_id}?index=${logging.index}&source=${logging.source}`);
     });
   });
 });

amundsen_application/static/js/ducks/tableMetadata/api/helpers.ts

+import * as qs from 'simple-query-string';
+
 import { filterFromObj, sortTagsAlphabetical } from 'ducks/utilMethods';
 
 import { OwnerDict, TableMetadata, Tag, User } from 'interfaces';
@@ -6,8 +8,8 @@ import * as API from './v0';
 /**
  * Generates the query string parameters needed for requests that act on a particular table resource.
  */
-export function getTableQueryParams(tableKey: string): string {
-  return `key=${encodeURIComponent(tableKey)}`;
+export function getTableQueryParams(key: string, index?: string, source?: string): string {
+  return qs.stringify({ key, index, source });
 }
 
 /**

amundsen_application/static/js/ducks/tableMetadata/api/tests/index.spec.ts

 import axios from 'axios';
+import * as qs from 'simple-query-string';
 
 import * as Helpers from '../helpers';
 
@@ -25,10 +26,30 @@ describe('helpers', () => {
      tableData: tableResponseData,
      msg: 'Success',
    };
-  })
-  it('getTableQueryParams',() => {
-    const tableKey = 'testKey';
-    expect(Helpers.getTableQueryParams(tableKey)).toEqual(`key=${encodeURIComponent(tableKey)}`)
+  });
+
+  describe('getTableQueryParams', () => {
+    it('generates table query params with a key',() => {
+      const tableKey = 'database://cluster.schema/table';
+      const queryString = Helpers.getTableQueryParams(tableKey);
+      const params = qs.parse(queryString);
+
+      expect(params.key).toEqual(tableKey);
+      expect(params.index).toEqual(undefined);
+      expect(params.source).toEqual(undefined);
+    });
+
+    it('generates query params with logging params',() => {
+      const tableKey = 'database://cluster.schema/table';
+      const index = '4';
+      const source = 'test-source';
+      const queryString = Helpers.getTableQueryParams(tableKey, index, source);
+      const params = qs.parse(queryString);
+
+      expect(params.key).toEqual(tableKey);
+      expect(params.index).toEqual(index);
+      expect(params.source).toEqual(source);
+    });
   });
 
   it('getTableDataFromResponseData',() => {

amundsen_application/static/js/ducks/tableMetadata/api/v0.ts

 import axios, { AxiosResponse, AxiosError } from 'axios';
 
-import {
-  GetPreviewDataRequest, GetTableDataRequest, UpdateTableOwnerRequest, UpdateTagsRequest,
-} from 'ducks/tableMetadata/types';
-
 import { PreviewData, PreviewQueryParams, TableMetadata, User, Tag } from 'interfaces';
 
 const API_PATH = '/api/metadata/v0';
@@ -27,7 +23,7 @@ import {
 
 export function getTableTags(tableKey: string) {
   const tableParams = getTableQueryParams(tableKey);
-  return axios.get(`${API_PATH}/table?${tableParams}&index=&source=`)
+  return axios.get(`${API_PATH}/table?${tableParams}`)
   .then((response: AxiosResponse<TableDataAPI>) => {
     return getTableTagsFromResponseData(response.data);
   });
@@ -48,9 +44,9 @@ export function updateTableTags(tagArray, tableKey: string) {
   return updatePayloads.map(payload => { axios(payload) });
 }
 
-export function getTableData(tableKey: string, searchIndex: string, source: string ) {
-  const tableParams = getTableQueryParams(tableKey);
-  return axios.get(`${API_PATH}/table?${tableParams}&index=${searchIndex}&source=${source}`)
+export function getTableData(tableKey: string, index?: string, source?: string ) {
+  const queryParams = getTableQueryParams(tableKey, index, source);
+  return axios.get(`${API_PATH}/table?${queryParams}`)
   .then((response: AxiosResponse<TableDataAPI>) => {
     return {
       data: getTableDataFromResponseData(response.data),
@@ -85,7 +81,7 @@ export function updateTableDescription(description: string, tableData: TableMeta
 
 export function getTableOwners(tableKey: string) {
   const tableParams = getTableQueryParams(tableKey);
-  return axios.get(`${API_PATH}/table?${tableParams}&index=&source=`)
+  return axios.get(`${API_PATH}/table?${tableParams}`)
   .then((response: AxiosResponse<TableDataAPI>) => {
     return getTableOwnersFromResponseData(response.data);
   });

amundsen_application/static/js/ducks/tableMetadata/reducer.ts

@@ -17,7 +17,7 @@ import tableOwnersReducer, { initialOwnersState, TableOwnerReducerState } from '
 import tableTagsReducer, { initialTagsState,  TableTagsReducerState } from './tags/reducer';
 
 /* ACTIONS */
-export function getTableData(key: string, searchIndex: string = '', source: string = ''): GetTableDataRequest {
+export function getTableData(key: string, searchIndex?: string, source?: string): GetTableDataRequest {
   return {
     payload: {
       key,

amundsen_application/static/js/ducks/user/api/v0.ts

 import axios, { AxiosResponse } from 'axios';
+import * as qs from 'simple-query-string';
 
 import { LoggedInUser, PeopleUser, Resource } from 'interfaces';
 
@@ -14,8 +15,10 @@ export function getLoggedInUser() {
     });
 }
 
-export function getUser(userId: string) {
-  return axios.get(`/api/metadata/v0/user?user_id=${userId}`)
+export function getUser(userId: string, index?: number, source?: string) {
+  const queryParams = qs.stringify({ index, source, user_id: userId });
+
+  return axios.get(`/api/metadata/v0/user?${queryParams}`)
     .then((response: AxiosResponse<UserAPI>) => {
       return response.data.user;
     });

amundsen_application/static/js/ducks/user/reducer.ts

@@ -18,8 +18,8 @@ export function getLoggedInUserSuccess(user: LoggedInUser): GetLoggedInUserRespo
   return { type: GetLoggedInUser.SUCCESS, payload: { user } };
 };
 
-export function getUser(userId: string): GetUserRequest {
-  return { type: GetUser.REQUEST, payload: { userId } };
+export function getUser(userId: string, index?: number, source?: string): GetUserRequest {
+  return { type: GetUser.REQUEST, payload: { userId, index, source } };
 };
 export function getUserFailure(): GetUserResponse {
   return { type: GetUser.FAILURE };

amundsen_application/static/js/ducks/user/sagas.ts

@@ -34,7 +34,8 @@ export function* getLoggedInUserWatcher(): SagaIterator {
 
 export function* getUserWorker(action: GetUserRequest): SagaIterator {
   try {
-    const user = yield call(API.getUser, action.payload.userId);
+    const payload = action.payload;
+    const user = yield call(API.getUser, payload.userId, payload.index, payload.source);
     yield put(getUserSuccess(user));
   } catch (e) {
     yield put(getUserFailure());

amundsen_application/static/js/ducks/user/tests/index.spec.ts

@@ -33,10 +33,14 @@ describe('user ducks', () => {
     user: PeopleUser,
   };
   let userId: string;
+  let source: string;
+  let index: number;
   beforeAll(() => {
     currentUser = globalState.user.loggedInUser;
     otherUser = globalState.user.profile;
     userId = 'testId';
+    source = 'test';
+    index = 0;
   });
 
   describe('actions', () => {
@@ -254,8 +258,8 @@ describe('user ducks', () => {
 
     describe('getUserWorker', () => {
       it('executes flow for returning a user given an id', () => {
-        testSaga(getUserWorker, getUser(userId))
-          .next().call(API.getUser, userId)
+        testSaga(getUserWorker, getUser(userId, index, source))
+          .next().call(API.getUser, userId, index, source)
           .next(otherUser.user).put(getUserSuccess(otherUser.user))
           .next().isDone();
       });

amundsen_application/static/js/ducks/user/types.ts

@@ -25,6 +25,8 @@ export interface GetUserRequest {
   type: GetUser.REQUEST;
   payload: {
     userId: string;
+    source?: string;
+    index?: number;
   };
 };
 export interface GetUserResponse {