From 9c48d926b9e3cb7208bef3deaea3cebc75f47bc0 Mon Sep 17 00:00:00 2001 From: Kenton Varda Date: Fri, 29 Nov 2024 14:38:25 -0600 Subject: [PATCH] Replace `logging` with `tails` in workerd.capnp. 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? --- src/workerd/server/server-test.c++ | 4 +-- src/workerd/server/server.c++ | 44 +++++++++++++++++++----------- src/workerd/server/workerd.capnp | 6 +++- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/workerd/server/server-test.c++ b/src/workerd/server/server-test.c++ index 46548c212c0..63437be3f1b 100644 --- a/src/workerd/server/server-test.c++ +++ b/src/workerd/server/server-test.c++ @@ -2310,9 +2310,7 @@ KJ_TEST("Server: tail workers") { `} ) ], - logging = ( - toServices = ["tail", "tail2"] - ), + tails = ["tail", "tail2"], ) ), ( name = "tail", diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++ index 4d2d4fb5aa9..9fcba74da3e 100644 --- a/src/workerd/server/server.c++ +++ b/src/workerd/server/server.c++ @@ -3228,24 +3228,36 @@ kj::Own 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(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(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; } } diff --git a/src/workerd/server/workerd.capnp b/src/workerd/server/workerd.capnp index a2453d7454f..56cb54ef3c7 100644 --- a/src/workerd/server/workerd.capnp +++ b/src/workerd/server/workerd.capnp @@ -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);