Skip to content
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

Add catalog graph plugin #7185

Merged
merged 4 commits into from
Sep 21, 2021
Merged

Add catalog graph plugin #7185

merged 4 commits into from
Sep 21, 2021

Conversation

Fox32
Copy link
Contributor

@Fox32 Fox32 commented Sep 15, 2021

Welcome to the catalog graph plugin! The catalog graph visualizes the relations
between entities, like ownership, grouping or API relationships.

The plugin comes with these features:

  • EntityCatalogGraphCard:
    A card that displays the directly related entities to the current entity.
    This card is for use on the entity page.
    The card can be customized, for example filtering for specific relations.

  • CatalogGraphPage:
    A standalone page that can be added to your application providing a viewer for your entities and their relations.
    The viewer can be used to navigate through the entities and filter for specific relations.
    You can access it from the EntityCatalogGraphCard.

  • EntityRelationsGraph:
    A react component that can be used to build own customized entity relation graphs.

It's similar to the system diagram and requests like #1204 and is based on suggestions from here.

image

Quick Demo

Entity Card:

CatalogGraphCard.mov

Standalone Viewer:

CatalogGraphStandaloneViewerV2.mov

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@changeset-bot
Copy link

changeset-bot bot commented Sep 15, 2021

⚠️ No Changeset found

Latest commit: 0758089

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

"@backstage/theme": "^0.2.10",
"@material-ui/core": "^4.12.2",
"@material-ui/icons": "^4.9.1",
"@material-ui/lab": "4.0.0-alpha.47",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This version is different from what we use elsewhere. I had some trouble with the autocomplete that is resolved here. We could update to this version everywhere, I don't think that this bump included any breaking changes. On the other side we could just sit it out and wait for #7094 that includes the autocomplete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actual, the build seems to fail because my version is to new #7196

@Fox32 Fox32 marked this pull request as ready for review September 15, 2021 09:01
@Fox32 Fox32 requested a review from a team as a code owner September 15, 2021 09:01
@freben
Copy link
Member

freben commented Sep 15, 2021

surprise-australiasgottalent

@Fox32 Fox32 force-pushed the feat/catalog-graph branch 2 times, most recently from 624893d to d979a1a Compare September 15, 2021 09:26
@freben
Copy link
Member

freben commented Sep 15, 2021

The ownedBy graphs showing in the video tell me that there's room for a future addition, that lets you toggle making relations into nodes too. Maybe with a different color. (A) -> (ownedBy / ownerOf) -> (B) and (C) and (D)

gets the fanout a bit under control and clarifies things when there are 8 relations of one type and three of another, etc

@Fox32
Copy link
Contributor Author

Fox32 commented Sep 15, 2021

The ownedBy graphs showing in the video tell me that there's room for a future addition, that lets you toggle making relations into nodes too. Maybe with a different color. (A) -> (ownedBy / ownerOf) -> (B) and (C) and (D)

Yeah, the idea is to have more rendering modes over time that everyone can choose how he want the display it. I already played around with toggling. The bad thing about it is that the graph can quite drastically change when you "open" a relation. That felt quite hard to use and keep track of the context then. The idea of grouping multiple entities into a single node is neat too 🤔

@freben
Copy link
Member

freben commented Sep 15, 2021

Ah but, even without the opening and closing. Even as a static thing, replacing arrows with arrow-node-arrow

@freben
Copy link
Member

freben commented Sep 15, 2021

And I didn't mean to collapse entities actually, even though that sounds interesting :) I was just not gonna make a nicer ascii graph, I meant three arrows to B, C, D as three separate nodes :)

@Fox32
Copy link
Contributor Author

Fox32 commented Sep 15, 2021

Ah but, even without the opening and closing. Even as a static thing, replacing arrows with arrow-node-arrow

Ah, yeah. Also played around with that idea. Can't show a screenshot (to much internal stuff on it), but I had two issues:

  • The Dependency Graph component has some trouble layouting everything with the additional nodes. But that could be solved with another renderer
  • I wanted to collapse multiple relations into one relation node, but there you need an intelligent heuristic to decide which to merge and which not (because they can come from both sides…) The problem is easier to see than to explain 😆

@adamdmharvey
Copy link
Member

Just wanted to stop in to say: well done, @Fox32 ! Beautiful rendering, and wonderful feature to drive up discoverability of the engineering output. This drives such value. Kudos!!!

@andrewthauer
Copy link
Collaborator

🎉 Amazing! Can't wait to try this out!

@julioz
Copy link
Contributor

julioz commented Sep 16, 2021

I like this a lot, we have implemented something similar internally. I would love to give it a try in our backstage instance 👍

What has been very valuable for us is to also expose "metrics" out of the dependency graph to spot "non-standard component relationships", for example, suggesting to break down a component's responsibility into multiple sub-components when we detect > X dependsOn (where X is an arbitrary number).
On top of that, the filtering capability has been useful (when paired with component annotations) for us to also identify "component groups", for example to answer the question "show me all components that expose at least one route to the public internet" or "show me all components that connect to this specific database" (and thus identify integration via DB instead of via API), etc.

If I can propose (yet another 😆) feature idea that we also have internally, is the capability to "simulate edition" of the graph. An engineer can then more easily sketch ideas of how the "system landscape" should be. At the end of the process, we allow them to exporting sketches to common graph description languages (like dot or mermaid).

@regicsolutions
Copy link

Love it!

@freben
Copy link
Member

freben commented Sep 16, 2021

Also, can't wait to combine this with like symbology or similar to give status overviews at a glance. For example, looking at your local "neighborhood" of nodes and seeing that one of your dependencies has a big warning triangle on it because it isn't updating properly or has some kind of alert going

Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
@Fox32
Copy link
Contributor Author

Fox32 commented Sep 17, 2021

CI is green now 🙂

Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

NICE!

.changeset/wicked-pugs-speak.md Outdated Show resolved Hide resolved
RELATION_DEPENDS_ON,
RELATION_DEPENDENCY_OF,
],
}}
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to have a defaults mechanism (kinda like the EntityTable columns or something like that)? This is a bit much to have to slap into the example app :) So if you don't give a selectedKinds it'll be CatalogGraphPage.defaultSelectedKinds or so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, it's a bit complicated here. Not providing selectedKinds means "all selected kind" or "no kinds filter". So I can't set a default for undefined (same for relations). On the other hand, setting these is just a matter of taste, we could also go with "no filter" in the example app and readme.

<EntityAboutCard variant="gridItem" />
</Grid>

<Grid item md={6} xs={12}>
<EntityCatalogGraphCard variant="gridItem" maxHeight={400} />
Copy link
Member

Choose a reason for hiding this comment

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

i saw some vertical jumping in the demo video; i still think these might be best to have with a fixed height, and possibly below the fold (under some other stuff) so it can load in peace and maybe even does its first render out of sight. That way it has a bit more vertical and horizontal space to play with too

Copy link
Contributor Author

@Fox32 Fox32 Sep 20, 2021

Choose a reason for hiding this comment

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

Yeah, fixed height is better. About the jumping inside the diagram: I wonder if that is something we can fix in the dependency graph. There is a debounce that where the timeout could be reduced to make it more responsive. But maybe we have to switch to another library under the hood anyway, as dagre seems to be kind of dead.

Copy link

Choose a reason for hiding this comment

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

@Fox32 do we have an issue for the switch?

packages/app/src/components/catalog/EntityPage.tsx Outdated Show resolved Hide resolved
kinds,
relations,
direction,
maxHeight,
Copy link
Member

Choose a reason for hiding this comment

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

is it worth having something like cardStyles or whatnot, that has the proper css type or similar? Or even, taking it farther, essentially cardProps: InfoCardProps? This is one of those components that's such a thin wrapper, and immediately somebody comes along and asks "what if i want to change the title of the card". So, not to rant too much, but sometimes I'm not sure what the right "abstraction level" is when we make components, and perhaps especially so for cards

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think a lot of people might want to customize the card. For now I hoped making the card a very tiny wrapper would be sufficient and people can just copy it. On the other hand, where do you stop. Would it be better to just pass all card properties through? What about the ones that I modify in my card, like deepLink or noPadding?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it's a complicated subject. I hope we'll get the time soon to work on clarifying the app-plugin contract so to speak

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I worked with mui 5 lately for another project, maybe they have some patterns we can take over. Like having the xs property on all components.

/**
* Optional kind of the entity.
*/
kind?: string;
Copy link
Member

Choose a reason for hiding this comment

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

would type?: string fit well in here too?

Copy link
Member

Choose a reason for hiding this comment

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

especially if that together with kind could be used to draw custom icons in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not used right now, but yeah, could be added later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also pass the full entity in and make the nodes more customizable. But that is maybe also something for later.

plugins/catalog-graph/src/plugin.ts Outdated Show resolved Hide resolved
@Fox32 Fox32 force-pushed the feat/catalog-graph branch 2 times, most recently from a8ab0d3 to ae0446c Compare September 20, 2021 11:19
Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

Let's ship this!

@Fox32 Fox32 merged commit f4004cd into backstage:master Sep 21, 2021
@Fox32 Fox32 deleted the feat/catalog-graph branch September 21, 2021 09:14
@dudarau
Copy link

dudarau commented Jan 5, 2022

@freben @Fox32 do we need to get rid of dagre? Do we have a ticket for this? If you don't mind I can work on it.

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.

None yet

7 participants