Disallow logging timestamps without data
Nobody has come up with a good use case or a way to read the data
afterwards. Until we have a use case, it isn't worth spending the
engineering figuring out how to expose this all.
Instead, when validating configs, enforce that we can't log timestamps
without data. Logs with timestamps and no data are possible to read,
but need --skip_missing_forwarding_entries and are not recommended.
Change-Id: I04534a09cba1bb7f87c0e4e966a3e20ae2007568
Signed-off-by: Austin Schuh <austin.schuh@bluerivertech.com>
diff --git a/aos/configuration.cc b/aos/configuration.cc
index 2b9ac6d..a00c8e7 100644
--- a/aos/configuration.cc
+++ b/aos/configuration.cc
@@ -337,6 +337,19 @@
<< ", can only use [-a-zA-Z0-9_/]";
}
+ // There is no good use case today for logging timestamps but not the
+ // corresponding data. Instead of plumbing through all of this on the
+ // reader side, let'd just disallow it for now.
+ if (c->logger() == LoggerConfig::NOT_LOGGED &&
+ c->has_destination_nodes()) {
+ for (const Connection *d : *c->destination_nodes()) {
+ CHECK(d->timestamp_logger() == LoggerConfig::NOT_LOGGED)
+ << ": Logging timestamps without data is not supported. If you "
+ "have a good use case, let's talk. "
+ << CleanedChannelToString(c);
+ }
+ }
+
// Make sure everything is sorted while we are here... If this fails,
// there will be a bunch of weird errors.
if (last_channel != nullptr) {
diff --git a/aos/configuration_test.cc b/aos/configuration_test.cc
index c650a5d..0b842cb 100644
--- a/aos/configuration_test.cc
+++ b/aos/configuration_test.cc
@@ -866,6 +866,16 @@
EXPECT_THAT(result, ::testing::ElementsAreArray({0, 1, 0, 0}));
}
+// Tests that we reject invalid logging configurations.
+TEST_F(ConfigurationDeathTest, InvalidLoggerConfig) {
+ EXPECT_DEATH(
+ {
+ FlatbufferDetachedBuffer<Configuration> config =
+ ReadConfig(ArtifactPath("aos/testdata/invalid_logging_configuration.json"));
+ },
+ "Logging timestamps without data");
+}
+
} // namespace testing
} // namespace configuration
} // namespace aos
diff --git a/aos/testdata/BUILD b/aos/testdata/BUILD
index 2971664..51283c8 100644
--- a/aos/testdata/BUILD
+++ b/aos/testdata/BUILD
@@ -19,6 +19,7 @@
"invalid_channel_name2.json",
"invalid_channel_name3.json",
"invalid_destination_node.json",
+ "invalid_logging_configuration.json",
"invalid_nodes.json",
"invalid_source_node.json",
"self_forward.json",
diff --git a/aos/testdata/invalid_logging_configuration.json b/aos/testdata/invalid_logging_configuration.json
new file mode 100644
index 0000000..cb883b7
--- /dev/null
+++ b/aos/testdata/invalid_logging_configuration.json
@@ -0,0 +1,37 @@
+{
+ "channels": [
+ {
+ "name": "/foo",
+ "type": ".aos.bar",
+ "max_size": 5,
+ "source_node": "pi2",
+ "logger": "NOT_LOGGED",
+ "destination_nodes": [
+ {
+ "name": "pi1",
+ "timestamp_logger": "LOCAL_AND_REMOTE_LOGGER"
+ }
+ ]
+ },
+ {
+ "name": "/foo2",
+ "type": ".aos.bar",
+ "source_node": "pi1",
+ "destination_nodes": [
+ {
+ "name": "pi2"
+ }
+ ]
+ }
+ ],
+ "nodes": [
+ {
+ "name": "pi1",
+ "hostname": "raspberrypi1"
+ },
+ {
+ "name": "pi2",
+ "hostname": "raspberrypi2"
+ }
+ ]
+}