Introducing Kafka Streams DevUI #20663

danielpetisme · 2021-10-11T09:49:55Z

This PR aims at providing a Dev UI for Kafka Streams.
The first element is a topology visualizer similar to https://zz85.github.io/kafka-streams-viz/.
I choose to use mermaid.js rather than a graphviz JS implementation.

The topology is fetched from the Arc container to supply a TopologyDescription object.
Then it's up to the frontend part to parse it, build a mermaid description and render the final graph.
Here you have an example

UI remarks

The textarea is reaonly, the topology visualizer aimed at rendering the current application topology not to be a general purpose Kafka Streams topology visualizer tool.
I used Mermaid default styling. Is there a Quarkus color palette? Is it worth it?
The textarea font could be reduce a bit... What do you think?

The main concern I have is regarding mermaid.js files, I had to update the vertx-http extension to bundle the JS files.
I didn't found a way to package static assets as part of the extension itself. I think for isolation/decoupling purposes it would be better to not have to update the vertx-http extension...
https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/DevUI.20custom.20static.20resources/near/256872499

I may add a "statics row on top of the current content displaying the numbers of sub-topologies, input/output topics as well as state stores.

This is the first draft so, feedbacks are more than welcome!

gsmet · 2021-10-11T10:42:44Z

Woot, this is very nice!

How complex can a topology be? I wonder if it's going to work well to have both on the same line? But maybe you can tweak things depending on the number of columns?

As for pushing the JS dependency to vertx-http, I think it has also some advantages as it would be good to reuse the same JS libraries across all Dev UI components. So I would lean towards keeping it that way.

gsmet · 2021-10-11T10:44:14Z

/cc @phillip-kruger for his opinion on the color palette. I personally think it looks pretty much OK as it is.

phillip-kruger · 2021-10-11T11:05:40Z

This looks awesome ! I think we can merge as is and tweek later if needed. You should be able to include the webjar in the runtime module of the extension and still access it from the js, but as @gsmet said, having it in vert-x module makes it easier to reuse, and also only include it when dev mode.

gsmet

I added a few comments.

gsmet · 2021-10-11T12:09:05Z

...eams/deployment/src/main/java/io/quarkus/kafka/streams/deployment/KafkaStreamsProcessor.java

@@ -53,7 +54,7 @@ void build(BuildProducer<FeatureBuildItem> feature,

        registerClassesThatAreLoadedThroughReflection(reflectiveClasses, launchMode);
        registerClassesThatAreAccessedViaJni(jniRuntimeAccessibleClasses);
-        addSupportForRocksDbLib(nativeLibs, config);
+        //        addSupportForRocksDbLib(nativeLibs, config);


My bad... I was having issue to use the 999-SNAPSHOT version and commenting this line was a workaroung.

gsmet · 2021-10-11T12:09:56Z

...nsions/kafka-streams/deployment/src/main/resources/dev-templates/kafka-streams-topology.html

@@ -0,0 +1,178 @@
+{#include main fluid=true}
+{#style}
+textarea {


Better specify a specific style. You never know what we will add later.

Done I use the #topology-description CSS id to apply the style.

gsmet · 2021-10-11T12:11:12Z

...eams/runtime/src/main/java/io/quarkus/kafka/streams/runtime/TopologyDescriptionSupplier.java

+
+    @Override
+    public TopologyDescription get() {
+        Topology topology = Arc.container().instance(Topology.class).get();


Are you sure the topology is always produced? What if we just added the extension and haven't configured anything yet? This is a case we need to handle.

You're right. Now I return the Topology object and test its presence (initially I thought of returning the String version of the topology but was not convenient for upcoming features).
Actually returning the Topology object makes much more sense.

gsmet · 2021-10-11T12:11:27Z

...nsions/kafka-streams/deployment/src/main/resources/dev-templates/kafka-streams-topology.html

+                    <label for="topology-description" class="col-form-label h1">Kafka Streams Topology</label>
+                    <textarea id="topology-description" readonly
+                              class="form-control-plaintext p-3 mb-5 bg-white r border rounded"
+                              rows='{info:topology.toString().split("\r\n|\r|\n").length} + 1'>


What if the topology is not yet defined?

Fixed.
an error Message is displayed

gsmet · 2021-10-11T12:11:50Z

...nsions/kafka-streams/deployment/src/main/resources/dev-templates/kafka-streams-topology.html

+            </form>
+        </div>
+        <div class="col-5">
+            <div id="graph">


I would use a more specific id (topology-graph?).

gsmet · 2021-10-11T13:12:26Z

...s/kafka-streams/runtime/src/main/java/io/quarkus/kafka/streams/runtime/TopologySupplier.java

+
+    @Override
+    public Topology get() {
+        return Arc.container().instance(Topology.class).get();


So you're always producing a topology even if nothing is configured? Because if not, there's a good chance this will fail.

Can you confirm that Arc.container().instance(Topology.class) will return null I nothing is configured?
I tested by disabling @Produces on an application.
Do you think it's enough?
Do you see any edge cases?

No it's OK, I thought this method was throwing an exception if not present but it simply returns null. So as long as you have taken into account the null case, we're good.

danielpetisme · 2021-10-11T13:13:25Z

Thanks for the review. I introduced an error message when the Topology is absent.

Feel free to proposed other wordings. I wanted to be explicit on the 2 main steps required to build a topology:

Having an application scoped bean
Having a producer method returning the topology.

What do you think?

danielpetisme · 2021-10-11T13:16:26Z

@gsmet @phillip-kruger I'm thinking of an alternative way to display the absence of the topology.
Rather than displaying an error message, I could simply not present the Topology link in the Kafka Streams tile.
In terms of DevX, it sounds maybe more pragmatic but it would prevent developers to have the hints I display in the error message (ie ApplicationScoped + Produce).
Which solution do you prefer?

phillip-kruger · 2021-10-11T13:28:00Z

@danielpetisme - I like what you have now, I don't think that is an error message, but rather an info message. Nice and clear.

danielpetisme · 2021-10-11T15:26:04Z

@gsmet I will let you hit the"Resolve conversation" button wherever it makes sense.

quarkus-bot · 2021-10-11T18:18:37Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `9eea828`

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Upload gc.log`	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 17

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/vault-agroal

📦 integration-tests/vault-agroal

✖ io.quarkus.vault.AgroalVaultITCase. - More details - Source on GitHub

java.lang.RuntimeException: Error waiting for test resource future to finish.
	at io.quarkus.test.common.TestResourceManager.waitForAllFutures(TestResourceManager.java:151)
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:136)

✖ io.quarkus.vault.AgroalVaultKv1ITCase. - More details - Source on GitHub

java.lang.RuntimeException: Error waiting for test resource future to finish.
	at io.quarkus.test.common.TestResourceManager.waitForAllFutures(TestResourceManager.java:151)
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:136)

✖ io.quarkus.vault.VaultKv1ITCase. - More details - Source on GitHub

java.lang.RuntimeException: Error waiting for test resource future to finish.
	at io.quarkus.test.common.TestResourceManager.waitForAllFutures(TestResourceManager.java:151)
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:136)

geoand · 2021-10-12T09:51:34Z

Awesome stuff!

Can we backport it to 2.4?

gsmet

I spotted a small remaining issue. Once fixed, I think we should merge it.

gsmet · 2021-10-12T11:29:42Z

...eams/deployment/src/main/java/io/quarkus/kafka/streams/deployment/KafkaStreamsProcessor.java

@@ -32,6 +32,7 @@
 import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.RuntimeReinitializedClassBuildItem;
 import io.quarkus.deployment.pkg.NativeConfig;
+import io.quarkus.kafka.streams.runtime.*;


This needs to go away. All the changes in this file are useless.

danielpetisme · 2021-10-12T13:48:55Z

@gsmet @geoand @phillip-kruger
I made a small variant, what do you think?
(for now it's only a mockup is not part of the current PR)
I'm not sure about it.

phillip-kruger · 2021-10-12T14:17:36Z

I like it

geoand · 2021-10-12T14:19:56Z

👍🏼

danielpetisme · 2021-10-12T14:35:15Z

Ok so now I have to dev it 😄
if there is any time constraints, feel free to merge the PR as is and I'll open a new one with the UI upgrade.

geoand · 2021-10-12T14:36:56Z

I think it's something we will want in 2.4, which means we would need it in in the next few days

gsmet · 2021-10-12T15:29:19Z

That looks cool! Please squash everything once done. Thanks!

quarkus-bot · 2021-10-12T19:20:12Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `bdf8e26`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Upload gc.log`	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 17

danielpetisme · 2021-10-13T07:21:54Z

Hello dream team,
Here is the final version of the PR.
@geoand I saw you have the "noteworthy" label. I attached clean screenshots if you want to add them in release notes

I have other ideas in mind regarding Kafka Streams Dev UI, stay tuned ;-)

geoand · 2021-10-13T07:24:55Z

Looks great! I marked it for inclusion into 2.4.

Thanks a lot for this!

phillip-kruger · 2021-10-13T07:41:09Z

very nice !

Comments addressed

quarkus-bot · 2021-10-13T11:55:33Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `ccedcec`

Status	Name	Failures	Logs	Raw logs
✖	Devtools Tests - JDK 11 Windows	⚠️ Check →	Logs	Raw logs
✖	Gradle Tests - JDK 11 Windows	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 17
✖	Maven Tests - JDK 11 Windows	⚠️ Check →	Logs	Raw logs
✖	Native Tests - Windows - hibernate-validator	⚠️ Check →	Logs	Raw logs

danielpetisme · 2021-10-13T13:56:02Z

The CI is failing I'm not sure it's due to the code I introduced...
Is it "normal"? Should I do something else?

geoand · 2021-10-13T14:02:55Z

Seems like some of of glitch... Mind rebasing on main please?

quarkus-bot · 2021-10-13T23:17:11Z

Failing Jobs - Building `f8b89c5`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Upload gc.log`	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 17
⌛	MicroProfile TCKs Tests	`Verify`	⚠️ Check →	Logs	Raw logs

quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/kafka-streams area/vertx labels Oct 11, 2021

phillip-kruger approved these changes Oct 11, 2021

View reviewed changes

gsmet requested changes Oct 11, 2021

View reviewed changes

danielpetisme force-pushed the devservices-kafkastreams-toplogy-viewer branch from 912d14a to 9eea828 Compare October 11, 2021 13:08

gsmet reviewed Oct 11, 2021

View reviewed changes

geoand added the release/noteworthy-feature label Oct 12, 2021

geoand requested a review from gsmet October 12, 2021 10:31

gsmet previously requested changes Oct 12, 2021

View reviewed changes

danielpetisme force-pushed the devservices-kafkastreams-toplogy-viewer branch from 9eea828 to bdf8e26 Compare October 12, 2021 13:53

danielpetisme force-pushed the devservices-kafkastreams-toplogy-viewer branch from bdf8e26 to 76cf3b4 Compare October 13, 2021 07:16

geoand added the triage/backport label Oct 13, 2021

danielpetisme force-pushed the devservices-kafkastreams-toplogy-viewer branch from 76cf3b4 to ccedcec Compare October 13, 2021 07:28

phillip-kruger added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 13, 2021

geoand removed area/vertx area/dependencies Pull requests that update a dependency file labels Oct 13, 2021

Introducing Kafka Streams DevUI

f8b89c5

danielpetisme force-pushed the devservices-kafkastreams-toplogy-viewer branch from ccedcec to f8b89c5 Compare October 13, 2021 18:49

quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/vertx labels Oct 13, 2021

geoand merged commit d1f6536 into quarkusio:main Oct 14, 2021

quarkus-bot bot added this to the 2.5 - main milestone Oct 14, 2021

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 14, 2021

danielpetisme deleted the devservices-kafkastreams-toplogy-viewer branch October 14, 2021 08:18

aloubyansky modified the milestones: 2.5 - main, 2.4.0.Final Oct 18, 2021

aloubyansky removed the triage/backport? label Oct 18, 2021

Introducing Kafka Streams DevUI #20663

Introducing Kafka Streams DevUI #20663

Conversation

danielpetisme commented Oct 11, 2021

gsmet commented Oct 11, 2021

gsmet commented Oct 11, 2021

phillip-kruger commented Oct 11, 2021

gsmet left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielpetisme Oct 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielpetisme commented Oct 11, 2021

danielpetisme commented Oct 11, 2021

phillip-kruger commented Oct 11, 2021

danielpetisme commented Oct 11, 2021

quarkus-bot bot commented Oct 11, 2021 • edited Loading

Failing Jobs - Building 9eea828

Failures

⚙️ JVM Tests - JDK 11 #

📦 integration-tests/vault-agroal

geoand commented Oct 12, 2021 • edited Loading

gsmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielpetisme commented Oct 12, 2021 • edited Loading

phillip-kruger commented Oct 12, 2021

geoand commented Oct 12, 2021

danielpetisme commented Oct 12, 2021 • edited Loading

geoand commented Oct 12, 2021

gsmet commented Oct 12, 2021

quarkus-bot bot commented Oct 12, 2021 • edited Loading

Failing Jobs - Building bdf8e26

danielpetisme commented Oct 13, 2021 • edited Loading

geoand commented Oct 13, 2021 • edited Loading

phillip-kruger commented Oct 13, 2021

quarkus-bot bot commented Oct 13, 2021 • edited Loading

Failing Jobs - Building ccedcec

danielpetisme commented Oct 13, 2021

geoand commented Oct 13, 2021

quarkus-bot bot commented Oct 13, 2021

Failing Jobs - Building f8b89c5

danielpetisme Oct 11, 2021 •

edited

Loading

quarkus-bot bot commented Oct 11, 2021 •

edited

Loading

Failing Jobs - Building `9eea828`

geoand commented Oct 12, 2021 •

edited

Loading

danielpetisme commented Oct 12, 2021 •

edited

Loading

danielpetisme commented Oct 12, 2021 •

edited

Loading

quarkus-bot bot commented Oct 12, 2021 •

edited

Loading

Failing Jobs - Building `bdf8e26`

danielpetisme commented Oct 13, 2021 •

edited

Loading

geoand commented Oct 13, 2021 •

edited

Loading

quarkus-bot bot commented Oct 13, 2021 •

edited

Loading

Failing Jobs - Building `ccedcec`

Failing Jobs - Building `f8b89c5`