On Sat, Nov 3, 2018 at 9:14 PM Vern Paxson <v...@corelight.com> wrote:
> Thanks for the pointers & thoughts! A quick question, more in a bit: > > > To better understand the existing behavior, here's the commit that > > introduced this (specifically with regards to conn_id): > > > https://github.com/bro/bro/commit/38a1aa5a346d10de32f9b40e0869cdb48a98974b > > ... > > > Note that for nested record types, the inner fields must likewise > > > be declared with &log. Consequently, conn_id is now declared with > > > &log in bro.init. > > Does your understanding of this accord with the current behavior when > running on testing/btest/scripts/base/frameworks/logging/attr.bro ? > The test suite result has it not logging Log$id, even though it's of > type conn_id, which has &log. (For my new version, it does log it.) > Hmm. I had to think about this for a bit, and I think it does agree with the commit message. It's rather subtle, but because the message talks about how the fields "must likewise be declared with &log," I can see how the expectation would be that *both* the conn_id declaration in init-bare and the usage in your record need to have the &log keyword to be logged. However, before reading that commit message, that was not my expectation for how Bro would behave. I've been playing around with this a bit more, and I think that what's described in the commit message is not the current behavior. Specifically, the following seem to behave the same: type conn_id: record { > orig_h: addr; > orig_p: port; > resp_h: addr; > resp_p: port; > } &log; > type conn_id: record { > orig_h: addr &log; > orig_p: port &log; > resp_h: addr &log; > resp_p: port &log; > }; > This example demonstrates that all fields are still logged: http://try.bro.org/#/trybro/saved/275829 In my mind, if the keyword is applied to a record, I would expect any new fields added to that record to also be logged. However, if I use conn_id as defined in init-bare (with &log applied to the record), and I redef conn_id as follows, it will not log the new field: redef record conn_id += { > nolog: bool &optional; > } > I believe that applying &log to a record is just shorthand to applying it individually to all fields on that record, whenever you define or redef that record. Simply put, I think the current behavior is not correct, and that we should take this opportunity to determine what the behavior *should* be. --Vlad
_______________________________________________ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
