Merge remote-tracking branch 'origin/topic/awelzel/record-field-redef'

* origin/topic/awelzel/record-field-redef:
  Support redef'ing the &log attribute of record fields

CHANGES

@@ -1,3 +1,33 @@
+5.1.0-dev.364 | 2022-08-11 11:59:00 +0200
+
+  * GH-2000: Support redef'ing the &log attribute of record fields (Arne Welzel, Corelight)
+
+    Add new syntax for adding and removing attributes from record fields:
+
+        redef RecordType$field_name += { &log };
+        redef RecordType$field_name -= { &log };
+
+    For now this only allowed for the &log attribute as the semantics are clear.
+    For &default and &optional the semantics aren't obvious and no use-cases have
+    been identified where those would make sense to change.
+
+    This enables a mechanism to add potentially interesting fields to the typical
+    Info records in base scripts, but letting users opt-into actually including
+    them into their log. At the same time, users that find specific fields in a
+    standard log uninteresting can opt-out without using `Log::Filter$exclude`
+    which can be difficult to use correctly. Patching or forking external packages
+    to remove columns from a log can also be avoided with this mechanism.
+
+    Closes #2000.
+
+  * GH-2262: telemetry: Remove singleton BIFs and the C++ pieces (Arne Welzel, Corelight)
+
+    The low-level singleton Telemetry BIFs have been removed with the that there
+    haven't been any users. Singleton metrics can be instantiated by providing
+    an empty label vector instead and aren't in any way a special concept.
+
+    Closes #2262.
+
 5.1.0-dev.360 | 2022-08-11 11:52:26 +0200
 
   * telemetry: Switch to histogram family bucket and bound accessors

NEWS

@@ -49,6 +49,14 @@ New Functionality
   For further details on the framework and examples, please refer to the
   Zeek documentation.
 
+- Allow redef'ing the ``&log`` attribute of record fields:
+
+	redef Notice::Info$email_dest -= { &log };
+
+  While the syntax allows for any attribute, only ``&log`` is supported. The
+  semantics for other record field attributes are not easy to grasp and there
+  were no obvious use-cases identified.
+
 Changed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-5.1.0-dev.360
+5.1.0-dev.364

src/parse.y

@@ -189,6 +189,56 @@ static void parse_redef_enum(ID* id)
 		}
 	}
 
+static void parse_redef_record_field(ID* id, const char* field, InitClass ic,
+                                     std::unique_ptr<std::vector<AttrPtr>> attrs)
+	{
+	if ( ! id->GetType() )
+		{
+		reporter->FatalError("unknown record identifier \"%s\"", id->Name());
+		return;
+		}
+
+	auto t = id->GetType();
+	if ( ! t || t->Tag() != TYPE_RECORD )
+		{
+		reporter->FatalError("identifier \"%s\" has type \"%s\", expected \"record\"",
+		                     id->Name(), type_name(t->Tag()));
+		return;
+		}
+
+	auto rt = t->AsRecordType();
+	auto idx = rt->FieldOffset(field);
+	if ( idx < 0 )
+		{
+		reporter->FatalError("field \"%s\" not in record \"%s\"", field, id->Name());
+		return;
+		}
+
+	auto decl = rt->FieldDecl(idx);
+	if ( ! decl->attrs )
+		if ( ic == INIT_EXTRA )
+			decl->attrs = make_intrusive<detail::Attributes>(decl->type,
+															 true /* in_record */,
+															 false /* is_global */);
+
+	for ( const auto& attr : *attrs )
+		{
+		// At this point, only support &log redef'ing.
+		if ( attr->Tag() != ATTR_LOG )
+			{
+				reporter->FatalError("Can only redef \"&log\" attributes of record fields");
+				return;
+			}
+
+		if ( ic == INIT_EXTRA )
+			decl->attrs->AddAttr(attr, true /* is_redef */);
+		else
+			// Removing attributes is a noop if they don't exist.
+			if ( decl->attrs )
+				decl->attrs->RemoveAttr(attr->Tag());
+		}
+	}
+
 static void extend_record(ID* id, std::unique_ptr<type_decl_list> fields,
                           std::unique_ptr<std::vector<AttrPtr>> attrs)
 	{
@@ -1306,6 +1356,20 @@ decl:
 			// Zeekygen already grabbed new enum IDs as the type created them.
 			}
 
+	|	TOK_REDEF TOK_RECORD  global_id '$' TOK_ID
+			{ cur_decl_type_id = $3; zeekygen_mgr->Redef($3, ::filename, INIT_EXTRA); }
+		TOK_ADD_TO '{' attr_list '}' ';'
+			{
+			cur_decl_type_id = 0;
+			parse_redef_record_field($3, $5, INIT_EXTRA, std::unique_ptr<std::vector<AttrPtr>>($9));
+			}
+	|	TOK_REDEF TOK_RECORD  global_id '$' TOK_ID
+			{ cur_decl_type_id = $3; zeekygen_mgr->Redef($3, ::filename, INIT_REMOVE); }
+		TOK_REMOVE_FROM '{' attr_list '}' ';'
+			{
+			cur_decl_type_id = 0;
+			parse_redef_record_field($3, $5, INIT_REMOVE, std::unique_ptr<std::vector<AttrPtr>>($9));
+			}
 	|	TOK_REDEF TOK_RECORD global_id
 			{ cur_decl_type_id = $3; zeekygen_mgr->Redef($3, ::filename); }
 		TOK_ADD_TO '{'

testing/btest/Baseline/core.record-field-redef-errors-10/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 1: field "no_such_field" not in record "M::Info"

testing/btest/Baseline/core.record-field-redef-errors-11/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 1: field "no_such_field" not in record "M::Info"

testing/btest/Baseline/core.record-field-redef-errors-12/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/record-field-redef-errors.zeek, line 2: syntax error, at or near "&"

testing/btest/Baseline/core.record-field-redef-errors-2/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/record-field-redef-errors.zeek, line 2: syntax error, at or near ";"

testing/btest/Baseline/core.record-field-redef-errors-3/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/record-field-redef-errors.zeek, line 2: syntax error, at or near "["

testing/btest/Baseline/core.record-field-redef-errors-4/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 2: Can only redef "&log" attributes of record fields

testing/btest/Baseline/core.record-field-redef-errors-5/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 2: Can only redef "&log" attributes of record fields

testing/btest/Baseline/core.record-field-redef-errors-6/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 2: Can only redef "&log" attributes of record fields

testing/btest/Baseline/core.record-field-redef-errors-7/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 2: identifier "M::ErrCode" has type "enum", expected "record"

testing/btest/Baseline/core.record-field-redef-errors-8/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 1: unknown record identifier "M::Unknown"

testing/btest/Baseline/core.record-field-redef-errors-9/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+fatal error in <...>/record-field-redef-errors.zeek, line 1: unknown record identifier "M::Unknown"

testing/btest/Baseline/core.record-field-redef-errors/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/record-field-redef-errors.zeek, line 6: syntax error, at or near "&log"

testing/btest/Baseline/core.record-field-redef/output

@@ -0,0 +1,10 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+Info record_fields\x0a, {
+[extra1] = [type_name=string, log=F, value=<uninitialized>, default_val=<uninitialized>],
+[extra2] = [type_name=string, log=F, value=<uninitialized>, default_val=<uninitialized>],
+[ts] = [type_name=time, log=T, value=<uninitialized>, default_val=<uninitialized>],
+[extra3] = [type_name=string, log=T, value=<uninitialized>, default_val=<uninitialized>],
+[msg] = [type_name=string, log=T, value=<uninitialized>, default_val=<uninitialized>],
+[extra4] = [type_name=string, log=T, value=<uninitialized>, default_val=<uninitialized>]
+}
+Info record, [ts=XXXXXXXXXX.XXXXXX, msg=msg, extra1=extra1 value, extra2=extra2 value, extra3=extra3 value, extra4=extra4 value]

testing/btest/Baseline/core.record-field-redef/test.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	test
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	msg	extra3	extra4
+#types	time	string	string	string
+XXXXXXXXXX.XXXXXX	msg	extra3 value	extra4 value
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/core/record-field-redef-errors.zeek

@@ -0,0 +1,63 @@
+# @TEST-DOC: redef record Record$field testing
+# @TEST-EXEC-FAIL: zeek -b setup.zeek %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+# Bad syntax
+redef record M::Info$ts -= &log;
+
+@TEST-START-NEXT
+# Really bad syntax
+redef record M::Info$ts -= { &log;
+
+@TEST-START-NEXT
+# Not the right syntax
+redef record M::Info$ts -= [ &log ];
+
+@TEST-START-NEXT
+# Can not make something optional
+redef record M::Info$ts += { &optional };
+
+@TEST-START-NEXT
+# Can not add &default
+redef record M::Info$addl += { &log &default="default"};
+
+@TEST-START-NEXT
+# Can not remove optional
+redef record M::Info$msg -= { &log &optional };
+
+@TEST-START-NEXT
+# Not a record
+redef record M::ErrCode$msg += { &log };
+
+@TEST-START-NEXT
+redef record M::Unknown$ts += { &log };
+
+@TEST-START-NEXT
+redef record M::Unknown$ts -= { &log };
+
+@TEST-START-NEXT
+redef record M::Info$no_such_field += { &log };
+
+@TEST-START-NEXT
+redef record M::Info$no_such_field -= { &log };
+
+@TEST-START-NEXT
+# This isn't reported very nicely: It's a syntax error rather than an unknown attribute
+redef record M::Info$ts += { &unknown };
+
+# @TEST-START-FILE setup.zeek
+module M;
+export {
+	type ErrCode: enum {
+		ECONFUSING,
+	};
+
+	type Info: record {
+		ts:   time   &log;
+		peer: string &log &default="zeek";
+		msg:  string &log &optional;
+		addl: string &optional;
+		noattrs: string;
+	};
+}
+# @TEST-END-FILE

testing/btest/core/record-field-redef.zeek

@@ -0,0 +1,56 @@
+# @TEST-DOC: Redef'ing of record fields for adding and removing &log from them.
+# @TEST-EXEC: zeek -b %INPUT >output
+# @TEST-EXEC: btest-diff output
+# @TEST-EXEC: btest-diff test.log
+
+module RedefRecordTest;
+
+export {
+        redef enum Log::ID += { LOG };
+
+        type Info: record {
+                ts: time &log;
+                msg: string &log;
+                extra1: string &log &optional;
+                extra2: string &optional;
+                extra3: string &optional;
+                extra4: string &optional;
+        };
+}
+
+# Adding an extra &log is fine: Making something &log by default
+# shouldn't break users.
+redef record Info$msg += { &log };
+
+# Don't log extra1
+redef record Info$extra1 -= { &log };
+
+# Don't log extra2 (default, but remove &log) again
+redef record Info$extra2 -= { &log };
+
+# Do log extra3
+redef record Info$extra3 += { &log };
+
+# Redef extra4 from global scope (remove and re-add &log)
+module GLOBAL;
+redef record RedefRecordTest::Info$extra4 -= { &log };
+redef record RedefRecordTest::Info$extra4 += { &log };
+
+module RedefRecordTest;
+
+# zeek_init() for testing of print and logging.
+event zeek_init()
+	{
+        print "Info record_fields\n", record_fields(Info);
+        local rec = Info(
+		$ts=double_to_time(1660142487.54),
+		$msg="msg",
+		$extra1="extra1 value",
+		$extra2="extra2 value",
+		$extra3="extra3 value",
+		$extra4="extra4 value"
+	);
+        print "Info record", rec;
+        Log::create_stream(LOG, [$columns=Info, $path="test"]);
+        Log::write(LOG, rec);
+	}