Update VS Code for Runtime Analysis Findings Pages #519

ahtrotta · 2022-11-10T22:52:31Z

Fixes #520
This PR uses webviews that are available from this PR

Overview

This PR adds webviews to the VS Code extension that provide information about findings. There are three main changes:

The Runtime Analysis tree view is now organized:

There is a Findings Overview page:

There is a Finding Details page for each finding:

Developing Locally

Build this branch of appmap-js locally.
a. git clone https:/getappmap/appmap-js.git.
b. cd appmap-js.
c. git checkout analysis-findings.
d. yarn.
e. yarn build.
Checkout this branch of vscode-appland.
Add this to the bottom of you package.json file in vscode-appland, replacing YOUR_PATH with the path to appmap-js on your machine::

"resolutions": {
    "@appland/components": "portal:<YOUR_PATH>/appmap-js/packages/components",
    "@appland/diagrams": "portal:<YOUR_PATH>/appmap-js/packages/diagrams",
    "@appland/models": "portal:<YOUR_PATH>/appmap-js/packages/models",
    "@appland/openapi": "portal:<YOUR_PATH>/appmap-js/packages/openapi",
    "@appland/scanner": "portal:<YOUR_PATH>/appmap-js/packages/scanner"
  }

Run yarn from the root of vscode-appland.
Run yarn watch from the root of vscode-appland.
Open vscode-appland in VS Code and launch the debugger extension host (press F5 or click the Run menu and then click Start Debugging).

dustinbyrne · 2022-11-30T18:15:30Z

src/services/findingsIndex.ts

+ return this.findings().filter((finding) => finding.finding.impactDomain === impactDomain);
+ }
+
+ findingsByHashV2(hash: string): ResolvedFinding[] {


Nit-pick, but I think this should just be findingsByHash - The fact that it's V2 is just an implementation detail that may change in the future.

No problem, I renamed it 👍

dustinbyrne · 2022-11-30T18:18:48Z

src/services/findingsIndex.ts

+ const findingsByRuleTitle = this.findings().filter(
+ (finding) => finding.finding.ruleTitle === ruleTitle
+ );


You can destructure the parameter here to cut down on verbosity (finding.finding). This isn't really an issue though, just an FYI.

filter(({ finding }) => finding.ruleTitle === ruleTitle)

Yeah, good point. I just changed it in a couple of places in this file.

dustinbyrne · 2022-11-30T18:20:44Z

src/services/findingsIndex.ts

+ const uniqueFindingsByHash = findingsByRuleTitle.reduce((accumulator, finding) => {
+ const hashV2 = finding.finding.hash_v2;
+
+ if (!(hashV2 in accumulator)) {
+ accumulator[hashV2] = finding;
+ }
+
+ return accumulator;
+ }, {});


If you're finding that there's a lot of duplicate logic between the extension and front-end we can consider adding a findings model to @appland/models.

dustinbyrne · 2022-11-30T18:26:15Z

src/tree/findingsTreeDataProvider.ts

+ const impactDomain = finding.finding.impactDomain;
+
+ if (impactDomain && !impactDomains.includes(impactDomain)) {
+ impactDomains.push(impactDomain);
+ }


Might be worth a case normalization here but I recognize the type is defined as 'Security' | 'Performance' | 'Maintainability' | 'Stability'.

🤷

dustinbyrne · 2022-11-30T18:32:48Z

src/webviews/findingInfoWebview.ts

+ const docPath = join(__dirname, `../node_modules/@appland/scanner/doc/rules/${dasherize(id)}.md`);
+
+ if (!fs.existsSync(docPath)) return;
+
+ // replace any carriage return with a newline
+ const content = fs.readFileSync(docPath, 'utf-8').replace(/\r\n/g, '\n');


The production build is bundled down to a single file, so this path won't exist. Instead of fs.readFileSync, this will need to be a dynamic import.

We'll need another custom type defined in types/custom.d.ts as well as a webpack loader rule for .md files. This may (?) break our tests as they use tsc compiled JavaScript (not webpack) but we'll cross that bridge when we come to it.

No worries, I just changed it so that the rule information is now retrieved from parsing appmap-findings.json instead of looking for the markdown file.

dustinbyrne

Looks great.

Remove duplicate listeners in installGuideView.js, remove extraneous 1 in installGuideWebview.ts, and convert the render arrow function in appmapView.js to a method.

…markdown file

Sometimes a location is not available. Maybe the path no longer exists in this project or the AppMap was shared from another source. Instead of `undefined`, attempt to fall back to something descriptive.

# [0.56.0](v0.55.0...v0.56.0) (2022-12-13) ### Features * Add dashboard and details pages for Runtime Analysis ([#519](#519)) ([88f3df8](88f3df8)) * Prompt the user to sign in before viewing instructions if they're not authenticated ([3827287](3827287))

appland-release · 2022-12-13T20:16:01Z

🎉 This PR is included in version 0.56.0 🎉

The release is available on:

v0.56.0
GitHub release

Your semantic-release bot 📦🚀

ahtrotta force-pushed the feat/runtime-analysis-findings-pages branch from 5604f6c to cb32a02 Compare November 18, 2022 20:53

dustinbyrne requested changes Nov 30, 2022

View reviewed changes

ahtrotta force-pushed the feat/runtime-analysis-findings-pages branch 3 times, most recently from 12d3868 to 6a5761b Compare December 2, 2022 20:46

ahtrotta marked this pull request as ready for review December 6, 2022 22:34

ahtrotta force-pushed the feat/runtime-analysis-findings-pages branch 3 times, most recently from 198e858 to 40c475d Compare December 7, 2022 00:47

ahtrotta requested a review from dustinbyrne December 7, 2022 01:04

dustinbyrne approved these changes Dec 7, 2022

View reviewed changes

ahtrotta and others added 13 commits December 9, 2022 16:42

fix: small fixes

f029622

Remove duplicate listeners in installGuideView.js, remove extraneous 1 in installGuideWebview.ts, and convert the render arrow function in appmapView.js to a method.

feat: add filtering methods to findings index

e5a302f

feat: add findings webviews

c140ee5

feat: support opening findings overview from finding details page

dcd5530

feat: include rule information in resolved findings

69ba9c2

refactor: retrieve rule information from resolved finding instead of …

7fdeddd

…markdown file

fix: remove unused imports

b55f53d

refactor: rename findingsByHash function

c38001f

refactor: use destructuring to reduce verbosity

01d4de3

test: add system tests

00f679a

chore: update scanner and components

552b9a2

Add some fallback labels to findings list items

1cb266c

Sometimes a location is not available. Maybe the path no longer exists in this project or the AppMap was shared from another source. Instead of `undefined`, attempt to fall back to something descriptive.

fix: fix broken system tests

ac9b664

dustinbyrne force-pushed the feat/runtime-analysis-findings-pages branch from 99376fb to ac9b664 Compare December 9, 2022 21:42

dustinbyrne merged commit 88f3df8 into master Dec 9, 2022

dustinbyrne deleted the feat/runtime-analysis-findings-pages branch December 9, 2022 21:56

appland-release added the released label Dec 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update VS Code for Runtime Analysis Findings Pages #519

Update VS Code for Runtime Analysis Findings Pages #519

ahtrotta commented Nov 10, 2022 •

edited

Loading

dustinbyrne Nov 30, 2022

ahtrotta Dec 1, 2022

dustinbyrne Nov 30, 2022

ahtrotta Dec 1, 2022

dustinbyrne Nov 30, 2022

dustinbyrne Nov 30, 2022 •

edited

Loading

dustinbyrne Nov 30, 2022

dustinbyrne Nov 30, 2022

ahtrotta Dec 1, 2022

dustinbyrne Dec 5, 2022

dustinbyrne left a comment

appland-release commented Dec 13, 2022

Update VS Code for Runtime Analysis Findings Pages #519

Update VS Code for Runtime Analysis Findings Pages #519

Conversation

ahtrotta commented Nov 10, 2022 • edited Loading

Overview

Developing Locally

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

dustinbyrne Nov 30, 2022 • edited Loading

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

dustinbyrne left a comment

Choose a reason for hiding this comment

appland-release commented Dec 13, 2022

ahtrotta commented Nov 10, 2022 •

edited

Loading

dustinbyrne Nov 30, 2022 •

edited

Loading