Skip to content

Commit

Permalink
Replace logging with tails in workerd.capnp.
Browse files Browse the repository at this point in the history
Using a simple array seems strictly easier to understand than a union with zero/one/many variants.

REVIEW: Can we simply remove `logging`? It was introduced relatively recently. Is anyone depending on it yet?
  • Loading branch information
kentonv committed Nov 29, 2024
1 parent 51f0350 commit 9c48d92
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 20 deletions.
4 changes: 1 addition & 3 deletions src/workerd/server/server-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -2310,9 +2310,7 @@ KJ_TEST("Server: tail workers") {
`}
)
],
logging = (
toServices = ["tail", "tail2"]
),
tails = ["tail", "tail2"],
)
),
( name = "tail",
Expand Down
44 changes: 28 additions & 16 deletions src/workerd/server/server.c++
Original file line number Diff line number Diff line change
Expand Up @@ -3228,24 +3228,36 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name,
}
}

auto logging = conf.getLogging();
switch (logging.which()) {
case config::Worker::Logging::Which::NONE: {
break;
}
case config::Worker::Logging::Which::TO_SERVICE: {
result.loggingServices = kj::arr(&lookupService(logging.getToService(),
kj::str("Worker \"", name, "\"'s logging")));
break;
result.loggingServices = KJ_MAP(tail, conf.getTails()) {
return &lookupService(tail, kj::str("Worker \"", name, "\"'s tails"));
};

// Also check deprecated `logging` field.
if (conf.hasTails()) {
if (!conf.getLogging().isNone()) {
reportConfigError(kj::str("service ", name,
": Config specifies both `logging` and `tails`. Please use only `tails`."));
}
case config::Worker::Logging::Which::TO_SERVICES: {
auto list = logging.getToServices();
auto builder = kj::heapArrayBuilder<Service*>(list.size());
for (auto svc: list) {
builder.add(&lookupService(svc, kj::str("Worker \"", name, "\"'s logging")));
} else {
auto logging = conf.getLogging();
switch (logging.which()) {
case config::Worker::Logging::Which::NONE: {
break;
}
case config::Worker::Logging::Which::TO_SERVICE: {
result.loggingServices = kj::arr(
&lookupService(logging.getToService(), kj::str("Worker \"", name, "\"'s logging")));
break;
}
case config::Worker::Logging::Which::TO_SERVICES: {
auto list = logging.getToServices();
auto builder = kj::heapArrayBuilder<Service*>(list.size());
for (auto svc: list) {
builder.add(&lookupService(svc, kj::str("Worker \"", name, "\"'s logging")));
}
result.loggingServices = builder.finish();
break;
}
result.loggingServices = builder.finish();
break;
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/workerd/server/workerd.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,12 @@ struct Worker {

moduleFallback @13 :Text;

# Tail worker configuration
tails @17 :List(ServiceDesignator);
# List of tail worker services that should receive tail events for this worker.
# See: https://developers.cloudflare.com/workers/observability/logs/tail-workers/

logging :union {
# DEPRECATED: Use `tails` instead. `logging` does exactly the same thing.
none @14 :Void;
toService @15 :ServiceDesignator;
toServices @16 :List(ServiceDesignator);
Expand Down

0 comments on commit 9c48d92

Please sign in to comment.