-
Notifications
You must be signed in to change notification settings - Fork 470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support new user analytics events in the Monitor Tab #943
Merged
yurishkuro
merged 12 commits into
jaegertracing:main
from
VladislavBryukhanov:jaegertracing/track-spm-events
May 30, 2022
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
69853ca
Added new SPM tracing events tracking
VladislavBryukhanov fcf2b8b
Added tests for new SPM events
VladislavBryukhanov c477d85
reworked tracking categories & actions names
VladislavBryukhanov 8f60b30
fixed license block dates; moved debounce from tracking module to com…
VladislavBryukhanov 2d7ecd6
Fix monitor tab crashing (#946)
a8dd210
Add repository to package.json (#949)
yurishkuro 80fed84
Preparing release v1.23.0 (#947)
pavolloffay b29b062
Bump github/codeql-action from 1 to 2 (#938)
dependabot[bot] 7add8cd
Clarify development setup
yurishkuro 77f85c3
Use red on scatterplot for traces if any spans have an error=true tag…
esnible e117396
Updated sorting events tracking
VladislavBryukhanov 1c6f9ac
Merge branch 'main' into jaegertracing/track-spm-events
VladislavBryukhanov File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
60 changes: 60 additions & 0 deletions
60
packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| // Copyright (c) 2022 Uber Technologies, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| import * as trackingUtils from '../../../utils/tracking'; | ||
| import { | ||
| CATEGORY_SEARCH_OPERATION, | ||
| CATEGORY_SELECT_SERVICE, | ||
| CATEGORY_SELECT_TIMEFRAME, | ||
| CATEGORY_VIEW_ALL_TRACES, | ||
| trackSelectService, | ||
| trackSelectTimeframe, | ||
| trackViewAllTraces, | ||
| trackSearchOperation, | ||
| } from './index.track'; | ||
|
|
||
| describe('ServicesView tracking', () => { | ||
| let trackEvent; | ||
|
|
||
| beforeAll(() => { | ||
| trackEvent = jest.spyOn(trackingUtils, 'trackEvent').mockImplementation(); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| trackEvent.mockClear(); | ||
| }); | ||
|
|
||
| it('trackViewAllTraces calls trackEvent with the match category and show action', () => { | ||
| trackViewAllTraces(); | ||
| expect(trackEvent).toHaveBeenCalledWith(CATEGORY_VIEW_ALL_TRACES, expect.any(String)); | ||
| }); | ||
|
|
||
| it('trackSelectService calls trackEvent with the match category and show action', () => { | ||
| const serviceName = 'service-name'; | ||
| trackSelectService(serviceName); | ||
| expect(trackEvent).toHaveBeenCalledWith(CATEGORY_SELECT_SERVICE, serviceName); | ||
| }); | ||
|
|
||
| it('trackSelectTimeframe calls trackEvent with the match category and show action', () => { | ||
| const timeframe = 'some-timeframe'; | ||
| trackSelectTimeframe(timeframe); | ||
| expect(trackEvent).toHaveBeenCalledWith(CATEGORY_SELECT_TIMEFRAME, timeframe); | ||
| }); | ||
|
|
||
| it('trackSearchOperation calls trackEvent with the match category and show action', async () => { | ||
| const searchQuery = 'name'; | ||
| trackSearchOperation(searchQuery); | ||
| expect(trackEvent).toHaveBeenCalledWith(CATEGORY_SEARCH_OPERATION, searchQuery); | ||
| }); | ||
| }); |
28 changes: 28 additions & 0 deletions
28
packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Copyright (c) 2022 The Jaeger Authors. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| import { trackEvent } from '../../../utils/tracking'; | ||
|
|
||
| const SPM_CATEGORY_BASE = 'jaeger/ux/trace/spm'; | ||
|
|
||
| export const CATEGORY_VIEW_ALL_TRACES = `${SPM_CATEGORY_BASE}/view-all-traces`; | ||
| export const CATEGORY_SELECT_SERVICE = `${SPM_CATEGORY_BASE}/select-service`; | ||
| export const CATEGORY_SELECT_TIMEFRAME = `${SPM_CATEGORY_BASE}/select-timeframe`; | ||
| export const CATEGORY_SEARCH_OPERATION = `${SPM_CATEGORY_BASE}/search-operation`; | ||
|
|
||
| export const trackViewAllTraces = () => trackEvent(CATEGORY_VIEW_ALL_TRACES, 'click'); | ||
| export const trackSelectService = (service: string) => trackEvent(CATEGORY_SELECT_SERVICE, service); | ||
| export const trackSelectTimeframe = (timeframe: string) => trackEvent(CATEGORY_SELECT_TIMEFRAME, timeframe); | ||
| export const trackSearchOperation = (searchQuery: string) => | ||
| trackEvent(CATEGORY_SEARCH_OPERATION, searchQuery); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why is it necessary to introduce a delay here? Aren't events reported async in the background anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is important to prevent dummy information reporting if user search for some specific name.
For example if the target name is "my-metrics" I attempt to track and send report only once with completed searching query, but without debounce there are 10 separate report requests with partial searching query will be send. The cause of this behaviour is that the onChange handler fires every time a new character is typed into the input, that's why these 10 reports are sent, without debounce:
"m"
"my"
"my-"
...
"my-metric"
"my-metrics"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying a pending debounce gets canceled if another one is issued? That was not my understanding of the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, even if suppress logging of partial names, are the partial queries still being sent to the backend? Having 1-1 between queries and logs seems reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regarding 1-st point - yes, you are right
regarding 2-nd point - the search field filters data only locally on front-end and doesn't sent any requests to the backend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So on point 1 - if debounce merely delays, won't you still end up with the same incremental logs?