Skip to content

Commit 54df76e

Browse files
Chandhana Solainathanmeta-codesync[bot]
authored andcommitted
telemetry: Instrument Overlay error paths with structured error logging
Summary: Logs overlay errors to perfpipe_edenfs_errors when overlay operations fail Reviewed By: vilatto Differential Revision: D106151434 fbshipit-source-id: a55596dc1f8d86d46a8f60eb58045809721aa20a
1 parent 4b2405b commit 54df76e

5 files changed

Lines changed: 53 additions & 0 deletions

File tree

eden/fs/inodes/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ cpp_library(
166166
"//eden/fs/service:thrift_util",
167167
"//eden/fs/sqlite:sqlite",
168168
"//eden/fs/store:backing_store_interface",
169+
"//eden/fs/telemetry:error_info",
169170
"//eden/fs/telemetry:error_logger",
170171
"//eden/fs/telemetry:file_access_structured_logger",
171172
"//eden/fs/telemetry:fs_event_logger",

eden/fs/inodes/Overlay.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@
3535
#include "eden/fs/inodes/sqlitecatalog/BufferedSqliteInodeCatalog.h"
3636
#include "eden/fs/inodes/sqlitecatalog/SqliteInodeCatalog.h"
3737
#include "eden/fs/sqlite/SqliteDatabase.h"
38+
#include "eden/fs/telemetry/EdenErrorInfoBuilder.h"
3839
#include "eden/fs/telemetry/EdenFsEventsLogger.h"
3940
#include "eden/fs/telemetry/EdenStats.h"
41+
#include "eden/fs/telemetry/ErrorLogger.h"
4042
#include "eden/fs/telemetry/LogEvent.h"
4143

4244
#ifndef _WIN32
@@ -769,6 +771,7 @@ void Overlay::saveOverlayDir(
769771
stats_->increment(&OverlayStats::saveOverlayDirSuccessful);
770772
} catch (const std::exception& e) {
771773
XLOGF(ERR, "Failed to save overlay dir {} {}", inodeNumber, e.what());
774+
errorLogger_.log(EdenErrorInfo::overlay(e, inodeNumber.get()));
772775
stats_->increment(&OverlayStats::saveOverlayDirFailure);
773776
throw;
774777
}
@@ -1036,6 +1039,7 @@ OverlayFile Overlay::openFile(
10361039
return file;
10371040
} catch (const std::exception& e) {
10381041
XLOGF(ERR, "Failed to open file {} {} {}", inodeNumber, headerId, e.what());
1042+
errorLogger_.log(EdenErrorInfo::overlay(e, inodeNumber.get()));
10391043
stats_->increment(&OverlayStats::openOverlayFileFailure);
10401044
throw;
10411045
}
@@ -1052,6 +1056,7 @@ OverlayFile Overlay::openFileNoVerify(InodeNumber inodeNumber) {
10521056
return file;
10531057
} catch (const std::exception& e) {
10541058
XLOGF(ERR, "Failed to open file {} {}", inodeNumber, e.what());
1059+
errorLogger_.log(EdenErrorInfo::overlay(e, inodeNumber.get()));
10551060
stats_->increment(&OverlayStats::openOverlayFileFailure);
10561061
throw;
10571062
}
@@ -1074,6 +1079,7 @@ OverlayFile Overlay::createOverlayFile(
10741079
return file;
10751080
} catch (const std::exception& e) {
10761081
XLOGF(ERR, "Failed to create file {} {}", inodeNumber, e.what());
1082+
errorLogger_.log(EdenErrorInfo::overlay(e, inodeNumber.get()));
10771083
stats_->increment(&OverlayStats::createOverlayFileFailure);
10781084
throw;
10791085
}
@@ -1096,6 +1102,7 @@ OverlayFile Overlay::createOverlayFile(
10961102
return file;
10971103
} catch (const std::exception& e) {
10981104
XLOGF(ERR, "Failed to create file {} {}", inodeNumber, e.what());
1105+
errorLogger_.log(EdenErrorInfo::overlay(e, inodeNumber.get()));
10991106
stats_->increment(&OverlayStats::createOverlayFileFailure);
11001107
throw;
11011108
}

eden/fs/inodes/OverlayFileAccess.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include "eden/fs/inodes/OverlayFile.h"
2323
#include "eden/fs/inodes/TreeInode.h"
2424
#include "eden/fs/model/Blob.h"
25+
#include "eden/fs/telemetry/EdenErrorInfoBuilder.h"
26+
#include "eden/fs/telemetry/ErrorLogger.h"
2527

2628
namespace facebook::eden {
2729

@@ -140,6 +142,8 @@ FileOffset OverlayFileAccess::getFileSize(InodeNumber ino, InodeBase* inode) {
140142
"overlay file for {} is too short for header: size={}",
141143
ino,
142144
st.st_size);
145+
overlay_->getErrorLogger().log(
146+
EdenErrorInfo::overlay("corrupt overlay file", ino.get()));
143147
throw InodeError(
144148
EIO,
145149
inode ? inode->inodePtrFromThis() : InodePtr{},

eden/fs/inodes/test/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ cpp_unittest(
364364
"//eden/fs/service:pretty_printers",
365365
"//eden/fs/telemetry:stats",
366366
"//eden/fs/telemetry:structured_logger",
367+
"//eden/fs/telemetry/test:capturing_scribe_logger",
367368
"//eden/fs/testharness:fake_backing_store_and_tree_builder",
368369
"//eden/fs/testharness:test_checks",
369370
"//eden/fs/testharness:test_mount",

eden/fs/inodes/test/OverlayTest.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "eden/fs/service/PrettyPrinters.h"
3939
#include "eden/fs/telemetry/EdenFsEventsLogger.h"
4040
#include "eden/fs/telemetry/EdenStats.h"
41+
#include "eden/fs/telemetry/test/CapturingScribeLogger.h"
4142
#include "eden/fs/testharness/FakeBackingStore.h"
4243
#include "eden/fs/testharness/FakeTreeBuilder.h"
4344
#include "eden/fs/testharness/TestChecks.h"
@@ -558,6 +559,45 @@ TEST_P(RawOverlayTest, cannot_create_overlay_file_in_corrupt_overlay) {
558559
overlay->close();
559560
}
560561

562+
TEST(OverlayErrorLoggingTest, createOverlayFileLogsErrorOnFailure) {
563+
folly::test::TemporaryDirectory testDir;
564+
auto scribe = std::make_shared<CapturingScribeLogger>();
565+
auto edenConfig = EdenConfig::createTestEdenConfig();
566+
edenConfig->enableErrorLogging.setValue(
567+
true, ConfigSourceType::Default, true);
568+
auto config = std::make_shared<ReloadableConfig>(edenConfig);
569+
ErrorLogger errorLogger(scribe, SessionInfo{}, config);
570+
571+
auto overlay = Overlay::create(
572+
canonicalPath(testDir.path().string()),
573+
kPathMapDefaultCaseSensitive,
574+
kInodeCatalogType,
575+
kInodeCatalogOptions,
576+
makeTestEdenFsEventsLogger(),
577+
errorLogger,
578+
makeRefPtr<EdenStats>(),
579+
*EdenConfig::createTestEdenConfig());
580+
overlay
581+
->initialize(
582+
std::make_shared<ReloadableConfig>(
583+
EdenConfig::createTestEdenConfig()))
584+
.get();
585+
586+
auto ino = overlay->allocateInodeNumber();
587+
588+
// Remove the overlay directory to force createOverlayFile to fail
589+
boost::filesystem::remove_all(testDir.path());
590+
591+
EXPECT_THROW(
592+
overlay->createOverlayFile(ino, folly::ByteRange{"contents"_sp}),
593+
std::system_error);
594+
595+
ASSERT_EQ(scribe->messages().size(), 1);
596+
const auto& msg = scribe->messages()[0];
597+
EXPECT_NE(msg.find("overlay"), std::string::npos)
598+
<< "Should contain overlay component, got: " << msg;
599+
}
600+
561601
TEST_P(RawOverlayTest, cannot_save_overlay_dir_when_closed) {
562602
overlay->close();
563603
auto ino2 = overlay->allocateInodeNumber();

0 commit comments

Comments
 (0)