review

Change-Id: I3dfebf963b2c78f3a6b0c5d3b97ba58d941e653d Signed-off-by: Kuat Yessenov <[email protected]>
kyessenov · Sep 19, 2024 · 041cd4f · 041cd4f
1 parent f6ac715
commit 041cd4f
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 16 deletions.
diff --git a/source/common/profiler/profiler.cc b/source/common/profiler/profiler.cc
@@ -68,30 +68,32 @@ bool Heap::stopProfiler() { return false; }
 namespace Envoy {
 namespace Profiler {
 
-static absl::optional<tcmalloc::MallocExtension::AllocationProfilingToken> alloc_profiler =
-    absl::nullopt;
+static tcmalloc::MallocExtension::AllocationProfilingToken* alloc_profiler = nullptr;
 
 absl::StatusOr<std::string> TcmallocProfiler::tcmallocHeapProfile() {
   auto profile = tcmalloc::MallocExtension::SnapshotCurrent(tcmalloc::ProfileType::kHeap);
   return tcmalloc::Marshal(profile);
 }
 
-bool TcmallocProfiler::startAllocationProfile() {
-  if (alloc_profiler) {
-    return false;
+absl::Status TcmallocProfiler::startAllocationProfile() {
+  if (alloc_profiler != nullptr) {
+    return absl::Status(absl::StatusCode::kFailedPrecondition,
+                        "Allocation profiler has already started");
   }
-  alloc_profiler = tcmalloc::MallocExtension::StartAllocationProfiling();
-  return true;
+  alloc_profiler = new tcmalloc::MallocExtension::AllocationProfilingToken(
+      tcmalloc::MallocExtension::StartAllocationProfiling());
+  return absl::OkStatus();
 }
 
 absl::StatusOr<std::string> TcmallocProfiler::stopAllocationProfile() {
   if (!alloc_profiler) {
     return absl::Status(absl::StatusCode::kFailedPrecondition,
                         "Allocation profiler is not started");
   }
-  const auto profile = std::move(alloc_profiler.value()).Stop();
+  const auto profile = std::move(*alloc_profiler).Stop();
   const auto result = tcmalloc::Marshal(profile);
-  alloc_profiler = absl::nullopt;
+  delete alloc_profiler;
+  alloc_profiler = nullptr;
   return result;
 }
 
@@ -108,12 +110,14 @@ absl::StatusOr<std::string> TcmallocProfiler::tcmallocHeapProfile() {
                       "Heap profile is not implemented in current build");
 }
 
-bool TcmallocProfiler::startAllocationProfile() { return false; }
-
-absl::StatusOr<std::string> TcmallocProfiler::stopAllocationProfile() {
+absl::Status TcmallocProfiler::startAllocationProfile() {
   return absl::Status(absl::StatusCode::kUnimplemented,
                       "Allocation profile is not implemented in current build");
-}
+
+  absl::StatusOr<std::string> TcmallocProfiler::stopAllocationProfile() {
+    return absl::Status(absl::StatusCode::kUnimplemented,
+                        "Allocation profile is not implemented in current build");
+  }
 
 } // namespace Profiler
 } // namespace Envoy

diff --git a/source/common/profiler/profiler.h b/source/common/profiler/profiler.h
@@ -73,7 +73,7 @@ class TcmallocProfiler {
   TcmallocProfiler() = default;
 
   static absl::StatusOr<std::string> tcmallocHeapProfile();
-  static bool startAllocationProfile();
+  static absl::Status startAllocationProfile();
   static absl::StatusOr<std::string> stopAllocationProfile();
 };
 

diff --git a/source/server/admin/profiling_handler.cc b/source/server/admin/profiling_handler.cc
@@ -105,8 +105,9 @@ Http::Code TcmallocProfilingHandler::handlerAllocationProfiler(Http::ResponseHea
   }
   const bool enable = enableVal.value() == "y";
   if (enable) {
-    const bool started = Profiler::TcmallocProfiler::startAllocationProfile();
-    if (!started) {
+    const auto started = Profiler::TcmallocProfiler::startAllocationProfile();
+    if (!started.ok()) {
+      response.add(started.message());
       return Http::Code::BadRequest;
     }
     response.add("OK\n");