FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #1740 from theopolis/events_improvements · TomA-R/osquery@c2b78fa · GitHub

/ osquery Public
forked from osquery/osquery

Commit c2b78fa

Browse files
Teddy Reed
committed
Merge pull request osquery#1740 from theopolis/events_improvements
Fix double event subscriber select
2 parents ef5ee38 + c4f3db1 commit c2b78fa

3 files changed

Lines changed: 55 additions & 20 deletions

File tree

‎include/osquery/events.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ class EventSubscriberPlugin : public Plugin {
513513
FRIEND_TEST(EventsDatabaseTests, test_record_indexing);
514514
FRIEND_TEST(EventsDatabaseTests, test_record_range);
515515
FRIEND_TEST(EventsDatabaseTests, test_record_expiration);
516+
FRIEND_TEST(EventsDatabaseTests, test_gentable);
516517
friend class BenchmarkEventSubscriber;
517518
};
518519

‎osquery/events/events.cpp‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ std::set<std::string> EventSubscriberPlugin::getIndexes(EventTime start,
141141

142142
std::string time_list;
143143
auto list_type = boost::lexical_cast<std::string>(size);
144-
auto status = db->Get(kEvents, index_key + "." + list_type, time_list);
145-
if (time_list.length() == 0) {
144+
db->Get(kEvents, index_key + "." + list_type, time_list);
145+
if (time_list.empty()) {
146146
// No events in this binning size.
147147
return indexes;
148148
}
@@ -186,6 +186,7 @@ std::set<std::string> EventSubscriberPlugin::getIndexes(EventTime start,
186186
size_t bin_stop = size * (step + 1);
187187
if (expire_events_ && expire_time_ > 0) {
188188
if (bin_stop <= expire_time_) {
189+
// This entire bin will be expired.
189190
expirations.push_back(bin);
190191
} else if (bin_start < expire_time_) {
191192
expireRecords(list_type, bin, false);
@@ -200,11 +201,11 @@ std::set<std::string> EventSubscriberPlugin::getIndexes(EventTime start,
200201
}
201202

202203
// Rewrite the index lists and delete each expired item.
203-
if (expirations.size() > 0) {
204+
if (!expirations.empty()) {
204205
expireIndexes(list_type, all_bins, expirations);
205206
}
206207

207-
if (bins.size() != 0) {
208+
if (!bins.empty()) {
208209
// If more precision was achieved though this list's binning.
209210
local_start = timeFromRecord(bins.front()) * size;
210211
start_max = (local_start < start_max) ? local_start : start_max;
@@ -286,11 +287,8 @@ std::vector<EventRecord> EventSubscriberPlugin::getRecords(
286287
std::vector<std::string> bin_records;
287288
{
288289
std::string record_value;
289-
if (!db->Get(kEvents, record_key + "." + index, record_value).ok()) {
290-
return records;
291-
}
292-
293-
if (record_value.length() == 0) {
290+
db->Get(kEvents, record_key + "." + index, record_value);
291+
if (record_value.empty()) {
294292
// There are actually no events in this bin, interesting error case.
295293
continue;
296294
}
@@ -395,17 +393,9 @@ EventID EventSubscriberPlugin::getEventID() {
395393

396394
QueryData EventSubscriberPlugin::get(EventTime start, EventTime stop) {
397395
QueryData results;
398-
Status status;
399-
400-
std::shared_ptr<DBHandle> db = nullptr;
401-
try {
402-
db = DBHandle::getInstance();
403-
} catch (const std::runtime_error& e) {
404-
LOG(ERROR) << "Cannot retrieve subscriber results database is locked";
405-
return results;
406-
}
407396

408397
// Get the records for this time range.
398+
auto db = DBHandle::getInstance();
409399
auto indexes = getIndexes(start, stop);
410400
auto records = getRecords(indexes);
411401
std::string events_key = "data." + dbNamespace();
@@ -421,7 +411,7 @@ QueryData EventSubscriberPlugin::get(EventTime start, EventTime stop) {
421411
std::string data_value;
422412
for (const auto& record : mapped_records) {
423413
Row r;
424-
status = db->Get(kEvents, record, data_value);
414+
auto status = db->Get(kEvents, record, data_value);
425415
if (data_value.length() == 0) {
426416
// There is no record here, interesting error case.
427417
continue;

‎osquery/events/tests/events_database_tests.cpp‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,18 @@
1313

1414
#include <gtest/gtest.h>
1515

16+
#include <osquery/core.h>
17+
#include <osquery/database.h>
1618
#include <osquery/events.h>
19+
#include <osquery/flags.h>
1720
#include <osquery/tables.h>
1821

1922
#include "osquery/database/db_handle.h"
2023

2124
namespace osquery {
2225

26+
DECLARE_uint64(events_expiry);
27+
2328
class EventsDatabaseTests : public ::testing::Test {};
2429

2530
class DBFakeEventPublisher
@@ -68,9 +73,10 @@ TEST_F(EventsDatabaseTests, test_record_indexing) {
6873
auto output = boost::algorithm::join(indexes, ", ");
6974
EXPECT_EQ(output, "3600.0, 3600.1, 3600.2");
7075

71-
// Restrict range to "most specific".
76+
// Restrict range to "most specific", which is an index by 10.
7277
indexes = sub->getIndexes(0, 5);
7378
output = boost::algorithm::join(indexes, ", ");
79+
// The order 10, 0th index include results with t = [0, 10).
7480
EXPECT_EQ(output, "10.0");
7581

7682
// Get a mix of indexes for the lower bounding.
@@ -79,6 +85,7 @@ TEST_F(EventsDatabaseTests, test_record_indexing) {
7985
EXPECT_EQ(output, "10.0, 10.1, 3600.1, 3600.2, 60.1");
8086

8187
// Rare, but test ONLY intermediate indexes.
88+
// Provide an optional third parameter to getIndexes: 1 = 10,(60),3600.
8289
indexes = sub->getIndexes(2, (3 * 3600), 1);
8390
output = boost::algorithm::join(indexes, ", ");
8491
EXPECT_EQ(output, "60.0, 60.1, 60.120, 60.60");
@@ -148,9 +155,46 @@ TEST_F(EventsDatabaseTests, test_record_expiration) {
148155
EXPECT_EQ(records.size(), 3U); // 11, 61, 3601
149156

150157
// Check that get/deletes did not act on cache.
158+
// This implies that RocksDB is flushing the requested delete records.
151159
sub->expire_time_ = 0;
152160
indexes = sub->getIndexes(0, 5000);
153161
records = sub->getRecords(indexes);
154162
EXPECT_EQ(records.size(), 3U); // 11, 61, 3601
155163
}
164+
165+
TEST_F(EventsDatabaseTests, test_gentable) {
166+
auto sub = std::make_shared<DBFakeEventSubscriber>();
167+
ASSERT_EQ(sub->optimize_time_, 0U);
168+
ASSERT_EQ(sub->expire_time_, 0U);
169+
170+
sub->testAdd(getUnixTime() - 1);
171+
sub->testAdd(getUnixTime());
172+
sub->testAdd(getUnixTime() + 1);
173+
174+
// Test the expire workflow by creating a short expiration time.
175+
FLAGS_events_expiry = 10;
176+
177+
std::vector<std::string> keys;
178+
scanDatabaseKeys("events", keys);
179+
EXPECT_GT(keys.size(), 10U);
180+
181+
// Perform a "select" equivalent.
182+
QueryContext context;
183+
auto results = sub->genTable(context);
184+
// Expect all non-expired results: 11, +
185+
EXPECT_EQ(results.size(), 9U);
186+
// The expiration time is now - events_expiry.
187+
EXPECT_GT(sub->expire_time_, getUnixTime() - (FLAGS_events_expiry * 2));
188+
EXPECT_LT(sub->expire_time_, getUnixTime());
189+
190+
results = sub->genTable(context);
191+
EXPECT_EQ(results.size(), 3U);
192+
193+
results = sub->genTable(context);
194+
EXPECT_EQ(results.size(), 3U);
195+
196+
keys.clear();
197+
scanDatabaseKeys("events", keys);
198+
EXPECT_LT(keys.size(), 30U);
199+
}
156200
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL