In use objects - add a profile type (#316)

- Adjust the watcher configuration to define profile types for in use objects
- Remove the unused metric type
- Allow allocation samples to co-exist with in use objects

docs/Build.md

@@ -35,6 +35,21 @@ RelCMake ../
 make -j 4 .
 ```
 
+### Running static analysis
+
+cppcheck is run with a dedicated build target from within a build folder.
+```bash
+make cppcheck
+```
+
+Clang tidy checks require llvm 17 to be installed.
+Using the build image will guarantee this.
+```bash
+CXX=clang++-17 CC=clang-17 source ./setup_env.sh
+MkBuildDir ClangDeb
+    DebCMake -DENABLE_CLANG_TIDY=ON ../
+```
+
 ### Updating libdatadog
 
 Head over to the libdatadog, do your changes and copy the library back to your vendor directory.

include/event_config.hpp

@@ -12,24 +12,24 @@
 #include <string>
 #include <vector>
 
+enum EventAggregationModePos {
+  kSumPos = 0,
+  kLiveSumPos = 1,
+  kNbEventAggregationModes
+};
+
 // Defines how a sample is aggregated when it is received
-enum class EventConfMode : uint32_t {
+enum class EventAggregationMode : uint32_t {
   kDisabled = 0,
-  kCallgraph = 1 << 0,     // flamegraph of resource usage
-  kMetric = 1 << 1,        // gauge of resource usage
-  kLiveCallgraph = 1 << 2, // report callgraph of resources still in use
-  kAll = kCallgraph | kMetric | kLiveCallgraph,
+  kSum = 1 << kSumPos,         // Sum of usage (example: overall CPU usage)
+  kLiveSum = 1 << kLiveSumPos, // Report live usage (example memory leaks)
+  kAll = kSum | kLiveSum,
 };
 
-ALLOW_FLAGS_FOR_ENUM(EventConfMode)
-
-constexpr bool Any(EventConfMode arg) {
-  return arg != EventConfMode::kDisabled;
-}
+ALLOW_FLAGS_FOR_ENUM(EventAggregationMode)
 
-constexpr bool AnyCallgraph(EventConfMode arg) {
-  return Any((arg & EventConfMode::kLiveCallgraph) |
-             (arg & EventConfMode::kCallgraph));
+constexpr bool Any(EventAggregationMode arg) {
+  return arg != EventAggregationMode::kDisabled;
 }
 
 // Defines how samples are weighted
@@ -135,7 +135,7 @@ enum class EventConfField {
 };
 
 struct EventConf {
-  EventConfMode mode{};
+  EventAggregationMode mode{};
 
   int64_t id{};
 

include/perf_watcher.hpp

@@ -30,6 +30,11 @@ struct PerfWatcherOptions {
       k_default_perf_stack_sample_size}; // size of the user stack to capture
 };
 
+struct PProfIndices {
+  int pprof_index = -1;
+  int pprof_count_index = -1;
+};
+
 struct PerfWatcher {
   int ddprof_event_type; // ddprof event type from DDPROF_EVENT_NAMES enum
   std::string desc;
@@ -57,25 +62,27 @@ struct PerfWatcher {
   // Other configs
   bool suppress_pid;
   bool suppress_tid;
-  int pprof_sample_idx;       // index into the SampleType in the pprof
-  int pprof_count_sample_idx; // index into the pprof for the count
-  bool instrument_self;       // do my own perfopen, etc
-  EventConfMode output_mode;  // defines how sample data is aggregated
+  PProfIndices pprof_indices[kNbEventAggregationModes]; // std and live
+  bool instrument_self; // do my own perfopen, etc
+  EventAggregationMode aggregation_mode;
 };
 
 // The Datadog backend only understands pre-configured event types.  Those
 // types are defined here, and then referenced in the watcher
 // The last column is a dependent type which is always aggregated as a count
 // whenever the main type is aggregated.
+//  type,    pprof,     unit, live-pprof,  sample_type,
+//     a,        b,        c,          d,            e,
 #define PROFILE_TYPE_TABLE(X)                                                  \
-  X(NOCOUNT, "nocount", nocount, NOCOUNT)                                      \
-  X(TRACEPOINT, "tracepoint", events, NOCOUNT)                                 \
-  X(CPU_NANOS, "cpu-time", nanoseconds, CPU_SAMPLE)                            \
-  X(CPU_SAMPLE, "cpu-samples", count, NOCOUNT)                                 \
-  X(ALLOC_SAMPLE, "alloc-samples", count, NOCOUNT)                             \
-  X(ALLOC_SPACE, "alloc-space", bytes, ALLOC_SAMPLE)
-
-#define X_ENUM(a, b, c, d) DDPROF_PWT_##a,
+  X(NOCOUNT, "nocount", nocount, "undef", NOCOUNT)                             \
+  X(TRACEPOINT, "tracepoint", events, "undef", NOCOUNT)                        \
+  X(CPU_NANOS, "cpu-time", nanoseconds, "undef", CPU_SAMPLE)                   \
+  X(CPU_SAMPLE, "cpu-samples", count, "undef", NOCOUNT)                        \
+  X(ALLOC_SAMPLE, "alloc-samples", count, "inuse-objects", NOCOUNT)            \
+  X(ALLOC_SPACE, "alloc-space", bytes, "inuse-space", ALLOC_SAMPLE)
+
+// defines enum of profile types
+#define X_ENUM(a, b, c, d, e) DDPROF_PWT_##a,
 typedef enum DDPROF_SAMPLE_TYPES {
   PROFILE_TYPE_TABLE(X_ENUM) DDPROF_PWT_LENGTH,
 } DDPROF_SAMPLE_TYPES;
@@ -158,14 +165,13 @@ int watcher_to_count_sample_type_id(const PerfWatcher *watcher);
 const char *event_type_name_from_idx(int idx);
 
 // Helper functions for sample types
-const char *sample_type_name_from_idx(int idx);
+const char *sample_type_name_from_idx(int idx, EventAggregationModePos pos);
 const char *sample_type_unit_from_idx(int idx);
 int sample_type_id_to_count_sample_type_id(int idx);
 
 // Helper functions, mostly for tests
 uint64_t perf_event_default_sample_type();
 void log_watcher(const PerfWatcher *w, int idx);
-
 std::string_view watcher_help_text();
 
 } // namespace ddprof

include/pprof/ddprof_pprof.hpp

@@ -36,7 +36,7 @@ DDRes pprof_create_profile(DDProfPProf *pprof, DDProfContext &ctx);
 DDRes pprof_aggregate(const UnwindOutput *uw_output,
                       const SymbolHdr &symbol_hdr, uint64_t value,
                       uint64_t count, const PerfWatcher *watcher,
-                      DDProfPProf *pprof);
+                      EventAggregationModePos value_pos, DDProfPProf *pprof);
 
 DDRes pprof_reset(DDProfPProf *pprof);
 
@@ -46,6 +46,7 @@ DDRes pprof_free_profile(DDProfPProf *pprof);
 
 void ddprof_print_sample(const UnwindOutput &uw_output,
                          const SymbolHdr &symbol_hdr, uint64_t value,
+                         EventAggregationModePos value_mode_pos,
                          const PerfWatcher &watcher);
 
 } // namespace ddprof

src/ddprof_cmdline.cc

@@ -90,15 +90,7 @@ bool watcher_from_config(EventConf *conf, PerfWatcher *watcher) {
   if (conf->value_scale != 0.0) {
     watcher->value_scale = conf->value_scale;
   }
-
-  // The output mode isn't set as part of the configuration templates; we
-  // always default to callgraph mode
-  if (conf->mode != EventConfMode::kDisabled) {
-    watcher->output_mode = conf->mode;
-  } else {
-    watcher->output_mode = EventConfMode::kCallgraph;
-  }
-
+  watcher->aggregation_mode = conf->mode;
   watcher->tracepoint_event = conf->eventname;
   watcher->tracepoint_group = conf->groupname;
   watcher->tracepoint_label = conf->label;
@@ -142,7 +134,10 @@ bool arg_yesno(const char *str, int mode) {
 bool watchers_from_str(const char *str, std::vector<PerfWatcher> &watchers,
                        uint32_t stack_sample_size) {
   std::vector<EventConf> configs;
-  const EventConf template_conf{.stack_sample_size = stack_sample_size};
+  const EventConf template_conf{
+      .mode = EventAggregationMode::kSum,
+      .stack_sample_size = stack_sample_size,
+  };
   if (EventConf_parse(str, template_conf, configs) != 0) {
     return false;
   }

src/ddprof_worker.cc

@@ -71,7 +71,7 @@ DDRes report_lost_events(DDProfContext &ctx) {
       LG_WRN("Reporting #%lu -> [%lu] lost samples for watcher #%d", nb_lost,
              value, watcher_idx);
       DDRES_CHECK_FWD(pprof_aggregate(
-          &us->output, us->symbol_hdr, value, nb_lost, watcher,
+          &us->output, us->symbol_hdr, value, nb_lost, watcher, kSumPos,
           ctx.worker_ctx.pprof[ctx.worker_ctx.i_current_pprof]));
       ctx.worker_ctx.lost_events_per_watcher[watcher_idx] = 0;
     }
@@ -179,10 +179,7 @@ DDRes ddprof_unwind_sample(DDProfContext &ctx, perf_event_sample *sample,
   }
 
   // Attempt to fully unwind if the watcher has a callgraph type
-  DDRes res = {};
-  if (AnyCallgraph(watcher->output_mode)) {
-    res = unwindstate__unwind(us);
-  }
+  DDRes res = unwindstate__unwind(us);
 
   /* This test is not 100% accurate:
    * Linux kernel does not take into account stack start (ie. end address since
@@ -227,12 +224,12 @@ DDRes aggregate_livealloc_stack(
     const LiveAllocation::PprofStacks::value_type &alloc_info,
     DDProfContext &ctx, const PerfWatcher *watcher, DDProfPProf *pprof,
     const SymbolHdr &symbol_hdr) {
-  DDRES_CHECK_FWD(pprof_aggregate(&alloc_info.first, symbol_hdr,
-                                  alloc_info.second._value,
-                                  alloc_info.second._count, watcher, pprof));
+  DDRES_CHECK_FWD(
+      pprof_aggregate(&alloc_info.first, symbol_hdr, alloc_info.second._value,
+                      alloc_info.second._count, watcher, kLiveSumPos, pprof));
   if (ctx.params.show_samples) {
     ddprof_print_sample(alloc_info.first, symbol_hdr, alloc_info.second._value,
-                        *watcher);
+                        kLiveSumPos, *watcher);
   }
   return ddres_init();
 }
@@ -388,22 +385,21 @@ DDRes ddprof_pr_sample(DDProfContext &ctx, perf_event_sample *sample,
   // Aggregate if unwinding went well (todo : fatal error propagation)
   if (!IsDDResFatal(res)) {
     struct UnwindState *us = ctx.worker_ctx.us;
-    if (Any(EventConfMode::kLiveCallgraph & watcher->output_mode)) {
-      // Live callgraph mode
-      // for now we hard code the live aggregation mode
+    if (Any(EventAggregationMode::kLiveSum & watcher->aggregation_mode)) {
       ctx.worker_ctx.live_allocation.register_allocation(
           us->output, sample->addr, sample->period, watcher_pos, sample->pid);
-    } else if (Any(EventConfMode::kCallgraph & watcher->output_mode)) {
+    }
+    if (Any(EventAggregationMode::kSum & watcher->aggregation_mode)) {
       // Depending on the type of watcher, compute a value for sample
       uint64_t const sample_val = perf_value_from_sample(watcher, sample);
 
       // in lib mode we don't aggregate (protect to avoid link failures)
       int const i_export = ctx.worker_ctx.i_current_pprof;
       DDProfPProf *pprof = ctx.worker_ctx.pprof[i_export];
       DDRES_CHECK_FWD(pprof_aggregate(&us->output, us->symbol_hdr, sample_val,
-                                      1, watcher, pprof));
+                                      1, watcher, kSumPos, pprof));
       if (ctx.params.show_samples) {
-        ddprof_print_sample(us->output, us->symbol_hdr, sample->period,
+        ddprof_print_sample(us->output, us->symbol_hdr, sample->period, kSumPos,
                             *watcher);
       }
     }

src/event_parser/event_parser.y

@@ -41,26 +41,21 @@ EventConf g_accum_event_conf = {};
 EventConf g_template_event_conf = {};
 std::vector<EventConf>* g_event_configs;
 
-std::optional<EventConfMode> mode_from_str(const std::string &str) {
-  EventConfMode mode = EventConfMode::kDisabled;
-  if (str.empty())
-    return mode;
+std::optional<EventAggregationMode> mode_from_str(const std::string &str) {
   const std::string a_str{"Aa*"};
-  const std::string l_str{"Ll"};
-  const std::string g_str{"Gg"};
-  const std::string m_str{"Mm"};
-
+  const std::string l_str{"Ll"}; // live sum
+  const std::string s_str{"Ss"}; // sum
+  EventAggregationMode mode = EventAggregationMode::kDisabled;
   for (const char &c : str) {
-    if (m_str.find(c) != std::string::npos) {
-      mode |= EventConfMode::kMetric;
-    } else if (g_str.find(c) != std::string::npos) {
-      mode |= EventConfMode::kCallgraph;
+    if (s_str.find(c) != std::string::npos) {
+      mode |= EventAggregationMode::kSum;
     } else if (l_str.find(c) != std::string::npos) {
-        mode |= EventConfMode::kLiveCallgraph;
+        mode |= EventAggregationMode::kLiveSum;
     } else if (a_str.find(c) != std::string::npos) {
-      mode |= EventConfMode::kAll;
+      mode |= EventAggregationMode::kAll;
     } else {
-      return {};
+      fprintf(stderr, "Warning, unexpected mode %c \n", c);
+      return {}; // unexpected mode
     }
   }
   return mode;
@@ -104,7 +99,7 @@ void conf_print(const EventConf *tp) {
   else
     printf("  label: <generated from event/groupname>\n");
 
-  const char *modenames[] = {"ILLEGAL", "callgraph", "metric", "live callgraph", "metric and callgraph"};
+  const char *modenames[] = {"ILLEGAL", "sum usage", "live usage"};
   printf("  type: %s\n", modenames[static_cast<unsigned>(tp->mode)]);
 
 
@@ -228,7 +223,8 @@ opt:
                delete $3;
                VAL_ERROR();
              }
-             g_accum_event_conf.mode |= *mode;
+             // override mode if present
+             g_accum_event_conf.mode = *mode;
              break;
            }
          default:
@@ -270,7 +266,7 @@ opt:
            g_accum_event_conf.value_scale = 0.0 + $3;
            break;
          case EventConfField::kMode:
-           g_accum_event_conf.mode = static_cast<EventConfMode>($3) & EventConfMode::kAll;
+           g_accum_event_conf.mode = static_cast<EventAggregationMode>($3) & EventAggregationMode::kAll;
            break;
 
          case EventConfField::kParameter:

src/exe/main.cc

@@ -341,8 +341,8 @@ int start_profiler_internal(std::unique_ptr<DDProfContext> ctx,
         reply.loaded_libs_check_interval_ms =
             ctx->params.loaded_libs_check_interval.count();
 
-        if (ctx->watchers[alloc_watcher_idx].output_mode ==
-            EventConfMode::kLiveCallgraph) {
+        if (ctx->watchers[alloc_watcher_idx].aggregation_mode ==
+            EventAggregationMode::kLiveSum) {
           reply.allocation_flags |= (1 << ReplyMessage::kLiveCallgraph);
         }
       }