Monitor Tab - New look for empty state + make empty state configurable #916

nofar9792 · 2022-03-22T12:25:50Z

Which problem is this PR solving?

Resolves #907

Short description of the changes

Add configuration + default configuration.
Use the config in the monitor empty state.
Furthermore, show bar in case the user hasn't sent metrics yet with link to the documentation

before:

after:

before:

after: (with bar)

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

packages/jaeger-ui/src/types/config.tsx

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

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

codecov · 2022-03-22T12:36:41Z

Codecov Report

Merging #916 (fd39826) into main (80a7502) will decrease coverage by 0.00%.
The diff coverage is 89.28%.

@@            Coverage Diff             @@
##             main     #916      +/-   ##
==========================================
- Coverage   95.33%   95.32%   -0.01%     
==========================================
  Files         240      240              
  Lines        7502     7510       +8     
  Branches     1871     1880       +9     
==========================================
+ Hits         7152     7159       +7     
- Misses        344      345       +1     
  Partials        6        6

Impacted Files	Coverage Δ
...nitor/ServicesView/operationDetailsTable/index.tsx	`100.00% <ø> (ø)`
...ackages/jaeger-ui/src/constants/default-config.tsx	`66.66% <0.00%> (-33.34%)`	⬇️
...ger-ui/src/components/Monitor/EmptyState/index.tsx	`92.85% <90.00%> (-7.15%)`	⬇️
...r-ui/src/components/Monitor/ServicesView/index.tsx	`97.05% <94.11%> (+0.02%)`	⬆️
packages/jaeger-ui/src/components/App/TopNav.tsx	`94.44% <0.00%> (+2.77%)`	⬆️

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 80a7502...fd39826. Read the comment docs.

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

packages/jaeger-ui/src/types/config.tsx

nofar9792 · 2022-03-24T15:23:39Z

@yuribit @albertteoh WDYT?

nofar9792 · 2022-03-25T15:38:03Z

@yuribit @albertteoh WDYT?

i meant @yurishkuro

yurishkuro · 2022-03-25T16:25:37Z

looks great to me, but I don't have the knowledge to review React code.

albertteoh

Nice work, @nofar9792! I've added a few comments to get yours and others' thoughts on wording and naming.

Similar to Yuri, I can't comment much on the React code due to lack of knowledge, so trust that @AlonMiz and @galangel had a good look over it.

albertteoh · 2022-03-26T07:40:21Z

packages/jaeger-ui/src/constants/default-config.tsx

+            'Service Performance Monitoring works in conjunction with a Prometheus compatible time series database. The metrics are aggregated from your tracing data through a one-time configuration. ',
+          button: {
+            text: 'Read the Documentation',
+            onClick: () => window.open('https://www.jaegertracing.io/docs/latest/frontend-ui/'),


I suggest referencing this URL: https://www.jaegertracing.io/docs/latest/atm/.

It's yet to be released, but should be available in the coming documentation release. It's currently viewable in: https://www.jaegertracing.io/docs/next-release/atm/

changing. BTW we get 404 now

BTW we get 404 now

That's fine, because that page hasn't been released yet, but should be released within 2 weeks, which is when Jaeger UI will also be released.

albertteoh · 2022-03-26T07:51:14Z

packages/jaeger-ui/src/constants/default-config.tsx

+      monitor: {
+        menuEnabled: true,
+        emptyState: {
+          mainTitle: 'Get started with Service Monitoring',


I noticed the term "Service Performance Monitoring" is used below while it's "Service Monitoring" here; should we make it consistent?

Perhaps for broader discussion among stakeholders, but what do folks think about renaming "ATM"/"Aggregated Trace Metrics" in https://www.jaegertracing.io/docs/next-release/atm/ to "Service Performance Monitoring" to be consistent with the UI, assuming the latter is the preferred name for this feature? Happy to make the change in docs if so, as I'm not particularly attached to "ATM" name.

What do you think?

+1, I think SPM is a better term that describes the business function, where as ATM describes the implementation.

If we do that, let's do a cleanup in the docs too.

changing to Service Performance Monitoring

let's do a cleanup in the docs too.

@yurishkuro, re: cleaning up the docs, besides renaming ATM in docs, were you referring to this? jaegertracing/documentation#541

Or did you have something else in mind?

not only that one, Albert already added documentation using the term ATM, so we'd want to change that.

packages/jaeger-ui/src/constants/default-config.tsx

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 · 2022-03-27T11:01:00Z

@albertteoh @yurishkuro
I fixed everything
I'm waiting for your approval
after all changes:

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 · 2022-03-27T13:24:54Z

i added support in configurable alert type. we will be able to change the alert in the empty state page:

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

albertteoh

LGTM, just that we should replace ATM with SPM, but we could do this in another PR.

albertteoh · 2022-03-28T10:56:41Z

packages/jaeger-ui/src/constants/default-config.tsx

+            'Service Performance Monitoring aggregates tracing data into RED metrics through a one-time configuration and visualizes these metrics through service and operation level dashboards.',
+          button: {
+            text: 'Read the Documentation',
+            onClick: () => window.open('https://www.jaegertracing.io/docs/latest/atm/'),


FYI, jaegertracing/documentation#567, so the URL will change to https://www.jaegertracing.io/docs/latest/spm/.

Alternatively, we can make this change in another PR, as I noticed there are other references to ATM here like MonitorATMEmptyState.

Apologies for the back and forth!

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 · 2022-03-29T11:17:41Z

@yurishkuro ping..?

nofar9792 · 2022-03-31T11:35:15Z

@yurishkuro ?

yurishkuro

please update the screenshots to reflect later changes
still need someone with UI/React experience to review this code

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

packages/jaeger-ui/src/constants/default-config.tsx

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 · 2022-04-03T08:52:36Z

please update the screenshots to reflect later changes

still need someone with UI/React experience to review this code

@yurishkuro
Screenshots have been updated
who is going to review my code? @AlonMiz and @galangel already did

nofar9792 added 4 commits March 20, 2022 18:22

Emprty state

b68c3c3

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

Small fixes

d71b396

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

Add image

c135e38

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

Remove get help

2ecf9aa

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 requested review from everett980, rubenvp8510 and tiffon as code owners March 22, 2022 12:25

AlonMiz reviewed Mar 22, 2022

View reviewed changes

packages/jaeger-ui/src/types/config.tsx Outdated Show resolved Hide resolved

AlonMiz reviewed Mar 22, 2022

View reviewed changes

packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx Outdated Show resolved Hide resolved

AlonMiz reviewed Mar 22, 2022

View reviewed changes

packages/jaeger-ui/src/components/Monitor/EmptyState/index.tsx Show resolved Hide resolved

AlonMiz reviewed Mar 22, 2022

View reviewed changes

packages/jaeger-ui/src/components/Monitor/EmptyState/index.tsx Outdated Show resolved Hide resolved

nofar9792 added 3 commits March 22, 2022 14:41

Button text

c2f0e2b

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

Sett state

7457324

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

Button config

90d05b0

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

galangel reviewed Mar 22, 2022

View reviewed changes

packages/jaeger-ui/src/types/config.tsx Show resolved Hide resolved

nofar9792 marked this pull request as draft March 23, 2022 09:30

nofar9792 marked this pull request as ready for review March 24, 2022 15:23

albertteoh reviewed Mar 26, 2022

View reviewed changes

Copy

af1b4bb

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 added 2 commits March 27, 2022 16:18

Support error

1aa44a6

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

Error

87d3db6

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

New text

fda8829

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

albertteoh mentioned this pull request Mar 28, 2022

ATM to SPM jaegertracing/documentation#567

Merged

albertteoh previously approved these changes Mar 28, 2022

View reviewed changes

Fix bar

bce459c

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

nofar9792 dismissed albertteoh’s stale review via bce459c March 28, 2022 13:36

yurishkuro reviewed Mar 31, 2022

View reviewed changes

nofar9792 and others added 6 commits April 3, 2022 10:49

Update packages/jaeger-ui/src/constants/default-config.tsx

69f2669

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

Update packages/jaeger-ui/src/constants/default-config.tsx

048425c

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

Update packages/jaeger-ui/src/constants/default-config.tsx

ff40876

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

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

99ab45c

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

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

c6b882e

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

New copy

fd39826

Signed-off-by: nofar9792 <nofar.cohen@logz.io>

yurishkuro merged commit e533b48 into jaegertracing:main Apr 3, 2022

nofar9792 deleted the empty-state-configurable branch April 3, 2022 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monitor Tab - New look for empty state + make empty state configurable #916

Monitor Tab - New look for empty state + make empty state configurable #916

nofar9792 commented Mar 22, 2022 •

edited

Loading

codecov bot commented Mar 22, 2022 •

edited

Loading

nofar9792 commented Mar 24, 2022

nofar9792 commented Mar 25, 2022

yurishkuro commented Mar 25, 2022

albertteoh left a comment

albertteoh Mar 26, 2022

nofar9792 Mar 27, 2022

albertteoh Mar 27, 2022

albertteoh Mar 26, 2022

yurishkuro Mar 26, 2022

yurishkuro Mar 26, 2022

nofar9792 Mar 27, 2022

albertteoh Mar 27, 2022

yurishkuro Mar 27, 2022

nofar9792 commented Mar 27, 2022 •

edited

Loading

nofar9792 commented Mar 27, 2022

albertteoh left a comment

albertteoh Mar 28, 2022

nofar9792 commented Mar 29, 2022

nofar9792 commented Mar 31, 2022

yurishkuro left a comment

nofar9792 commented Apr 3, 2022

Monitor Tab - New look for empty state + make empty state configurable #916

Monitor Tab - New look for empty state + make empty state configurable #916

Conversation

nofar9792 commented Mar 22, 2022 • edited Loading

Which problem is this PR solving?

Short description of the changes

codecov bot commented Mar 22, 2022 • edited Loading

Codecov Report

nofar9792 commented Mar 24, 2022

nofar9792 commented Mar 25, 2022

yurishkuro commented Mar 25, 2022

albertteoh left a 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

nofar9792 commented Mar 27, 2022 • edited Loading

nofar9792 commented Mar 27, 2022

albertteoh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nofar9792 commented Mar 29, 2022

nofar9792 commented Mar 31, 2022

yurishkuro left a comment

Choose a reason for hiding this comment

nofar9792 commented Apr 3, 2022

nofar9792 commented Mar 22, 2022 •

edited

Loading

codecov bot commented Mar 22, 2022 •

edited

Loading

nofar9792 commented Mar 27, 2022 •

edited

Loading