broker icon indicating copy to clipboard operation
broker copied to clipboard

Roadmap for Zeek 5.0

Open rsmmr opened this issue 4 years ago • 10 comments

This is a meta ticket tracking next steps.

For Zeek 5.0:

  • [x] Bring back OpenSSL in #201.
  • [x] Fix shutdown glitches in #201
  • [x] Merge #201 (w/ reduced ALM functionality)
  • [x] Fix UBSan "vptr" check failures (https://github.com/zeek/zeek/issues/1176)
  • [x] Add Websocket-based client API for event submission/subscription, using new serialization (#219)
  • [x] Check whether we can disable any warnings from CAF. That one is probably tricky, because we are building CAF as part of Broker. Hence, only suppressing warnings from CAF headers wouldn't be sufficient. This means we probably need to add a 'no-warnings' flag to CAF's CMake.
  • [x] Ensure Broker protocol will remain backwards compatible with future changes (e.g., serialization format switch)
  • [x] Clean up and triage existing tickets

After 5.0:

  • Switch Broker protocol to new serialization
  • Revisit need for Python bindings
  • Revisit full ALM
  • Clean up and modernize code where possible
  • Decide on new serialization format

rsmmr avatar Feb 21 '22 07:02 rsmmr

You might add https://github.com/zeek/zeek/issues/1743 here too.

timwoj avatar Feb 22 '22 20:02 timwoj

Yep, thanks for the pointer. That one fell off the radar as it seems. I'm not sure if there's anything we can do about it, as the stack trace indicates it's crashing in std::random_device. But I'll evaluate it after the ALM merge is done.

Neverlord avatar Feb 22 '22 20:02 Neverlord

Good point. It could be related to this: https://github.com/rr-debugger/rr/issues/2459, which appears to have been fixed a while ago. I'll comment on the zeek issue about it.

timwoj avatar Feb 22 '22 20:02 timwoj

Putting this as a reminder here, also probably for after 5.0 since it's just a 'nice-to-have':

  • Provide a JSON schema for the new WebSocket protocol.

Neverlord avatar May 20 '22 08:05 Neverlord

Revisit need for Python bindings.

With WebSockets available in 5.0 as language-agnostic way to communicate to Broker: why not deprecate the bindings with 5.0?

We currently have 6 tickets open that are related to the Python bindings. I think simply closing all of them as 'wontfix' is probably the best way to move forward. If we do want to make interfacing with Broker easier from Python, we could instead provide a couple of (pure Python) utility functions that streamline operating on the WebSocket messages by converting between the Broker JSON format and proper Python types.

Neverlord avatar May 26 '22 07:05 Neverlord

Yeah, I like that, and had indeed also already thought about providing a new Python module that would help with the WebSocket/JSON encoding.

However, I'd wait one version before deprecating the Python bindings: we're just adding WebSocket now with close-to-zero experience using it; let's give it a cycle before we recommend people move over to it for existing Python clients. That also allows the Python bindings to remain not-deprecated for the 5.0 LTS cycle, giving people plenty time.

So, I propose we do this with 5.1. I agree, though, that we then don't need to work on all those tickets in the meantime; we can just go into maintainance mode.

rsmmr avatar May 26 '22 09:05 rsmmr

That also allows the Python bindings to remain not-deprecated for the 5.0 LTS cycle, giving people plenty time.

Just for my understanding: what difference does it make to deprecate the bindings in 5.1 versus deprecating with 6.0? In both cases, we'll have to support the Python bindings until 7.0, or does the Zeek policy work differently?

Neverlord avatar May 26 '22 15:05 Neverlord

Check whether we can disable any warnings from CAF. That one is probably tricky, because we are building CAF as part of Broker. Hence, only suppressing warnings from CAF headers wouldn't be sufficient. This means we probably need to add a 'no-warnings' flag to CAF's CMake.

Zeek and Broker modify global CMake variables that are meant to capture user-configurations, e.g., CMAKE_CXX_FLAGS. Since CAF is integrated as sub-directory, it gets all the warning (and other) flags passed down automatically. Ideally, Zeek and Broker would leave the global flags alone and work with target properties instead. In the current setup, I don't see a sane way to 'drop' the flags that Zeek and Broker have added without ignoring all user-defined flags as well (which we shouldn't do IMHO).

Neverlord avatar Jun 03 '22 06:06 Neverlord

Ideally, Zeek and Broker would leave the global flags alone and work with target properties instead.

After my fight with c-ares, I completely agree with this. It's something I'd definitely like to fix about our CMake setup.

timwoj avatar Jun 03 '22 15:06 timwoj

That also allows the Python bindings to remain not-deprecated for the 5.0 LTS cycle, giving people plenty time.

Just for my understanding: what difference does it make to deprecate the bindings in 5.1 versus deprecating with 6.0? In both cases, we'll have to support the Python bindings until 7.0, or does the Zeek policy work differently?

That's correct. My point was that if we had deprecated them in 5.0 already, people would start getting warnings immediately from now on.

rsmmr avatar Jul 04 '22 08:07 rsmmr

Is there still something left todo here or can we close this ticket? We have a separate ticket for the Python binding removal / deprecation and the CMake overhaul is underway. 🙂

Neverlord avatar Feb 26 '23 10:02 Neverlord

Nope, I'm fine with closing it.

timwoj avatar Mar 02 '23 21:03 timwoj