Skip to content

Commit f61baf2

Browse files
Chandhana Solainathanmeta-codesync[bot]
authored andcommitted
telemetry: Add structured error logging to SaplingBackingStore layer
Summary: Threads ErrorLogger through SaplingBackingStore so that backing store fetch failures can be logged to perfpipe_edenfs_errors. Reviewed By: vilatto Differential Revision: D105073799 fbshipit-source-id: d91114567bcc5e1979a86f3e19467c5004711287
1 parent 54df76e commit f61baf2

8 files changed

Lines changed: 23 additions & 0 deletions

File tree

eden/fs/service/EdenMain.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ std::shared_ptr<SaplingBackingStore> createSaplingBackingStore(
193193
reloadableConfig,
194194
std::move(runtimeOptions),
195195
params.serverState->getEdenFsEventsLogger(),
196+
params.serverState->getErrorLogger(),
196197
std::make_unique<BackingStoreLogger>(
197198
params.serverState->getEdenFsEventsLogger(),
198199
params.serverState->getProcessInfoCache()),

eden/fs/store/sl/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ cpp_library(
8686
"//eden/fs/store:context",
8787
"//eden/fs/store:store",
8888
"//eden/fs/telemetry:activity_buffer",
89+
"//eden/fs/telemetry:error_logger",
8990
"//eden/scm/lib/backingstore:backingstore", # @manual
9091
"//eden/scm/lib/backingstore:backingstore@header", # @manual
9192
"//eden/scm/lib/backingstore:sapling_native_backingstore",

eden/fs/store/sl/SaplingBackingStore.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,15 @@ SaplingBackingStore::SaplingBackingStore(
118118
std::shared_ptr<ReloadableConfig> config,
119119
std::unique_ptr<SaplingBackingStoreOptions> runtimeOptions,
120120
std::shared_ptr<EdenFsEventsLogger> edenFsEventsLogger,
121+
ErrorLogger& errorLogger,
121122
std::unique_ptr<BackingStoreLogger> logger,
122123
FaultInjector* FOLLY_NONNULL faultInjector)
123124
: stats_(stats.copy()),
124125
config_(config),
125126
serverThreadPool_(serverThreadPool),
126127
queue_(std::move(config)),
127128
edenFsEventsLogger_{std::move(edenFsEventsLogger)},
129+
errorLogger_(errorLogger),
128130
logger_(std::move(logger)),
129131
faultInjector_{*faultInjector},
130132
runtimeOptions_(computeRuntimeOptions(std::move(runtimeOptions))),
@@ -187,13 +189,15 @@ SaplingBackingStore::SaplingBackingStore(
187189
std::shared_ptr<ReloadableConfig> config,
188190
std::unique_ptr<SaplingBackingStoreOptions> runtimeOptions,
189191
std::shared_ptr<EdenFsEventsLogger> edenFsEventsLogger,
192+
ErrorLogger& errorLogger,
190193
std::unique_ptr<BackingStoreLogger> logger,
191194
FaultInjector* FOLLY_NONNULL faultInjector)
192195
: stats_(std::move(stats)),
193196
config_(config),
194197
serverThreadPool_(executor),
195198
queue_(std::move(config)),
196199
edenFsEventsLogger_{std::move(edenFsEventsLogger)},
200+
errorLogger_(errorLogger),
197201
logger_(std::move(logger)),
198202
faultInjector_{*faultInjector},
199203
runtimeOptions_(std::move(runtimeOptions)),

eden/fs/store/sl/SaplingBackingStore.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class BackingStoreLogger;
6666
class ReloadableConfig;
6767
class UnboundedQueueExecutor;
6868
class EdenStats;
69+
class ErrorLogger;
6970
class SaplingImportRequest;
7071
class EdenFsEventsLogger;
7172
class FaultInjector;
@@ -185,6 +186,7 @@ class SaplingBackingStore final
185186
std::shared_ptr<ReloadableConfig> config,
186187
std::unique_ptr<SaplingBackingStoreOptions> runtimeOptions,
187188
std::shared_ptr<EdenFsEventsLogger> edenFsEventsLogger,
189+
ErrorLogger& errorLogger,
188190
std::unique_ptr<BackingStoreLogger> logger,
189191
FaultInjector* FOLLY_NONNULL faultInjector);
190192

@@ -203,6 +205,7 @@ class SaplingBackingStore final
203205
std::shared_ptr<ReloadableConfig> config,
204206
std::unique_ptr<SaplingBackingStoreOptions> runtimeOptions,
205207
std::shared_ptr<EdenFsEventsLogger> edenFsEventsLogger,
208+
ErrorLogger& errorLogger,
206209
std::unique_ptr<BackingStoreLogger> logger,
207210
FaultInjector* FOLLY_NONNULL faultInjector);
208211

@@ -775,6 +778,7 @@ class SaplingBackingStore final
775778
std::vector<std::thread> threads_;
776779

777780
std::shared_ptr<EdenFsEventsLogger> edenFsEventsLogger_;
781+
ErrorLogger& errorLogger_;
778782

779783
/**
780784
* Logger for backing store imports

eden/fs/store/sl/test/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ cpp_unittest(
4141
"//eden/fs/store/sl:sapling_import_request",
4242
"//eden/fs/store/sl:sapling_import_request_queue",
4343
"//eden/fs/store/sl:sapling_object_id",
44+
"//eden/fs/telemetry:error_logger",
4445
"//eden/fs/telemetry:stats",
4546
"//eden/fs/telemetry:structured_logger",
4647
"//eden/fs/testharness:hg_repo",

eden/fs/store/sl/test/SaplingBackingStoreTest.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "eden/fs/store/sl/SaplingBackingStoreOptions.h"
3131
#include "eden/fs/telemetry/EdenFsEventsLogger.h"
3232
#include "eden/fs/telemetry/EdenStats.h"
33+
#include "eden/fs/telemetry/ErrorLogger.h"
3334
#include "eden/fs/testharness/HgRepo.h"
3435
#include "eden/fs/testharness/TestConfigSource.h"
3536
#include "eden/scm/lib/backingstore/include/SaplingBackingStoreError.h"
@@ -94,6 +95,7 @@ struct SaplingBackingStoreTestBase : TestRepo, ::testing::Test {
9495
struct SaplingBackingStoreNoFaultInjectorTest : SaplingBackingStoreTestBase {
9596
FaultInjector faultInjector{/*enabled=*/false};
9697
folly::InlineExecutor executor = folly::InlineExecutor::instance();
98+
ErrorLogger noopErrorLogger{nullptr, {}, nullptr};
9799

98100
std::shared_ptr<SaplingBackingStore> queuedBackingStore =
99101
std::make_shared<SaplingBackingStore>(
@@ -105,6 +107,7 @@ struct SaplingBackingStoreNoFaultInjectorTest : SaplingBackingStoreTestBase {
105107
edenConfig,
106108
std::make_unique<SaplingBackingStoreOptions>(),
107109
makeTestEdenFsEventsLogger(edenConfig, stats),
110+
/*errorLogger=*/noopErrorLogger,
108111
std::make_unique<BackingStoreLogger>(),
109112
&faultInjector);
110113
};
@@ -116,6 +119,7 @@ struct SaplingBackingStoreWithFaultInjectorTest : SaplingBackingStoreTestBase {
116119
// Use a real executor so coroutine tests don't trip the coro::Task
117120
// DCHECK on InlineExecutor (Task.h:470).
118121
folly::CPUThreadPoolExecutor executor{1};
122+
ErrorLogger noopErrorLogger{nullptr, {}, nullptr};
119123

120124
std::shared_ptr<SaplingBackingStore> queuedBackingStore =
121125
std::make_shared<SaplingBackingStore>(
@@ -127,6 +131,7 @@ struct SaplingBackingStoreWithFaultInjectorTest : SaplingBackingStoreTestBase {
127131
edenConfig,
128132
std::make_unique<SaplingBackingStoreOptions>(),
129133
makeTestEdenFsEventsLogger(edenConfig, stats),
134+
/*errorLogger=*/noopErrorLogger,
130135
std::make_unique<BackingStoreLogger>(),
131136
&faultInjector);
132137
};
@@ -137,6 +142,7 @@ struct SaplingBackingStoreWithFaultInjectorIgnoreConfigTest
137142
std::make_shared<TestConfigSource>(ConfigSourceType::SystemConfig)};
138143
FaultInjector faultInjector{/*enabled=*/true};
139144
folly::InlineExecutor executor = folly::InlineExecutor::instance();
145+
ErrorLogger noopErrorLogger{nullptr, {}, nullptr};
140146

141147
std::shared_ptr<SaplingBackingStore> queuedBackingStore =
142148
std::make_shared<SaplingBackingStore>(
@@ -148,6 +154,7 @@ struct SaplingBackingStoreWithFaultInjectorIgnoreConfigTest
148154
edenConfig,
149155
std::make_unique<SaplingBackingStoreOptions>(),
150156
makeTestEdenFsEventsLogger(edenConfig, stats),
157+
/*errorLogger=*/noopErrorLogger,
151158
std::make_unique<BackingStoreLogger>(),
152159
&faultInjector);
153160
};

eden/fs/store/test/BUCK

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ cpp_unittest(
2727
"//eden/fs/store/filter:hg_sparse_filter",
2828
"//eden/fs/store/sl:sapling_backing_store",
2929
"//eden/fs/store/sl:sapling_backing_store_options",
30+
"//eden/fs/telemetry:error_logger",
3031
"//eden/fs/telemetry:stats",
3132
"//eden/fs/telemetry:structured_logger",
3233
"//eden/fs/testharness:fake_backing_store_and_tree_builder",

eden/fs/store/test/FilteredBackingStoreTest.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "eden/fs/store/sl/SaplingBackingStoreOptions.h"
3434
#include "eden/fs/telemetry/EdenFsEventsLogger.h"
3535
#include "eden/fs/telemetry/EdenStats.h"
36+
#include "eden/fs/telemetry/ErrorLogger.h"
3637
#include "eden/fs/testharness/FakeFilter.h"
3738
#include "eden/fs/testharness/HgRepo.h"
3839
#include "eden/fs/testharness/TestUtil.h"
@@ -154,6 +155,8 @@ struct SaplingFilteredBackingStoreTest : FilteredBackingStoreTestBase {
154155

155156
folly::InlineExecutor executor_ = folly::InlineExecutor::instance();
156157

158+
ErrorLogger noopErrorLogger{nullptr, {}, nullptr};
159+
157160
std::shared_ptr<SaplingBackingStore> wrappedStore_{
158161
std::make_shared<SaplingBackingStore>(
159162
repo.path(),
@@ -168,6 +171,7 @@ struct SaplingFilteredBackingStoreTest : FilteredBackingStoreTestBase {
168171
/*xplatLogger=*/nullptr,
169172
edenConfig,
170173
stats.copy()),
174+
/*errorLogger=*/noopErrorLogger,
171175
std::make_unique<BackingStoreLogger>(),
172176
&faultInjector)};
173177
};

0 commit comments

Comments
 (0)