Skip to content

feat: log marker indicator in the waterfall view #899

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

Merged
merged 13 commits into from
Jun 11, 2021
Merged

Conversation

itssharmasandeep
Copy link
Contributor

Description

Log marker indicator in the waterfall view

Testing

Local testing done.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

Log markers
Screenshot 2021-06-03 at 6 55 53 PM

Log-marker-hover
Screenshot 2021-06-03 at 6 56 36 PM

@itssharmasandeep itssharmasandeep requested a review from a team as a code owner June 3, 2021 13:41
@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #899 (cd1e654) into main (83db9d9) will decrease coverage by 0.17%.
The diff coverage is 61.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #899      +/-   ##
==========================================
- Coverage   85.34%   85.17%   -0.18%     
==========================================
  Files         803      806       +3     
  Lines       16509    16618     +109     
  Branches     2074     2088      +14     
==========================================
+ Hits        14090    14154      +64     
- Misses       2387     2432      +45     
  Partials       32       32              
Impacted Files Coverage Δ
projects/components/src/public-api.ts 100.00% <ø> (ø)
.../components/src/sequence/sequence-chart.service.ts 89.36% <0.00%> (-1.95%) ⬇️
...s/waterfall/waterfall/waterfall-chart.component.ts 46.73% <25.80%> (-12.64%) ⬇️
...s/waterfall/waterfall-widget-renderer.component.ts 44.44% <37.50%> (-2.23%) ⬇️
...omponents/src/sequence/sequence-chart.component.ts 86.00% <42.85%> (-7.19%) ⬇️
...components/src/tabs/content/tab-group.component.ts 58.33% <64.70%> (+2.77%) ⬆️
...ets/waterfall/waterfall/waterfall-chart.service.ts 93.75% <66.66%> (-1.42%) ⬇️
...sequence/renderer/sequence-bar-renderer.service.ts 94.00% <81.25%> (-6.00%) ⬇️
...c/shared/components/span-detail/span-detail-tab.ts 100.00% <100.00%> (ø)
...ed/components/span-detail/span-detail.component.ts 100.00% <100.00%> (ø)
... and 6 more

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 83db9d9...cd1e654. Read the comment docs.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

arjunlalb
arjunlalb previously approved these changes Jun 8, 2021
Copy link
Contributor

@arjunlalb arjunlalb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

color: segment.color
color: segment.color,
markers: segment.markers
.map((marker: Marker) => ({ ...marker, markerTime: marker.markerTime - minStart }))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: some comments would be helpful


export interface MarkerDatum {
marker: Marker;
origin: ElementRef;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we could use the approach we discussed earlier then that would be great. Mainly, a way to avoid passing an elementRef

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will take a look in this.

Copy link
Contributor

@anandtiwary anandtiwary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@github-actions

This comment has been minimized.

@anandtiwary anandtiwary merged commit d02e165 into main Jun 11, 2021
@anandtiwary anandtiwary deleted the log-marker branch June 11, 2021 06:13
@github-actions
Copy link

Unit Test Results

    4 files  ±0  258 suites  +1   13m 36s ⏱️ - 3m 14s
929 tests +3  929 ✔️ +3  0 💤 ±0  0 ❌ ±0 
935 runs  +3  935 ✔️ +3  0 💤 ±0  0 ❌ ±0 

Results for commit d02e165. ± Comparison against base commit 83db9d9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants