feat(subscriber): support grpc-web and add `grpc-web` feature #498

Rustin170506 · 2023-12-01T14:04:35Z

See more at https://discord.com/channels/500028886025895936/838895414455435335/1180164585236471898

Description

This pull request adds support for grpc-web to console-subscriber. Once you enable this feature by calling the enable_grpc_web function, you can connect the console-subscriber gRPC server using a browser client.

Explanation of Changes

Added a new feature called grpc-web which requires the tonic-web crate as a dependency.
A new API named serve_with_grpc_web has been introduced. It appears to be similar to the serve_with API. However, if we were to use the same API with serve_with, it would result in a bound issue. We attempted to combine serve_with_grpc_web and serve_with, but it would create a very complex trait bound for the function. Therefore, we decided to introduce a new API to address this problem.
Added a new example named grpc_web to show how to use the into_parts API to customize the CORS layer.

Rustin170506 · 2023-12-01T14:08:11Z

Tested in my console-web project(still private, I will open source it after I finish the basic features and views):
js code:

<script>
import { createPromiseClient } from "@connectrpc/connect";
import { createGrpcWebTransport } from "@connectrpc/connect-web";
import { Instrument } from "./gen/instrument_connect";
import { InstrumentRequest } from "./gen/instrument_pb";

const transport = createGrpcWebTransport({
  baseUrl: "http://127.0.0.1:8888",
});

const client = createPromiseClient(Instrument, transport);
const stream = client.watchUpdates(new InstrumentRequest());
(async () => {
  for await (const value of stream) {
    console.log(value);
  }
})();
</script>

console:

Rustin170506

🔢 Self-check

console-subscriber/Cargo.toml

console-subscriber/examples/grpc_web.rs

console-subscriber/src/lib.rs

hds

This is looking great. I've tried to answer your questions and give some feedback.

I'm not sure how feasible it would be, but is there any way we could include a single HTML+JS file which is able to read from the gRPC-web endpoint as part of this example? Or do we need to pull in too many JS dependencies?

console-subscriber/examples/grpc_web.rs

console-subscriber/src/lib.rs

console-subscriber/Cargo.toml

console-subscriber/src/builder.rs

Rustin170506 · 2023-12-05T01:19:42Z

I'm not sure how feasible it would be, but is there any way we could include a single HTML+JS file which is able to read from the gRPC-web endpoint as part of this example? Or do we need to pull in too many JS dependencies?

I believe using pure HTML and JavaScript might be somewhat challenging due to the requirement of generating protocol files, necessitating a node.js project. However, I'm open to adding a basic node.js example. My plan is to use 'connect' as it greatly simplifies the process of generating protocol files. What are your thoughts on this approach?

hds · 2023-12-05T06:43:48Z

That sounds like a good approach, using connect. I'm not very familiar with JS tech these days, I just think that if we're adding support for gRPC-web then it would be nice to have a small example which shows it in use.

Thank you!

Rustin170506 · 2024-01-30T14:43:18Z

I've updated my PR again.

A new API called serve_with_grpc_web will allow users to pass a server build and cors layer and it serve it.
For subscriber builder, we only server it with grpc support when grpc-web feature is enabled and users pass a cors layer.
Nothing changes to server_with, users can still use it to pass a builder with some basic gPRC settings.

The reason I made this change is that it seems the point of serve_with is not for supporting passing any server builder with layers, it is just for doing some basic gRPC settings. If I am wrong, please correct me. Thanks!

hds

As discussed, I think this is the right approach, however I believe we should keep the cors layer (and tower-http) out of our public API.

console-subscriber/src/builder.rs

console-subscriber/examples/grpc_web.rs

console-subscriber/src/lib.rs

Rustin170506

I planned to add a real web client example after we merge this API change first. I think it would help reduce the noise from adding a node app to it.

hds

Everything is looking great! I think we just need a bit more documentation (including some examples / doctests) and this PR will be good to merge.

console-subscriber/examples/grpc_web.rs

console-subscriber/src/builder.rs

console-subscriber/src/lib.rs

console-subscriber/examples/grpc_web.rs

Rustin170506

🔢 Self-check

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506

I have a concern about the default port for the grpc-web server. 6669 is a restricted port on chrome. So unless users change it, they won't be able to access it in chrome. So maybe we should change the default port when users enable grpc-web support. But I also worry that this would cause more chaos about the default value. Do you have any suggestions?

hds · 2024-02-11T21:18:00Z

I have a concern about the default port for the grpc-web server. 6669 is a restricted port on chrome. So unless users change it, they won't be able to access it in chrome. So maybe we should change the default port when users enable grpc-web support. But I also worry that this would cause more chaos about the default value. Do you have any suggestions?

I wouldn't dynamically change the default port based on using grpc-web. However, it's probably worth setting the port to some other value in the grpc-web example, perhaps something typical like 8080. And also add a comment stating why a different port is used. That way, at least for people copying from the example, there will be extra information.

Perhaps also adding a line to the documentation for enable_grpc_web stating that some browsers may block the default port 6669 would be a good idea.

hds

Looking good, some suggestions based on your comment regarding the default port and also a couple of style suggestions that you could apply directly.

console-subscriber/src/builder.rs

console-subscriber/src/lib.rs

console-subscriber/src/builder.rs

Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 · 2024-02-13T14:22:49Z

However, it's probably worth setting the port to some other value in the grpc-web example, perhaps something typical like 8080.

I've already set it to 9999 before. It worked well on my different browsers. (chrome/arc/firefox/safari)

Perhaps also adding a line to the documentation for enable_grpc_web stating that some browsers may block the default port 6669 would be a good idea.

Added. Thanks for your suggestion!

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506

🔢 Self-check

Rustin170506 · 2024-02-13T14:40:16Z

console-subscriber/examples/grpc_web.rs

+                    .add_service(instrument_server);
+                let serve = router.serve(std::net::SocketAddr::new(
+                    std::net::IpAddr::V4(std::net::Ipv4Addr::new(127, 0, 0, 1)),
+                    // 6669 is a restricted port on Chrome, so we cannot use it. We use a different port instead.


Rustin170506 · 2024-02-13T14:40:33Z

console-subscriber/src/lib.rs

+    ///     .add_service(instrument_server);
+    /// let serve = router.serve(std::net::SocketAddr::new(
+    ///     std::net::IpAddr::V4(std::net::Ipv4Addr::new(127, 0, 0, 1)),
+    /// // 6669 is a restricted port on Chrome, so we cannot use it. We use a different port instead.


console-subscriber/src/lib.rs

Rustin170506 · 2024-02-16T13:27:46Z

@hds Before we move forward with this PR, is there anything I need to do?

hds

Looks good. Thanks for all the time you spent on this PR, it's great!

hds · 2024-02-16T13:34:37Z

@hds Before we move forward with this PR, is there anything I need to do?

@hi-rustin Sorry! I thought I merged this already. Done now! Thank you!

Rustin170506 · 2024-02-16T15:08:35Z

Thanks for your review! 💚 💙 💜 💛 ❤️

…scriber-v0.3.0 (#557) ## 🤖 New release * `tokio-console`: 0.1.10 -> 0.1.11 * `console-api`: 0.6.0 -> 0.7.0 * `console-subscriber`: 0.2.0 -> 0.3.0 ## `tokio-console` ## tokio-console-v0.1.11 - (2024-06-10) ### Added - Replace target column with kind column in tasks view ([#478](#478)) ([903d9fa](903d9fa)) - Add flags and configurations for warnings ([#493](#493)) ([174883f](174883f)) - Add `--allow` flag ([#513](#513)) ([8da7037](8da7037)) ### Documented - Add note about running on Windows ([#510](#510)) ([a0d20fd](a0d20fd)) ### Fixed - Ignore key release events ([#468](#468)) ([715713a](715713a)) - Accept only `file://`, `http://`, `https://` URI ([#486](#486)) ([031bddd](031bddd)) - Fix column sorting in resources tab ([#491](#491)) ([96c65bd](96c65bd)) - Only trigger lints on async tasks ([#517](#517)) ([4593222](4593222)) - Remove duplicate controls from async ops view ([#519](#519)) ([f28ba4a](f28ba4a)) - Add pretty format for 'last woken' time ([#529](#529)) ([ea11ad8](ea11ad8)) ## `console-api` ## console-api-v0.7.0 - (2024-06-10) ### <a id = "0.7.0-breaking"></a>Breaking Changes - **Bump tonic to 0.11 ([#547](#547 ([ef6816c](https://github.com/tokio-rs/console/commit/ef6816caa0fe84171105b513425506f25d3082af))<br />This is a breaking change for users of `console-api` and `console-subscriber`, as it changes the public `tonic` dependency to a semver-incompatible version. This breaks compatibility with `tonic` 0.10.x. ### Documented - Fix typo in proto ([#472](#472)) ([2dd3559](2dd3559)) ### Updated - [**breaking**](#0.7.0-breaking) Bump tonic to 0.11 ([#547](#547)) ([ef6816c](ef6816c)) ## `console-subscriber` ## console-subscriber-v0.3.0 - (2024-06-10) ### <a id = "0.3.0-breaking"></a>Breaking Changes - **Bump tonic to 0.11 ([#547](#547 ([ef6816c](https://github.com/tokio-rs/console/commit/ef6816caa0fe84171105b513425506f25d3082af))<br />This is a breaking change for users of `console-api` and `console-subscriber`, as it changes the public `tonic` dependency to a semver-incompatible version. This breaks compatibility with `tonic` 0.10.x. ### Added - Replace target column with kind column in tasks view ([#478](#478)) ([903d9fa](903d9fa)) - Reduce retention period to fit in max message size ([#503](#503)) ([bd3dd71](bd3dd71)) - Support grpc-web and add `grpc-web` feature ([#498](#498)) ([4150253](4150253)) ### Documented - Add a grpc-web app example ([#526](#526)) ([4af30f2](4af30f2)) - Fix typo in doc comment ([#543](#543)) ([ff22678](ff22678)) ### Fixed - Don't save poll_ops if no-one is receiving them ([#501](#501)) ([1656c79](1656c79)) - Ignore metadata that is not a span or event ([#554](#554)) ([852a977](852a977)) ### Updated - [**breaking**](#0.3.0-breaking) Bump tonic to 0.11 ([#547](#547)) ([ef6816c](ef6816c))

Rustin170506 requested a review from a team as a code owner December 1, 2023 14:04

Rustin170506 marked this pull request as draft December 1, 2023 14:04

Rustin170506 changed the title ~~POC: feat: support grpc-web and add web feature~~ feat: support grpc-web and add web feature Dec 3, 2023

Rustin170506 force-pushed the rustin-patch-grpc-web branch from f57ce01 to 545750c Compare December 3, 2023 13:10

Rustin170506 commented Dec 3, 2023

View reviewed changes

console-subscriber/Cargo.toml Show resolved Hide resolved

console-subscriber/examples/grpc_web.rs Outdated Show resolved Hide resolved

console-subscriber/src/lib.rs Outdated Show resolved Hide resolved

Rustin170506 marked this pull request as ready for review December 3, 2023 13:14

hds requested changes Dec 4, 2023

View reviewed changes

console-subscriber/examples/grpc_web.rs Outdated Show resolved Hide resolved

console-subscriber/src/lib.rs Outdated Show resolved Hide resolved

console-subscriber/Cargo.toml Show resolved Hide resolved

console-subscriber/src/builder.rs Outdated Show resolved Hide resolved

Rustin170506 requested a review from hds December 5, 2023 01:20

Rustin170506 marked this pull request as draft January 22, 2024 15:39

Rustin170506 changed the title ~~feat: support grpc-web and add web feature~~ WIP: feat: support grpc-web and add web feature Jan 23, 2024

Rustin170506 marked this pull request as ready for review January 30, 2024 14:42

Rustin170506 changed the title ~~WIP: feat: support grpc-web and add web feature~~ feat: support grpc-web and add web feature Jan 30, 2024

hds requested changes Jan 30, 2024

View reviewed changes

Rustin170506 commented Feb 4, 2024

View reviewed changes

Rustin170506 requested a review from hds February 4, 2024 14:57

hds requested changes Feb 7, 2024

View reviewed changes

console-subscriber/examples/grpc_web.rs Outdated Show resolved Hide resolved

console-subscriber/src/builder.rs Show resolved Hide resolved

console-subscriber/src/lib.rs Show resolved Hide resolved

console-subscriber/examples/grpc_web.rs Outdated Show resolved Hide resolved

Rustin170506 changed the title ~~feat: support grpc-web and add web feature~~ feat: support grpc-web and add grpc-web feature Feb 10, 2024

Rustin170506 commented Feb 10, 2024

View reviewed changes

Rustin170506 added 8 commits February 10, 2024 11:00

feat: add new API to support grpc-web

2cbd34c

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

feat: add cors layer

884f153

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

refactor: reuse code

1f6c951

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

fix: do not change server_with

37acf97

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

docs: update desc

54642d4

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

fix: do not depend on cors

6dde803

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

chore: remove new line

9b7c210

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

docs: add docs for enable_grpc_web

96e5c0d

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

docs: add examples for serve_with_grpc_web

fa50b4c

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 force-pushed the rustin-patch-grpc-web branch from 99c4414 to fa50b4c Compare February 10, 2024 03:00

Rustin170506 commented Feb 10, 2024

View reviewed changes

Rustin170506 requested a review from hds February 10, 2024 03:08

hds reviewed Feb 11, 2024

View reviewed changes

Rustin170506 and others added 4 commits February 13, 2024 22:08

Update console-subscriber/src/builder.rs

a5a741c

Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>

Update console-subscriber/src/builder.rs

8246b95

Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>

fix: move feature flag below comments

22cc8f5

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

docs: add comments about default port

95ba1ca

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

fix: remove duplication

a6e881a

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 commented Feb 13, 2024

View reviewed changes

Rustin170506 requested a review from hds February 13, 2024 14:40

Merge branch 'main' into rustin-patch-grpc-web

e887798

hds reviewed Feb 13, 2024

View reviewed changes

console-subscriber/src/lib.rs Outdated Show resolved Hide resolved

Update console-subscriber/src/lib.rs

0fd9df4

hds changed the title ~~feat: support grpc-web and add grpc-web feature~~ feat(subscriber): support grpc-web and add grpc-web feature Feb 13, 2024

Merge branch 'main' into rustin-patch-grpc-web

a904b25

Rustin170506 requested a review from hds February 16, 2024 13:27

hds approved these changes Feb 16, 2024

View reviewed changes

hds merged commit 4150253 into tokio-rs:main Feb 16, 2024
16 checks passed

Rustin170506 deleted the rustin-patch-grpc-web branch February 16, 2024 15:08

yerke mentioned this pull request Feb 27, 2024

subscriber: add grpc-web support to console-subscriber #497

Closed

github-actions bot mentioned this pull request Jun 7, 2024

chore: release tokio-console-v0.1.11, console-api-v0.7.0, console-subscriber-v0.3.0 #557

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(subscriber): support grpc-web and add `grpc-web` feature #498

feat(subscriber): support grpc-web and add `grpc-web` feature #498

Rustin170506 commented Dec 1, 2023 •

edited

Loading

Rustin170506 commented Dec 1, 2023 •

edited

Loading

Rustin170506 left a comment

hds left a comment

Rustin170506 commented Dec 5, 2023

hds commented Dec 5, 2023

Rustin170506 commented Jan 30, 2024

hds left a comment

Rustin170506 left a comment

hds left a comment

Rustin170506 left a comment

Rustin170506 left a comment

hds commented Feb 11, 2024

hds left a comment

Rustin170506 commented Feb 13, 2024

Rustin170506 left a comment

Rustin170506 Feb 13, 2024

Rustin170506 Feb 13, 2024

Rustin170506 commented Feb 16, 2024

hds left a comment

hds commented Feb 16, 2024

Rustin170506 commented Feb 16, 2024

feat(subscriber): support grpc-web and add grpc-web feature #498

feat(subscriber): support grpc-web and add grpc-web feature #498

Conversation

Rustin170506 commented Dec 1, 2023 • edited Loading

Description

Explanation of Changes

Rustin170506 commented Dec 1, 2023 • edited Loading

Rustin170506 left a comment

Choose a reason for hiding this comment

hds left a comment

Choose a reason for hiding this comment

Rustin170506 commented Dec 5, 2023

hds commented Dec 5, 2023

Rustin170506 commented Jan 30, 2024

hds left a comment

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

hds left a comment

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

hds commented Feb 11, 2024

hds left a comment

Choose a reason for hiding this comment

Rustin170506 commented Feb 13, 2024

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 Feb 13, 2024

Choose a reason for hiding this comment

Rustin170506 Feb 13, 2024

Choose a reason for hiding this comment

Rustin170506 commented Feb 16, 2024

hds left a comment

Choose a reason for hiding this comment

hds commented Feb 16, 2024

Rustin170506 commented Feb 16, 2024

feat(subscriber): support grpc-web and add `grpc-web` feature #498

feat(subscriber): support grpc-web and add `grpc-web` feature #498

Rustin170506 commented Dec 1, 2023 •

edited

Loading

Rustin170506 commented Dec 1, 2023 •

edited

Loading