diff --git a/eden/common/telemetry/LogEvent.h b/eden/common/telemetry/LogEvent.h index 742bee8..8c82692 100644 --- a/eden/common/telemetry/LogEvent.h +++ b/eden/common/telemetry/LogEvent.h @@ -19,6 +19,11 @@ struct TypedEvent { virtual const char* getType() const = 0; }; +struct TypelessEvent { + virtual ~TypelessEvent() = default; + virtual void populate(DynamicEvent&) const = 0; +}; + // Used for unit testing struct TestEvent : public TypedEvent { // Keep populate() and getType() pure virtual to force subclasses @@ -27,4 +32,10 @@ struct TestEvent : public TypedEvent { virtual const char* getType() const override = 0; }; +// Used for unit testing +struct TypelessTestEvent : public TypelessEvent { + // Keep populate() pure virtual to force subclasses to implement it + virtual void populate(DynamicEvent&) const override = 0; +}; + } // namespace facebook::eden diff --git a/eden/common/telemetry/StructuredLogger.cpp b/eden/common/telemetry/StructuredLogger.cpp index c6c3f00..f0bcffb 100644 --- a/eden/common/telemetry/StructuredLogger.cpp +++ b/eden/common/telemetry/StructuredLogger.cpp @@ -25,13 +25,16 @@ StructuredLogger::StructuredLogger(bool enabled, SessionInfo sessionInfo) sessionId_{getSessionId()}, sessionInfo_{std::move(sessionInfo)} {} -DynamicEvent StructuredLogger::populateDefaultFields(const char* type) { +DynamicEvent StructuredLogger::populateDefaultFields( + std::optional type) { DynamicEvent event; if (kExplicitTimeField) { event.addInt("time", ::time(nullptr)); } event.addInt("session_id", sessionId_); - event.addString("type", type); + if (type.has_value()) { + event.addString("type", *type); + } event.addString("user", sessionInfo_.username); event.addString("host", sessionInfo_.hostname); event.addString("os", sessionInfo_.os); diff --git a/eden/common/telemetry/StructuredLogger.h b/eden/common/telemetry/StructuredLogger.h index 7782fe4..680a28a 100644 --- a/eden/common/telemetry/StructuredLogger.h +++ b/eden/common/telemetry/StructuredLogger.h @@ -20,6 +20,18 @@ class StructuredLogger { explicit StructuredLogger(bool enabled, SessionInfo sessionInfo); virtual ~StructuredLogger() = default; + void logEvent(const TypelessEvent& event) { + // Avoid a bunch of work if it's going to be thrown away by the + // logDynamicEvent implementation. + if (!enabled_) { + return; + } + + DynamicEvent de{populateDefaultFields(std::nullopt)}; + event.populate(de); + logDynamicEvent(std::move(de)); + } + void logEvent(const TypedEvent& event) { // Avoid a bunch of work if it's going to be thrown away by the // logDynamicEvent implementation. @@ -35,7 +47,7 @@ class StructuredLogger { protected: virtual void logDynamicEvent(DynamicEvent event) = 0; - virtual DynamicEvent populateDefaultFields(const char* type); + virtual DynamicEvent populateDefaultFields(std::optional type); bool enabled_; uint32_t sessionId_; diff --git a/eden/common/telemetry/test/ScubaStructuredLoggerTest.cpp b/eden/common/telemetry/test/ScubaStructuredLoggerTest.cpp index 6033f76..ef5d144 100644 --- a/eden/common/telemetry/test/ScubaStructuredLoggerTest.cpp +++ b/eden/common/telemetry/test/ScubaStructuredLoggerTest.cpp @@ -43,6 +43,19 @@ struct TestLogEvent : public TestEvent { } }; +struct TypelessTestLogEvent : public TypelessTestEvent { + std::string str; + int number = 0; + + TypelessTestLogEvent(std::string str, int number) + : str(std::move(str)), number(number) {} + + void populate(DynamicEvent& event) const override { + event.addString("str", str); + event.addInt("number", number); + } +}; + struct ScubaStructuredLoggerTest : public ::testing::Test { std::shared_ptr scribe{ std::make_shared()}; @@ -96,3 +109,32 @@ TEST_F(ScubaStructuredLoggerTest, json_contains_types_at_top_level_and_values) { UnorderedElementsAre("str", "user", "host", "type", "os", "osver")); #endif } + +TEST_F( + ScubaStructuredLoggerTest, + typeless_json_doesnt_contain_type_at_top_level) { + logger.logEvent(TypelessTestLogEvent{"different name", 12}); + EXPECT_EQ(1, scribe->lines.size()); + const auto& line = scribe->lines[0]; + auto doc = folly::parseJson(line); + EXPECT_TRUE(doc.isObject()); + EXPECT_THAT(keysOf(doc), UnorderedElementsAre("int", "normal")); + + auto ints = doc["int"]; + EXPECT_TRUE(ints.isObject()); + EXPECT_THAT( + keysOf(ints), UnorderedElementsAre("time", "number", "session_id")); + + auto normals = doc["normal"]; + EXPECT_TRUE(normals.isObject()); +#if defined(__APPLE__) + EXPECT_THAT( + keysOf(normals), + UnorderedElementsAre( + "str", "user", "host", "os", "osver", "system_architecture")); +#else + EXPECT_THAT( + keysOf(normals), + UnorderedElementsAre("str", "user", "host", "os", "osver")); +#endif +}