Skip to content

Implement original tail worker support for local dev#3021

Merged
jasnell merged 1 commit intomainfrom
jsnell/workerd-tail-workers
Oct 30, 2024
Merged

Implement original tail worker support for local dev#3021
jasnell merged 1 commit intomainfrom
jsnell/workerd-tail-workers

Conversation

@jasnell
Copy link
Collaborator

@jasnell jasnell commented Oct 28, 2024

Enables tail workers for local development in workerd

@jasnell jasnell requested review from a team as code owners October 28, 2024 23:47
@jasnell jasnell force-pushed the jsnell/workerd-tail-workers branch from 7188e44 to 8046c83 Compare October 29, 2024 15:47
@jasnell jasnell force-pushed the jsnell/workerd-tail-workers branch from 8046c83 to 828f9c6 Compare October 29, 2024 16:09
@jasnell
Copy link
Collaborator Author

jasnell commented Oct 29, 2024

Odd / non-obvious crashes in this PR on windows :-/ ... not getting any useful traces out of the CI run and unable to reproduce locally so I'll have to do a bit of trial-and-error debugging via fixup commits.

@jasnell jasnell force-pushed the jsnell/workerd-tail-workers branch 3 times, most recently from 0ab6cc9 to c5433df Compare October 30, 2024 01:13
@jasnell jasnell force-pushed the jsnell/workerd-tail-workers branch from c5433df to 33a9135 Compare October 30, 2024 01:37
Copy link
Collaborator

@danlapid danlapid left a comment

Choose a reason for hiding this comment

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

LGTM, was the se translator useful? Do we want to add it in a separate PR?

@jasnell
Copy link
Collaborator Author

jasnell commented Oct 30, 2024

... was the se translator useful? ...

Not really. Enabling the flag necessary seemed to make the build hang forever. I managed to spot the issue while I was waiting for it to finish building lol

@jasnell jasnell merged commit 152df47 into main Oct 30, 2024
@jasnell jasnell deleted the jsnell/workerd-tail-workers branch October 30, 2024 14:33
@kentonv
Copy link
Member

kentonv commented Nov 29, 2024

Are there any tests for this?

moduleFallback @13 :Text;

# Tail worker configuration
logging :union {
Copy link
Member

Choose a reason for hiding this comment

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

This could have just been:

tails @14 :List(ServiceDesignator);

The union adds unnecessarily complexity.

Is anyone using this yet? Can we change it?

Please remember to tag me on changes to capnp APIs.

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.

3 participants