Skip to content

Conversation

@eh-am
Copy link
Collaborator

@eh-am eh-am commented Sep 12, 2022

Render annotations created via the backend (creating annotation via UI will come in a new PR):
Kapture 2022-09-14 at 17 28 48

There's still some visual glitches that I would like to address (in separate PRs) before considering this feature done:

To create an annotation

curl -X POST -d '{ "appName": "pyroscope.server.cpu", "content": "hi i am an annotation", "timestamp": 1663187023 }' localhost:4040/api/annotations

You can get the current timestamp with date +%s

@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2022

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
webapp/public/assets/app.js 431.23 KB (+0.16% 🔺) 8.7 s (+0.16% 🔺) 4.2 s (-5.02% 🔽) 12.8 s
webapp/public/assets/app.css 16.69 KB (+0.52% 🔺) 334 ms (+0.52% 🔺) 0 ms (+100% 🔺) 334 ms
webapp/public/assets/styles.css 9.49 KB (0%) 190 ms (0%) 0 ms (+100% 🔺) 190 ms
packages/pyroscope-flamegraph/dist/index.js 92.08 KB (0%) 1.9 s (0%) 1.4 s (+10.92% 🔺) 3.2 s
packages/pyroscope-flamegraph/dist/index.node.js 91.97 KB (0%) 1.9 s (0%) 555 ms (-23.27% 🔽) 2.4 s
packages/pyroscope-flamegraph/dist/index.css 7.32 KB (0%) 147 ms (0%) 0 ms (+100% 🔺) 147 ms

@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Base: 66.31% // Head: 66.62% // Increases project coverage by +0.31% 🎉

Coverage data is based on head (4ca2335) compared to base (a557183).
Patch coverage: 93.75% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1489      +/-   ##
==========================================
+ Coverage   66.31%   66.62%   +0.31%     
==========================================
  Files         139      143       +4     
  Lines        4755     4843      +88     
  Branches     1288     1306      +18     
==========================================
+ Hits         3153     3226      +73     
- Misses       1597     1612      +15     
  Partials        5        5              
Impacted Files Coverage Δ
webapp/javascript/services/render.ts 17.31% <75.00%> (+0.14%) ⬆️
...pt/components/TimelineChart/Annotation.module.scss 61.54% <88.89%> (ø)
...pp/javascript/components/TimelineChart/markings.ts 92.86% <92.86%> (ø)
...javascript/components/TimelineChart/Annotation.tsx 96.97% <96.97%> (ø)
webapp/javascript/models/annotation.ts 100.00% <100.00%> (ø)
webapp/javascript/redux/reducers/continuous.ts 26.41% <100.00%> (+0.20%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@eh-am
Copy link
Collaborator Author

eh-am commented Sep 12, 2022

/create-server

@Rperry2174
Copy link
Contributor

Long term I think it would be cool to make these annotations look something like this:
mockup_annotations_00

Where there are multiple "types" depending on the type of annotations

timestamp: number
) {
const f = annotations.find(
(a) => Math.abs(a.timestamp - timestamp) < THRESHOLD
Copy link
Contributor

Choose a reason for hiding this comment

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

as I understand you've added THRESHOLD to have kind of distance of watching markings, but does it the same for all markings? how does it work you have 3 annotations near each other? for example, on timestamps 15:30:25, 15:30:27, 15:30:28?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's a great point actually, i didn't think of this at all, let me see what i can do

Copy link
Collaborator Author

@eh-am eh-am Sep 15, 2022

Choose a reason for hiding this comment

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

added a solution, although not perfect, hopefully will fix in #1505

Copy link
Contributor

Choose a reason for hiding this comment

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

@eh-am, this problem might be a bit deeper, I have no idea why, but these annotation marks (#1505) looks like vertical selection lines in left/right timelines on ComparisonView page. Just take look at them more attentively. Dotted line + black draggable area. I added this appearance only for left/right single timeline selection lines, is there chance you pass some incorrect props? any kind of props conflict? But no idea what they are doing in annotations PR.
image

@eh-am eh-am marked this pull request as ready for review September 14, 2022 21:02
@Rperry2174
Copy link
Contributor

/create-server

@eh-am
Copy link
Collaborator Author

eh-am commented Sep 15, 2022

/create-server

Copy link
Contributor

@Rperry2174 Rperry2174 left a comment

Choose a reason for hiding this comment

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

Lgtm

@eh-am eh-am merged commit 0e57137 into main Sep 15, 2022
@eh-am eh-am deleted the feat/annotations-ui branch September 15, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants