🎁 Pretty print cloud events via wasm #24

cardil · 2023-04-17T17:13:10Z

Changes

🎁 Pretty print cloud events via wasm

Fixes #15

Found issues

Mandrel image fails to build native image with wasmtime-java (GLIBC_2.32 not found) quarkusio/quarkus-images#227
Invalid default data for parsing in binary mode cloudevents/sdk-javascript#538

Conflicts fixed: - quarkus/src/main/java/com/redhat/openshift/knative/showcase/events/Endpoint.java

openshift-ci · 2023-04-17T17:13:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [cardil]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…ty-print

mgencur

@cardil Hey, I have just put a few comments to get some clarification.

I'm not familiar with WASM but I suppose the code that actually does the formatting of the received cloud events is not in this PR. It's somewhere external? Looks like this PR loads and calls the WASM modele that was created somewhere else.

mgencur · 2023-04-20T11:34:21Z

expressjs/scripts/extract-webjar.js

+ * @callback Log
+ * @param {string} message
+ * @param {...any} args
+ */


Does this belong to any function? If not, let's remove it.

It is used on line 69. This is done, so you get a code assist in IDE, like vscode.

mgencur · 2023-04-20T11:58:56Z

frontend/scripts/webjar.ts

+    const tmpDir = os.tmpdir()
+    const tmp = await fs.mkdtemp(`${tmpDir}/wasm-oci-`)
+    const reg = new WasmRegistry(tmp)
+    const image = Image.parse('quay.io/cardil/cloudevents-pretty-print@sha256:134269c7090bcb923d8b8764920bb25923e38411413eac7e4240524f1a40dc74')


If I understand it correctly, you created the cloudevents-pretty-print.wasm in advance, then put it in the image above and the application is now pulling the image and consuming the wasm file from it?
Where/how was the cloudevents-pretty-print.wasm created? Where's the code?

Yes. The code is here: https://github.com/cardil/cloudevents-pretty-print

I didn't put the whole WASM project in this repo, so not to cloud the project even more. I could transfer it to https://github.com/openshift-knative org, if you think it's a good idea.

However, I doubt the code in https://github.com/cardil/cloudevents-pretty-print will change in any significant way, so I don't know is that worth it.

mgencur · 2023-04-20T12:01:20Z

quarkus/pom.xml

@@ -105,6 +110,16 @@
      <artifactId>frontend</artifactId>
      <version>main</version>
    </dependency>
+    <dependency>
+      <groupId>com.redhat.openshift.knative.showcase</groupId>
+      <artifactId>cloudevents-pp-wasm</artifactId>


Where's the source code for this dependency?

The code is here: https://github.com/cardil/cloudevents-pretty-print (in Rust). It gets built onto a WASM module, and published on quay.io/cardil/cloudevents-pretty-print

The script in frontend app, downloads that WASM module from the registry, and packages it as jar onto local ~/.m2 directory.

That's done, so it is easily available for Java app (same as web files were provided before)

cardil · 2023-04-20T12:45:37Z

Here's a quick performance check of WASM pretty printer in Quarkus:

mgencur · 2023-04-21T09:08:40Z

Hi @cardil , it's good to see WASM in action and how it can be glued together. Thanks for that. The code looks good but it also brings huge complexity to this showcase application that doesn't pay off.

If I exclude the package-lock.json from my consideration it's still about 400 lines of glue code just to load and use the WASM module. This source code of this WASM module is also in a different repository, written in a different language and also pulled through an image that was published to your own image registry (the personal registry is a minor details, the complexity of pulling this through image will remain when moving under openshift-knative/showcase). The actual code that does the formatting of the cloud event is just about 40-50 lines in RUST and as you said it's not likely that this code will change much. Personally, I'd prefer writing this formatting of event in both Java and JS - for both backends separately, or maybe do it on frontend that is shared by both backends. Even if loading a WASM module is quick it can hardly show the effect with this short event formatting stuff. It would be more reasonable for big libraries being loaded.

I'd prefer if someone else could give their opinion as well before merging this.

rhuss · 2023-04-25T09:17:49Z

Thanks, @cardil, for this experiment, it looks interesting and might pave the way for future experimentation with WASM (e.g., how to put a runtime best in the image).

However, I think we should not add this to this showcase image. Let me explain why:

We should be clear about the purpose of this image. It's meant to be used as-is in smoke tests or demos, to verify and demonstrate Knative, and maybe also for debugging. So the scope is, by intention, quite limited. It is not meant to be a blueprint for creating showcase-like applications, so the user of this image will not look into the source (at least, this is not the goal). Of course, anybody can look and learn from the source like anybody can look and learn from the Knative backend code. But that is not the intention; we don't optimize for that. Also, the app should not transform into a Swiss army knife or Frankenstein, the art is to stay close to the original focus.
The nice thing about container images is that somebody creates them and somebody is using them, so they build an excellent handover point. We care about the users of these images, not about the ones who are creating those (which are mostly us).
Since our team (that changes over time) maintains that image, one non-functional goal is to keep the maintenance costs as low as possible for the features it supports. This implies:
- As few dependencies as possible to avoid rebuilds because of CVEs and other upstream changes.
- No extra functionality that is not needed for the original use case, especially none that is not visible to the end-user (like a WASM plugin architecture).
- Optimize by abstraction only if there is a clear benefit for the future. Since I don't expect that showcase to grow much more (and indeed, it shouldn't!), adding such an abstraction here is overkill.

@cardil, sorry that I don't have a more positive answer. I definitely see the value in experimenting and learning new technologies like WASM. Still, only because we have (supposedly) a new golden hammer doesn't mean everything is now a nail. We can consider moving this experimentation to another playground.

One example that I see is to try to get the queue-proxy functionality into a WASM module that can be used as Dapr middleware. I think this is more natural instead of adding a user-invisible addition of implementation details that adds more dependencies (the WASM deps are not even 100% stable, I would guess) and does not have any benefits when it comes to maintenance.

cardil · 2023-05-11T10:14:40Z

I'll prepare simpler implementation

/close

openshift-ci · 2023-05-11T10:14:43Z

@cardil: Closed this PR.

In response to this:

I'll prepare simpler implementation

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mgencur · 2023-05-11T10:47:02Z

Thanks @cardil

PoC of WASM pretty print of CloudEvents

77ce55f

Conflicts fixed: - quarkus/src/main/java/com/redhat/openshift/knative/showcase/events/Endpoint.java

openshift-ci bot added the do-not-merge/work-in-progress label Apr 17, 2023

openshift-ci bot added the approved label Apr 17, 2023

cardil added kind/enhancement New feature or request and removed approved labels Apr 17, 2023

Using wasm-oci to download WASM from OCI registries

b4022df

cardil force-pushed the feature/wasm-pretty-print branch from 9dbea4c to b4022df Compare April 17, 2023 17:18

openshift-ci bot added the approved label Apr 17, 2023

Extract multiple webjars

bd18b84

cardil force-pushed the feature/wasm-pretty-print branch from cf0d62f to 3c6191d Compare April 18, 2023 18:10

Pretty printing the CE in Express using WASI

33d820d

cardil force-pushed the feature/wasm-pretty-print branch from 3c6191d to ef4aff1 Compare April 18, 2023 23:18

Include error from wasm module in pretty print error message

bff8677

cardil force-pushed the feature/wasm-pretty-print branch from ef4aff1 to bff8677 Compare April 18, 2023 23:20

cardil added 2 commits April 19, 2023 01:28

Remove useless jsdoc

68d46fd

Merge remote-tracking branch 'upstream/latest' into feature/wasm-pret…

f0031ab

…ty-print

This was referenced Apr 19, 2023

Mandrel image fails to build native image with wasmtime-java (GLIBC_2.32 not found) quarkusio/quarkus-images#227

Open

Invalid default data for parsing in binary mode cloudevents/sdk-javascript#538

Closed

cardil added 3 commits April 19, 2023 13:45

Print WASM error message if any in Quarkus

5afc41c

Central logging for Express (par with Quarkus)

117b06d

Fixing Quarkus native build

fd54cc0

cardil marked this pull request as ready for review April 19, 2023 23:08

openshift-ci bot removed the do-not-merge/work-in-progress label Apr 19, 2023

openshift-ci bot requested review from aliok and lberk April 19, 2023 23:08

mgencur reviewed Apr 20, 2023

View reviewed changes

cardil requested a review from mgencur April 21, 2023 08:57

openshift-ci bot closed this May 11, 2023

cardil mentioned this pull request May 11, 2023

🎁 Pretty print cloud events #25

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎁 Pretty print cloud events via wasm #24

🎁 Pretty print cloud events via wasm #24

cardil commented Apr 17, 2023 •

edited

Loading

openshift-ci bot commented Apr 17, 2023

mgencur left a comment

mgencur Apr 20, 2023

cardil Apr 20, 2023

mgencur Apr 20, 2023

cardil Apr 20, 2023

cardil Apr 20, 2023

mgencur Apr 20, 2023

cardil Apr 20, 2023 •

edited

Loading

cardil commented Apr 20, 2023

mgencur commented Apr 21, 2023

rhuss commented Apr 25, 2023 •

edited

Loading

cardil commented May 11, 2023

openshift-ci bot commented May 11, 2023

mgencur commented May 11, 2023

🎁 Pretty print cloud events via wasm #24

🎁 Pretty print cloud events via wasm #24

Conversation

cardil commented Apr 17, 2023 • edited Loading

Changes

Found issues

openshift-ci bot commented Apr 17, 2023

mgencur left a comment

Choose a reason for hiding this comment

mgencur Apr 20, 2023

Choose a reason for hiding this comment

cardil Apr 20, 2023

Choose a reason for hiding this comment

mgencur Apr 20, 2023

Choose a reason for hiding this comment

cardil Apr 20, 2023

Choose a reason for hiding this comment

cardil Apr 20, 2023

Choose a reason for hiding this comment

mgencur Apr 20, 2023

Choose a reason for hiding this comment

cardil Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

cardil commented Apr 20, 2023

mgencur commented Apr 21, 2023

rhuss commented Apr 25, 2023 • edited Loading

cardil commented May 11, 2023

openshift-ci bot commented May 11, 2023

mgencur commented May 11, 2023

cardil commented Apr 17, 2023 •

edited

Loading

cardil Apr 20, 2023 •

edited

Loading

rhuss commented Apr 25, 2023 •

edited

Loading