Event: Deprecate default network timestamp metadata

This deprecates the Event constructor and the ``ts`` parameter of Enqueue()
Instead, versions are introduced that take a detail::MetadataVectorPtr which
can hold the network timestamp metadata and is meant to be allocated by the
caller instead of automatically during Enqueue() or within the Event
constructor.

This also introduces a BifConst ``EventMetadata::add_network_timestamp`` to
opt-in adding network timestamps to events globally. It's disabled by
default as there are not a lot of known use cases that need this.

NEWS

@@ -15,6 +15,15 @@ Breaking Changes
   files. We tested builds of all of the existing third-party packages and only noticed one
   or two failures, but there is a possibility for breakage related to this cleanup.
 
+- Network timestamps are not added to events by default anymore. Use the following
+  redef line to enable them:
+
+	redef EventMetadata::add_network_timestamp = T;
+
+  The background is that event metadata has become more generic and may incur
+  a small overhead when enabled. There's not enough users of network timestamp
+  metadata to justify the complexity of treating it separate.
+
 New Functionality
 -----------------
 
@@ -65,6 +74,13 @@ Deprecated Functionality
 
   The replacement script doesn't populate the ``email_body_sections`` anymore either.
 
+- The ``zeek::Event()`` constructor was deprecated. Use ``event_mgr::Enqueue()``
+  or ``event_mgr::Dispatch()`` instead.
+
+- Passing ``ts`` as the last argument to ``EventMgr::Enqueue()`` has been deprecated
+  and will lead to compile time warnings. Use ``EventMgr::Enqueue(detail::MetadataVectorPtr meta, ...)``
+  for populating ``meta`` accordingly.
+
 Zeek 7.2.0
 ==========
 

scripts/base/init-bare.zeek

@@ -592,6 +592,15 @@ export {
 		id: EventMetadata::ID; ##< The registered :zeek:see:`EventMetadata::ID` value.
 		val: any;              ##< The value. Its type matches what was passed to :zeek:see:`EventMetadata::register`.
 	};
+
+	## Add network timestamp metadata to all events.
+	##
+	## Adding network timestamp metadata affects local and
+	## remote events. Events scheduled have a network timestamp
+	## of when the scheduled timer was supposed to expire, which
+	## might be a value before the network_time() when the event
+	## was actually dispatched.
+	const add_network_timestamp: bool = F &redef;
 }
 
 module FTP;

src/Event.cc

@@ -12,6 +12,7 @@
 #include "zeek/iosource/Manager.h"
 #include "zeek/plugin/Manager.h"
 
+#include "const.bif.netvar_h"
 #include "event.bif.netvar_h"
 
 zeek::EventMgr zeek::event_mgr;
@@ -52,6 +53,19 @@ Event::Event(const EventHandlerPtr& arg_handler, zeek::Args arg_args, util::deta
         Ref(obj);
 }
 
+Event::Event(detail::EventMetadataVectorPtr arg_meta, const EventHandlerPtr& arg_handler, zeek::Args arg_args,
+             util::detail::SourceID arg_src, analyzer::ID arg_aid, Obj* arg_obj)
+    : handler(arg_handler),
+      args(std::move(arg_args)),
+      src(arg_src),
+      aid(arg_aid),
+      obj(arg_obj),
+      next_event(nullptr),
+      meta(std::move(arg_meta)) {
+    if ( obj )
+        Ref(obj);
+}
+
 double Event::Time() const {
     if ( ! meta )
         return 0.0;
@@ -120,8 +134,60 @@ EventMgr::~EventMgr() {
 }
 
 void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID src, analyzer::ID aid, Obj* obj,
-                       double ts) {
+                       DeprecatedTimestamp deprecated_ts) {
-    QueueEvent(new Event(h, std::move(vl), src, aid, obj, ts));
+    detail::EventMetadataVectorPtr meta;
+
+    double ts = double(deprecated_ts);
+    if ( BifConst::EventMetadata::add_network_timestamp ) {
+        if ( ts < 0.0 ) // default -1.0, modify to current network_time
+            ts = run_state::network_time;
+
+        // In v8.1 when the deprecated_ts parameters is gone: Just use run_state::network_time directly here.
+        meta = detail::MakeEventMetadataVector(ts);
+    }
+    else if ( ts >= 0.0 ) {
+        // EventMetadata::add_network_timestamp is false, but EventMgr::Enqueue()
+        // with an explicit (non-negative) timestamp is used. That's a deprecated
+        // API, but we continue to support it until v8.1.
+        meta = detail::MakeEventMetadataVector(ts);
+    }
+
+    QueueEvent(new Event(std::move(meta), h, std::move(vl), src, aid, obj));
+}
+
+void EventMgr::Enqueue(detail::EventMetadataVectorPtr meta, const EventHandlerPtr& h, Args vl,
+                       util::detail::SourceID src, analyzer::ID aid, Obj* obj) {
+    if ( BifConst::EventMetadata::add_network_timestamp ) {
+        // If all events are supposed to have a network time attached, ensure
+        // that the meta vector was passed *and* contains a network timestamp.
+        bool has_time = false;
+
+        if ( ! meta ) {
+            // No metadata vector at all, make one with a timestamp.
+            meta = detail::MakeEventMetadataVector(run_state::network_time);
+        }
+        else {
+            // Check all entries for a network timestamp
+            for ( const auto& m : *meta ) {
+                if ( m.Id() == static_cast<zeek_uint_t>(detail::MetadataType::NetworkTimestamp) ) {
+                    has_time = true;
+
+                    if ( m.Val()->GetType()->Tag() != TYPE_TIME ) {
+                        // This should've been caught during parsing.
+                        zeek::reporter->InternalError("event metadata timestamp has wrong type: %s",
+                                                      obj_desc_short(m.Val()->GetType().get()).c_str());
+                    }
+                }
+            }
+
+            if ( ! has_time ) {
+                auto tv = zeek::make_intrusive<zeek::TimeVal>(run_state::network_time);
+                meta->push_back({static_cast<zeek_uint_t>(detail::MetadataType::NetworkTimestamp), std::move(tv)});
+            }
+        }
+    }
+
+    QueueEvent(new Event(std::move(meta), h, std::move(vl), src, aid, obj));
 }
 
 void EventMgr::QueueEvent(Event* event) {
@@ -150,7 +216,13 @@ void EventMgr::Dispatch(Event* event, bool no_remote) {
 }
 
 void EventMgr::Dispatch(const EventHandlerPtr& h, zeek::Args vl) {
-    auto* ev = new Event(h, std::move(vl));
+    detail::EventMetadataVectorPtr meta;
+
+    // If all events should have network timestamps, create the vector holding one.
+    if ( BifConst::EventMetadata::add_network_timestamp )
+        meta = detail::MakeEventMetadataVector(run_state::network_time);
+
+    auto* ev = new Event(std::move(meta), h, std::move(vl), util::detail::SOURCE_LOCAL, 0, nullptr);
 
     // Technically this isn't queued, but still give plugins a chance to
     // intercept the event and cancel or modify it if really wanted.

src/Event.h

@@ -53,6 +53,7 @@ EventMetadataVectorPtr MakeEventMetadataVector(double t);
 
 class Event final : public Obj {
 public:
+    [[deprecated("Remove in v8.1: Do not instantiate raw events. Use EventMgr::Dispatch() or EventMgr::Enqueue().")]]
     Event(const EventHandlerPtr& handler, zeek::Args args, util::detail::SourceID src = util::detail::SOURCE_LOCAL,
           analyzer::ID aid = 0, Obj* obj = nullptr, double ts = run_state::network_time);
 
@@ -70,6 +71,10 @@ public:
 private:
     friend class EventMgr;
 
+    // Construct an event with a metadata vector. Passing arg_meta as nullptr is explicitly allowed.
+    Event(detail::EventMetadataVectorPtr arg_meta, const EventHandlerPtr& arg_handler, zeek::Args arg_args,
+          util::detail::SourceID arg_src, analyzer::ID arg_aid, Obj* arg_obj);
+
     // This method is protected to make sure that everybody goes through
     // EventMgr::Dispatch().
     void Dispatch(bool no_remote = false);
@@ -84,6 +89,8 @@ private:
 };
 
 class EventMgr final : public Obj, public iosource::IOSource {
+    class DeprecatedTimestamp;
+
 public:
     ~EventMgr() override;
 
@@ -99,10 +106,10 @@ public:
      * @param obj  an arbitrary object to use as a "cookie" or just hold a
      * reference to until dispatching the event.
      * @param ts  timestamp at which the event is intended to be executed
-     * (defaults to current network time).
+     * (defaults to current network time - deprecated).
      */
     void Enqueue(const EventHandlerPtr& h, zeek::Args vl, util::detail::SourceID src = util::detail::SOURCE_LOCAL,
-                 analyzer::ID aid = 0, Obj* obj = nullptr, double ts = run_state::network_time);
+                 analyzer::ID aid = 0, Obj* obj = nullptr, DeprecatedTimestamp ts = {});
 
     /**
      * A version of Enqueue() taking a variable number of arguments.
@@ -113,6 +120,19 @@ public:
         return Enqueue(h, zeek::Args{std::forward<Args>(args)...});
     }
 
+    /**
+     * Enqueue() with metadata vector support.
+     * @param meta  Metadata to attach to the event, can be nullptr.
+     * @param h  reference to the event handler to later call.
+     * @param vl  the argument list to the event handler call.
+     * @param src  indicates the origin of the event (local versus remote).
+     * @param aid  identifies the protocol analyzer generating the event.
+     * @param obj  an arbitrary object to use as a "cookie" or just hold a
+     * reference to until dispatching the event.
+     */
+    void Enqueue(detail::EventMetadataVectorPtr meta, const EventHandlerPtr& h, zeek::Args vl,
+                 util::detail::SourceID src = util::detail::SOURCE_LOCAL, analyzer::ID aid = 0, Obj* obj = nullptr);
+
     [[deprecated("Remove in v8.1: Use Dispatch(handler, args) instead.")]]
     void Dispatch(Event* event, bool no_remote = false);
 
@@ -162,6 +182,24 @@ public:
     uint64_t num_events_dispatched = 0;
 
 private:
+    /**
+     * Helper class to produce a compile time warning if Enqueue() is called with an explicit timestamp.
+     *
+     * Remove in v8.1.
+     */
+    class DeprecatedTimestamp {
+    public:
+        DeprecatedTimestamp() : d(-1.0) {}
+        [[deprecated("Use overload EventMgr::Enqueue(EventMetadataVectorPtr meta, ...) to pass timestamp metadata")]]
+        /*implicit*/ DeprecatedTimestamp(double d)
+            : d(d) {}
+
+        explicit operator double() const { return d; }
+
+    private:
+        double d;
+    };
+
     void QueueEvent(Event* event);
 
     Event* current = nullptr;

src/Expr.cc

@@ -24,6 +24,8 @@
 #include "zeek/script_opt/Expr.h"
 #include "zeek/script_opt/ScriptOpt.h"
 
+#include "const.bif.netvar_h"
+
 namespace zeek::detail {
 
 const char* expr_name(ExprTag t) {
@@ -3827,11 +3829,17 @@ ScheduleTimer::ScheduleTimer(const EventHandlerPtr& arg_event, Args arg_args, do
 
 void ScheduleTimer::Dispatch(double /* t */, bool /* is_expire */) {
     if ( event ) {
-        // An event's intended timestamp might be in the past as timer expiration is driven by
+        detail::EventMetadataVectorPtr meta;
-        // network time. Guarantee that the intended timestamp is never in the future (e.g.,
+
-        // when all timers are expired on shutdown).
+        if ( BifConst::EventMetadata::add_network_timestamp ) {
-        auto ts = std::min(this->Time(), run_state::network_time);
+            // An event's intended timestamp might be in the past as timer expiration is driven by
-        event_mgr.Enqueue(event, std::move(args), util::detail::SOURCE_LOCAL, 0, nullptr, ts);
+            // network time. Guarantee that the intended timestamp is never in the future (e.g.,
+            // when all timers are expired on shutdown).
+            auto ts = std::min(this->Time(), run_state::network_time);
+            meta = detail::MakeEventMetadataVector(ts);
+        }
+
+        event_mgr.Enqueue(std::move(meta), event, std::move(args));
     }
 }
 

src/const.bif

@@ -34,3 +34,5 @@ const Log::flush_interval: interval;
 const Log::write_buffer_size: count;
 
 const Storage::expire_interval: interval;
+
+const EventMetadata::add_network_timestamp: bool;

testing/btest/broker/remote_event_auto_ts.zeek

@@ -17,6 +17,7 @@
 # @TEST-START-FILE send.zeek
 
 redef exit_only_after_terminate = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global runs = 0;
 global ping: event(msg: string, intended_ts: time);
@@ -58,6 +59,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string)
 # @TEST-START-FILE recv.zeek
 
 redef exit_only_after_terminate = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global msg_count = 0;
 

testing/btest/broker/remote_event_schedule_ts.zeek

@@ -17,6 +17,7 @@
 # @TEST-START-FILE send.zeek
 
 redef exit_only_after_terminate = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global runs = 0;
 global ping: event(msg: string, intended_ts: time, publish_ts: time);
@@ -64,6 +65,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string)
 # @TEST-START-FILE recv.zeek
 
 redef exit_only_after_terminate = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global msg_count = 0;
 

testing/btest/broker/remote_event_ts.zeek

@@ -15,6 +15,7 @@
 # @TEST-START-FILE send.zeek
 
 redef exit_only_after_terminate = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global runs = 0;
 global ping: event(msg: string, intended_ts: time);
@@ -54,6 +55,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string)
 # @TEST-START-FILE recv.zeek
 
 redef exit_only_after_terminate = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global msg_count = 0;
 

testing/btest/broker/remote_event_ts_compat.zeek

@@ -17,6 +17,7 @@
 
 redef exit_only_after_terminate = T;
 redef allow_network_time_forward = F;
+redef EventMetadata::add_network_timestamp = T;
 
 event zeek_init()
 	{

testing/btest/broker/web-socket-events-metadata.zeek

@@ -16,6 +16,7 @@
 redef allow_network_time_forward = F;
 redef exit_only_after_terminate = T;
 redef Broker::disable_ssl = T;
+redef EventMetadata::add_network_timestamp = T;
 
 global event_count = 0;
 

testing/btest/language/event-ts-scheduled.zeek

@@ -1,6 +1,8 @@
 # @TEST-EXEC: zeek -b -r $TRACES/ticks-dns-1hr.pcap %INPUT > out
 # @TEST-EXEC: btest-diff out
 
+redef EventMetadata::add_network_timestamp = T;
+
 global runs = 0;
 
 event test(schedule_time: time)

testing/btest/language/event-ts.zeek

@@ -4,6 +4,8 @@
 # Note: We use a PCAP with DNS queries only so that we have a single packet per
 # time step. Thus the run loop will be executed only once per time step.
 
+redef EventMetadata::add_network_timestamp = T;
+
 global runs = -1;
 
 event test(depth: count)