[Security Solution] modify circular deps checker to output images of circular deps graphs #75579

oatkiller · 2020-08-20T16:47:12Z

Summary

The Security Solution team has a script that tries to block circular dependencies between typescript files. When the script detects a circular dependency it will now attempt to use graphviz to output an SVG showing a visualization of the circular dependency. This should help developers understand the circular dependency.

When run with `--help`

raustin-mbp:security_solution raustin$ node scripts/check_circular_deps --help

  node scripts/check_circular_deps

  Check the Security Solution plugin for circular deps. If any are found, this will throw an Error.

  Options:
    --svg,             Output SVGs of circular dependency graphs
    --verbose, -v      Log verbosely
    --debug            Log debug messages (less than verbose)
    --quiet            Only log errors
    --silent           Don't log anything
    --help             Show this message

When there is a circular dep and no flags were passed

raustin-mbp:security_solution raustin$ node scripts/check_circular_deps
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.5.0

YOUR TYPESCRIPT VERSION: 3.9.5

Please only submit bug reports when using the officially supported version.

=============
Run this program with the --svg flag to save an SVG showing the dependency graph.
ERROR SIEM circular dependencies of imports has been found:
       - public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts

When graphviz is installed and there is a circular dep and `--svg` was passed:

raustin-mbp:security_solution raustin$ node scripts/check_circular_deps --svg
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.5.0

YOUR TYPESCRIPT VERSION: 3.9.5

Please only submit bug reports when using the officially supported version.

=============
Attempting to save SVG for circular dependency: public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts
Saved SVG: /var/folders/1r/y028x41x3kxd8jzrgf3vf3dr0000gp/T/security_solution-circular-dep-0.svg
ERROR SIEM circular dependencies of imports has been found:
       - public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts

when there are 2 circular deps and you use `--svg`

raustin-mbp:security_solution raustin$ node scripts/check_circular_deps --svg
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.5.0

YOUR TYPESCRIPT VERSION: 3.9.5

Please only submit bug reports when using the officially supported version.

=============
Attempting to save SVG for circular dependency: public/app/404.tsx,public/network/routes.tsx
Saved SVG: /var/folders/1r/y028x41x3kxd8jzrgf3vf3dr0000gp/T/security_solution-circular-dep-0.svg
Attempting to save SVG for circular dependency: public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts
Saved SVG: /var/folders/1r/y028x41x3kxd8jzrgf3vf3dr0000gp/T/security_solution-circular-dep-1.svg
ERROR SIEM circular dependencies of imports has been found:
       - public/app/404.tsx,public/network/routes.tsx
       - public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts

when there are two circular deps and no flags were passed

raustin-mbp:security_solution raustin$ node scripts/check_circular_deps
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.5.0

YOUR TYPESCRIPT VERSION: 3.9.5

Please only submit bug reports when using the officially supported version.

=============
Run this program with the --svg flag to save an SVG showing the dependency graph.
ERROR SIEM circular dependencies of imports has been found:
       - public/app/404.tsx,public/network/routes.tsx
       - public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts

When there are circular deps, `--svg` was passed, and graphviz isn't installed

raustin-mbp:security_solution raustin$ node scripts/check_circular_deps --svg
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.5.0

YOUR TYPESCRIPT VERSION: 3.9.5

Please only submit bug reports when using the officially supported version.

=============
Attempting to save SVG for circular dependency: public/timelines/store/timeline/types.ts,public/timelines/store/timeline/model.ts,public/common/store/types.ts
ERROR UNHANDLED ERROR
ERROR Error: Graphviz could not be found. Ensure that "gvpr" is in your $PATH.
      Error: Command failed: gvpr -V
      /bin/sh: gvpr: command not found

          at exec.catch (/Users/raustin/Code/elastic/kibana/node_modules/madge/lib/graph.js:36:10)

Checklist

Doesn't do anything dramatic when graphviz is missing (as it will be for most users and CI)
Works when there is no circular dep detected
Works when multiple circular dependencies are detected.
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

oatkiller · 2020-08-20T16:48:05Z

x-pack/plugins/security_solution/scripts/check_circular_deps/run_check_circular_deps_cli.js

@@ -34,6 +34,29 @@ run(

    const circularFound = result.circular();
    if (circularFound.length !== 0) {
+      let count = 0;


Should I write one image per circular that is found?

Should I only run this logic if a flag is passed to the script?

Any other good ideas?

I think having it behind a flag makes sense. Depending on the output I can go either way for individual vs grouped images

This looks great thanks!

michaelolo24 · 2020-08-20T20:10:17Z

Can you add an example of what this outputs from your other PR?

FrankHassanabad

Nice! 👍

dplumlee

wow nice markdown

kibanamachine · 2020-08-20T22:52:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d3c5b83

Build metrics

✅ unchanged

History

💚 Build #69460 succeeded 0ee93a87b203c5e4d82ba4226ab617e2611dde86

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…circular deps graphs (elastic#75579)

…circular deps graphs (#75579) (#75660)

* master: (71 commits) [Lens] Show 'No data for this field' for empty field in accordion (elastic#73772) Skip failing lens test Configure ScopedHistory consistenty regardless of URL used to mount app (elastic#75074) Fix returned payload by "search" usage collector (elastic#75340) [Security Solution] Fix missing key error (elastic#75576) Upgrade EUI to v27.4.1 (elastic#75240) Update datasets UI copy to data streams (elastic#75618) [Lens] Register saved object references (elastic#74523) [DOCS] Update links to Beats documentation (elastic#70380) [Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData (elastic#75616) [Usage Collection Schemas] Remove Legacy entries (elastic#75652) [Dashboard First] Lens Originating App Breadcrumb (elastic#75470) Improve login UI error message. (elastic#75642) [Security Solution] modify circular deps checker to output images of circular deps graphs (elastic#75579) [Data Telemetry] Add index pattern to identify "meow" attacks (elastic#75163) Migrate CSP usage collector to `kibana_usage_collection` plugin (elastic#75536) [Console] Get ES Config from core (elastic#75406) [Uptime] Add delay in telemetry test (elastic#75162) [Lens] Use index pattern service instead saved object client (elastic#74654) Embeddable input (elastic#73033) ...

…circular deps graphs (elastic#75579)

oatkiller requested a review from XavierM August 20, 2020 16:47

oatkiller requested review from a team as code owners August 20, 2020 16:47

oatkiller commented Aug 20, 2020

View reviewed changes

modify circular deps checker to output images of circular deps graphs

d3c5b83

oatkiller force-pushed the circular-deps-checker-image branch from 0ee93a8 to d3c5b83 Compare August 20, 2020 21:03

oatkiller added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 20, 2020

FrankHassanabad approved these changes Aug 20, 2020

View reviewed changes

dplumlee approved these changes Aug 20, 2020

View reviewed changes

XavierM approved these changes Aug 21, 2020

View reviewed changes

michaelolo24 approved these changes Aug 21, 2020

View reviewed changes

oatkiller merged commit da0da4c into elastic:master Aug 21, 2020

oatkiller mentioned this pull request Aug 21, 2020

[7.x] [Security Solution] modify circular deps checker to output images of circular deps graphs (#75579) #75660

Merged

oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Aug 21, 2020

[Security Solution] modify circular deps checker to output images of …

75b6a2e

…circular deps graphs (elastic#75579)

oatkiller pushed a commit that referenced this pull request Aug 21, 2020

[Security Solution] modify circular deps checker to output images of …

622bf2d

…circular deps graphs (#75579) (#75660)

thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Aug 21, 2020

[Security Solution] modify circular deps checker to output images of …

2f1a03e

…circular deps graphs (elastic#75579)

dplumlee mentioned this pull request Sep 22, 2020

Create tool to identify and visualize circular imports between plugins #78162

Closed

oatkiller deleted the circular-deps-checker-image branch March 31, 2022 11:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] modify circular deps checker to output images of circular deps graphs #75579

[Security Solution] modify circular deps checker to output images of circular deps graphs #75579

oatkiller commented Aug 20, 2020 •

edited

Loading

oatkiller Aug 20, 2020

michaelolo24 Aug 20, 2020

michaelolo24 Aug 21, 2020

michaelolo24 commented Aug 20, 2020

FrankHassanabad left a comment

dplumlee left a comment

kibanamachine commented Aug 20, 2020

[Security Solution] modify circular deps checker to output images of circular deps graphs #75579

[Security Solution] modify circular deps checker to output images of circular deps graphs #75579

Conversation

oatkiller commented Aug 20, 2020 • edited Loading

Summary

Checklist

For maintainers

oatkiller Aug 20, 2020

Choose a reason for hiding this comment

michaelolo24 Aug 20, 2020

Choose a reason for hiding this comment

michaelolo24 Aug 21, 2020

Choose a reason for hiding this comment

michaelolo24 commented Aug 20, 2020

FrankHassanabad left a comment

Choose a reason for hiding this comment

dplumlee left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 20, 2020

💚 Build Succeeded

Build metrics

History

oatkiller commented Aug 20, 2020 •

edited

Loading