Support new user analytics events in the Monitor Tab #943

VladislavBryukhanov · 2022-05-05T16:18:35Z

Which problem is this PR solving?

Resolves #942

Short description of the changes

Added new tracking events to SPM tracing functionality

yurishkuro · 2022-05-05T17:07:08Z

packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.tsx

@@ -0,0 +1,34 @@
+// Copyright (c) 2017 The Jaeger Authors.


Suggested change

// Copyright (c) 2017 The Jaeger Authors.

// Copyright (c) 2022 The Jaeger Authors.

yurishkuro · 2022-05-05T17:09:02Z

packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.tsx

+
+export const ACTION_SELECT_SERVICE = 'select-service';
+export const ACTION_SELECT_TIMEFRAME = 'select-timeframe';
+export const ACTION_VIEW_ALL_TRACES = 'view-all-traces';


Please elaborate what this means (comments?) Is there an alternative to viewing "all" traces?

yurishkuro · 2022-05-05T17:09:20Z

packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.tsx

+export const ACTION_SELECT_SERVICE = 'select-service';
+export const ACTION_SELECT_TIMEFRAME = 'select-timeframe';
+export const ACTION_VIEW_ALL_TRACES = 'view-all-traces';
+export const ACTION_SEARCH_OPERATION = 'search-operation';


Same - search operation where?

I refactored the tracking methods usage and use category instead, is it clearer?

yurishkuro · 2022-05-05T17:13:03Z

packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.tsx

+export const trackSelectTimeframe = (timeframe: string) =>
+  trackEvent(CATEGORY_SPM, ACTION_SELECT_TIMEFRAME, timeframe);
+
+export const trackSearchOperationDebounced = _debounce(


organizationally, it seems this should be just trackSearchOperation, and the debouncing can be done by the caller.

yurishkuro · 2022-05-13T20:07:19Z

packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx

@@ -68,6 +75,8 @@ type TDispatchProps = {
  fetchAllServiceMetrics: (serviceName: string, query: MetricsAPIQueryParams) => void;
 };

+const trackSearchOperationDebounced = _debounce(searchQuery => trackSearchOperation(searchQuery), 1000);


Just curious, why is it necessary to introduce a delay here? Aren't events reported async in the background anyway?

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"

Are you saying a pending debounce gets canceled if another one is issued? That was not my understanding of the function.

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.

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

So on point 1 - if debounce merely delays, won't you still end up with the same incremental logs?

yurishkuro · 2022-05-13T20:09:59Z

lgtm overall. Please make sure all commits are signed (often easier to squash them and resubmit as one).

codecov · 2022-05-13T20:18:31Z

Codecov Report

Merging #943 (1c6f9ac) into main (c15ab14) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #943      +/-   ##
==========================================
- Coverage   95.49%   95.39%   -0.11%     
==========================================
  Files         240      242       +2     
  Lines        7524     7552      +28     
  Branches     1887     1836      -51     
==========================================
+ Hits         7185     7204      +19     
- Misses        332      341       +9     
  Partials        7        7

Impacted Files	Coverage Δ
...rc/components/Monitor/ServicesView/index.track.tsx	`100.00% <100.00%> (ø)`
...r-ui/src/components/Monitor/ServicesView/index.tsx	`97.24% <100.00%> (+0.16%)`	⬆️
...ServicesView/operationDetailsTable/index.track.tsx	`100.00% <100.00%> (ø)`
...nitor/ServicesView/operationDetailsTable/index.tsx	`100.00% <100.00%> (ø)`
...mponents/TracePage/TraceStatistics/tableValues.tsx	`95.51% <0.00%> (-3.11%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c15ab14...1c6f9ac. Read the comment docs.

pavolloffay · 2022-05-19T11:28:14Z

Is there any screenshot or mockup for this feature?

yurishkuro · 2022-05-19T14:42:12Z

@pavolloffay it's an internal observability, aka user tracking

nofar9792 · 2022-05-24T09:13:11Z

@yurishkuro can we merge it or do we need to fix something before?

yurishkuro · 2022-05-24T16:12:07Z

we need DCO check (signed commits) to pass before I can merge.

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

…ponent Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Signed-off-by: nofar9792 <nofar.cohen@logz.io> Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

* Preparing release v1.23.0 Signed-off-by: Pavol Loffay <p.loffay@gmail.com> * Fix Signed-off-by: Pavol Loffay <p.loffay@gmail.com> * Fix Signed-off-by: Pavol Loffay <p.loffay@gmail.com> * Fix Signed-off-by: Pavol Loffay <p.loffay@gmail.com> * Fix Signed-off-by: Pavol Loffay <p.loffay@gmail.com> Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 1 to 2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@v1...v2) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

For jaegertracing#952 Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

…jaegertracing#951) * Use red on scatterplot for traces if any spans have an error=true tag Signed-off-by: Ed Snible <snible@us.ibm.com> * Used 'yarn prettier' to appease 'yarn lint' Signed-off-by: Ed Snible <snible@us.ibm.com> Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

VladislavBryukhanov · 2022-05-27T11:24:18Z

DCO issue resolved @yurishkuro

VladislavBryukhanov requested review from everett980, rubenvp8510 and tiffon as code owners May 5, 2022 16:18

VladislavBryukhanov marked this pull request as draft May 5, 2022 16:32

yurishkuro reviewed May 5, 2022

View reviewed changes

yurishkuro changed the title ~~Jaegertracing/track spm events~~ Support new user analytics events in the Monitor Tab May 10, 2022

VladislavBryukhanov force-pushed the jaegertracing/track-spm-events branch from 341fa3a to cfb8eea Compare May 12, 2022 07:25

yurishkuro reviewed May 13, 2022

View reviewed changes

VladislavBryukhanov requested a review from yurishkuro May 18, 2022 12:24

VladislavBryukhanov marked this pull request as ready for review May 18, 2022 13:26

VladislavBryukhanov and others added 11 commits May 26, 2022 11:09

Added new SPM tracing events tracking

69853ca

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Added tests for new SPM events

fcf2b8b

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

reworked tracking categories & actions names

c477d85

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

fixed license block dates; moved debounce from tracking module to com…

8f60b30

…ponent Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Fix monitor tab crashing (jaegertracing#946)

2d7ecd6

Signed-off-by: nofar9792 <nofar.cohen@logz.io> Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Add repository to package.json (jaegertracing#949)

a8dd210

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Clarify development setup

7add8cd

For jaegertracing#952 Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

Updated sorting events tracking

e117396

Signed-off-by: VladislavBryukhanov <gfedcba625@gmail.com>

VladislavBryukhanov force-pushed the jaegertracing/track-spm-events branch from c3df2e9 to e117396 Compare May 26, 2022 08:12

Merge branch 'main' into jaegertracing/track-spm-events

1c6f9ac

VladislavBryukhanov closed this May 30, 2022

VladislavBryukhanov reopened this May 30, 2022

yurishkuro approved these changes May 30, 2022

View reviewed changes

yurishkuro merged commit e55b3a6 into jaegertracing:main May 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support new user analytics events in the Monitor Tab #943

Support new user analytics events in the Monitor Tab #943

VladislavBryukhanov commented May 5, 2022 •

edited

yurishkuro May 5, 2022

VladislavBryukhanov May 13, 2022

yurishkuro May 5, 2022

yurishkuro May 5, 2022

VladislavBryukhanov May 13, 2022

yurishkuro May 5, 2022

VladislavBryukhanov May 13, 2022

yurishkuro May 13, 2022

VladislavBryukhanov May 18, 2022

yurishkuro May 18, 2022

yurishkuro May 18, 2022

VladislavBryukhanov May 19, 2022

yurishkuro May 27, 2022

yurishkuro commented May 13, 2022

codecov bot commented May 13, 2022 •

edited

pavolloffay commented May 19, 2022

yurishkuro commented May 19, 2022 •

edited

nofar9792 commented May 24, 2022

yurishkuro commented May 24, 2022

VladislavBryukhanov commented May 27, 2022

	// Copyright (c) 2017 The Jaeger Authors.
	// Copyright (c) 2022 The Jaeger Authors.

Support new user analytics events in the Monitor Tab #943

Support new user analytics events in the Monitor Tab #943

Conversation

VladislavBryukhanov commented May 5, 2022 • edited

Which problem is this PR solving?

Short description of the changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro commented May 13, 2022

codecov bot commented May 13, 2022 • edited

Codecov Report

pavolloffay commented May 19, 2022

yurishkuro commented May 19, 2022 • edited

nofar9792 commented May 24, 2022

yurishkuro commented May 24, 2022

VladislavBryukhanov commented May 27, 2022

VladislavBryukhanov commented May 5, 2022 •

edited

codecov bot commented May 13, 2022 •

edited

yurishkuro commented May 19, 2022 •

edited