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

Support streaming tail workers over rpc #3709

Merged
merged 1 commit into from
Mar 12, 2025
Merged

Conversation

danlapid
Copy link
Collaborator

@danlapid danlapid commented Mar 12, 2025

As it stands workers tailed by streaming tail workers have to know they are tailing to a streaming tail worker.
This is a limitation that should be refactored away in the future but for the moment we can't depend on hasHandler to infer this over rpc.

At the moment it produces the following error on the traced workerd instance:

exception = workerd/io/trace-stream.c++:976: disconnected: Streaming tail session canceled

This is a known issue preexisting that Felix is working on in a separate PR.

To test use the following files:
config.capnp:

using Workerd = import "/workerd/workerd.capnp";

const tailWorkerExample :Workerd.Config = (
  services = [
    (name = "main", worker = .helloWorld),
    (name = "log", external = ( address = "127.0.0.1:8081", http = ( capnpConnectHost = "capnp" ))),
  ],
  sockets = [ ( name = "http", address = "*:8080", http = (), service = "main" ) ],
);

const helloWorld :Workerd.Worker = (
  modules = [
    (name = "worker", esModule = embed "worker.js")
  ],
  compatibilityDate = "2024-10-14",
  streamingTails = ["log"],
  bindings = [(name = "log", service = "log")]
);

log.capnp:

using Workerd = import "/workerd/workerd.capnp";

const tailWorkerExample :Workerd.Config = (
  services = [
    (name = "log", worker = .logWorker),
  ],
  sockets = [ ( name = "rpc", address = "*:8081", http = ( capnpConnectHost = "capnp" ), service = "log" ) ],
);

const logWorker :Workerd.Worker = (
  modules = [
    (name = "worker", esModule = embed "tail.js")
  ],
  compatibilityDate = "2024-10-14",
);

tail.js:

export default {
  tailStream(...args) {
    console.log('tail in worker b');
    console.log(...args);
    return (...args) => {
      console.log(...args);
    };
  },
};

worker.js:

export default {
  async fetch(req, env) {
    console.log('log from worker a');
    return new Response('response from worker a');
  },
};

bash:

workerd serve deps/workerd/samples/tail-workers/log.capnp
workerd serve deps/workerd/samples/tail-workers/config.capnp
curl http://localhost:8080

@danlapid danlapid requested review from a team as code owners March 12, 2025 21:10
@fhanau
Copy link
Contributor

fhanau commented Mar 12, 2025

If we're adding streamingTails, samples/tail-workers/config.capnp and src/workerd/api/tail-worker-test.wd-test need to be updated accordingly.

@jasnell
Copy link
Member

jasnell commented Mar 12, 2025

The basic change here LGTM but @fhanau is correct that the test will need to be updated... as likely will the sample.

@danlapid danlapid force-pushed the dlapid/streamingTail_rpc branch from badfd3a to 0bac3b5 Compare March 12, 2025 22:03
@danlapid
Copy link
Collaborator Author

@fhanau @jasnell done, stamps appreciated.

Verified

This commit was signed with the committer’s verified signature.
dvandersluis Daniel Vandersluis
As it stands workers tailed by streaming tail workers have to know they are tailing to a streaming tail worker.
This is a limitation that should be refactored away in the future but for the moment we can't depend on hasHandler to infer this over rpc.
@danlapid danlapid force-pushed the dlapid/streamingTail_rpc branch from 0bac3b5 to 1bd1814 Compare March 12, 2025 22:15
@danlapid danlapid merged commit dbb7a58 into main Mar 12, 2025
17 checks passed
@danlapid danlapid deleted the dlapid/streamingTail_rpc branch March 12, 2025 22:30
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

3 participants