This can happen if either there's no network timestamp associated with
an event, or there's currently no event being dispatched. Using 0.0
isn't great as it's the normal start timestamp before reading a network
packet. Using -1.0 gives the caller a chance to check and realize what's
going on.
It seems less surprising if only local events receive automatic network
timestamp metadata. For remote events the automatic value will most
likely be misleading.
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.
This removes the ts attribute from Event and instead allocates a vector for
storing metadata. By default, adds the network time as a TimeVal. Later
patches will make the allocation of the vector optional by introducing a
different constructor so that users that are not interested in network
timestamp metadata do not take the allocation hit.
Moving the explicit ``ts`` out of the event is done in order to treat it
just as generic metadata, too. However, the Time() accessor is adapted to
lookup the value from the metadata vector instead.
Allow users to call event_mgr.Dispatch(handler, args) instead of
constructing the Event instance themselves. Deprecate the old API
and replace users.
There's a subtle change that net_done() may be propagated via
auto_publish() now, but that still needs opt-in from script land
and likely no one did that, or else they'd expected to have it
work anyhow.
The ts parameter was only added to Call() for the Broker::auto_publish()
functionality and propagating the network timestamp. By now, the auto-publish
functionality is deprecated, so it'd be good to cleanup that signature. There
won't be any need for no_remote in the future either.
Allow users to just use Call() instead.
Avoid proliferation of various members on EventMgr by storing the
pointer of the current event instead.
This subtly changes the behavior of some builtin functions as they would
have returned the prior event's data when executed outside of event
draining (e.g. C++ level hook invocations), but I think that's actually
for the better.
While it seems interesting functionality, this hasn't been documented,
maintained or knowingly leveraged for many years.
There are various other approaches today, too:
* We track the number of event handler invocations regardless of
profiling. It's possible to approximate a load_sample event by
comparing the result of two get_event_stats() calls. Or, visualize
the corresponding counters in a Prometheus setup to get an idea of
event/s broken down by event names.
* HookCallFunction() allows to intercept script execution, including
measuring the time execution takes.
* The global call_stack and g_frame_stack can be used from plugins
(and even external processes) to walk the Zeek script stack at certain
points to implement a sampling profiler.
* USDT probes or more plugin hooks will likely be preferred over Zeek
builtin functionality in the future.
Relates to #3458
It can be visible overhead to call write() on the underlying pipe of the
EventMgr's flare whenever the first event is enqueued during an IO loop
iteration. Particularly in scenarios where there's about 1 event per packet
for long lived connections and script-side event processing is fast.
Given the event manager is drained anyhow at the end of the main loop, this
shouldn't be needed. In fact, the EventMgr.Process() method is basically
a stub. The one reason it is needed is when more events are enqueued during
a drain. That, however, can be dealt with by implementing GetNextTimeout()
to return 0.0 when there's more events queued. This way the main-loop's poll
timeout is 0.0 and it'll continue immediately.
This also allows to removes some extra code and drop the recently introduced
InitPostFork() addition: Without a pipe, there's no need to recreate it.
This largely copies over Spicy's `.clang-format` configuration file. The
one place where we deviate is header include order since Zeek depends on
headers being included in a certain order.
Because EventMgr is defined globally as an object (rather than a global
pointer to an EventMgr object), its pipe is created even before main()
is entered. This further means that in the fork-based supervisor setup,
all Zeek processes created from the top-level supervisor process share
the same pipe object for the EventMgr. In turn, whenever any of the
processes enqueued an event, the flare was fired and ready for reading
on all other processes in the cluster, causing much contention and
unneeded overhead.
Closes#3190
* jgras/topic/jgras/event-ts:
Add compatibility tests for timestamped events.
Add timestamps to auto published broker events.
Add timestamps to manually published broker events.
Annotate scheduled events with intended timestamp.
Add timestamp to events.
One timestamp to ts rename during the merge.
The whole docs read like this was only used to update the
network_time, so there may be a follow-up to ditch EventMgr
even being an IOSource (which could be argued it's not IO).
This also changes the argument type of Func::operator() to zeek::Args*
to allow plugins to be able to alter function arguments in place as
was previously documented.
