Internal commit.

PiperOrigin-RevId: 238673108
Change-Id: I91c981ad6f230b92744a9d8081e42edb5bcef689

Author: Cloud Debugger Team

src/agent/Makefile

@@ -33,9 +33,10 @@ CFLAGS += \
 THIRD_PARTY_LIB_PATH ?= /usr/local/lib
 THIRD_PARTY_INCLUDE_PATH ?= /usr/local/include
 SYSTEM_LIB_PATH ?= /usr/lib/x86_64-linux-gnu
-ASM_JAR_PATH ?= /usr/lib/asm-7.0/lib/asm-7.0.jar
-ASM_COMMONS_JAR_PATH ?= /usr/lib/asm-7.0/lib/asm-commons-7.0.jar
-ASM_UTIL_JAR_PATH ?= /usr/lib/asm-7.0/lib/asm-util-7.0.jar
+ASM_JAR_PATH ?= /usr/lib/asm-7.1/lib/asm-7.1.jar
+ASM_COMMONS_JAR_PATH ?= /usr/lib/asm-7.1/lib/asm-commons-7.1.jar
+ASM_DEPRECATED_JAR_PATH ?= /usr/lib/asm-7.1/lib/asm-deprecated-7.1.jar
+ASM_UTIL_JAR_PATH ?= /usr/lib/asm-7.1/lib/asm-util-7.1.jar
 GOOGLE_API_JAVA_CLIENT_PATH ?= /usr/lib/google-api-java-client
 JARJAR_PATH ?= ../../build/jarjar.jar
 JAVA_BUILD ?= maven
@@ -245,9 +246,10 @@ ifeq ($(JAVA_BUILD),maven)
 	cp internals/target/cdbg_java_agent_internals-SNAPSHOT-jar-with-dependencies.jar $(INTERNALS_JAR_FULL)
 else
 	mkdir -p internals/target
-	cd internals ; $(JAVAC) -g:none -cp $(ASM_JAR_PATH):$(ASM_COMMONS_JAR_PATH):$(ASM_UTIL_JAR_PATH):$(GSON_JAR_PATH):$(GOOGLE_API_CLIENT_JARS_CLASS_PATH):$(SNAKE_YAML_JAR_PATH) -d target src/main/java/com/google/devtools/cdbg/debuglets/java/*.java
+	cd internals ; $(JAVAC) -g:none -cp $(ASM_JAR_PATH):$(ASM_COMMONS_JAR_PATH):$(ASM_DEPRECATED_JAR_PATH):$(ASM_UTIL_JAR_PATH):$(GSON_JAR_PATH):$(GOOGLE_API_CLIENT_JARS_CLASS_PATH):$(SNAKE_YAML_JAR_PATH) -d target src/main/java/com/google/devtools/cdbg/debuglets/java/*.java
 	cd internals/target ; jar xf $(ASM_JAR_PATH) ; rm -rf META-INF
 	cd internals/target ; jar xf $(ASM_COMMONS_JAR_PATH) ; rm -rf META-INF
+	cd internals/target ; jar xf $(ASM_DEPRECATED_JAR_PATH) ; rm -rf META-INF
 	cd internals/target ; jar xf $(ASM_UTIL_JAR_PATH) ; rm -rf META-INF
 	cd internals/target ; jar xf $(GSON_JAR_PATH) ; rm -rf META-INF
 	cd internals/target ; jar xf $(SNAKE_YAML_JAR_PATH) ; rm -rf META-INF

src/agent/callbacks_monitor.h

@@ -40,13 +40,12 @@ class CallbacksMonitor {
 
   typedef std::list<OngoingCall>::iterator Id;
 
-  CallbacksMonitor(
+  explicit CallbacksMonitor(
       int max_call_duration_ms,
       std::function<int64()> fn_gettime = MonotonicClockMillis)
       : max_call_duration_ms_(max_call_duration_ms),
         fn_gettime_(fn_gettime),
-        last_unhealthy_time_ms_(-1) {
-  }
+        last_unhealthy_time_ms_(-1) {}
 
   ~CallbacksMonitor() {
     if (!ongoing_calls_.empty()) {

src/agent/canary_control.cc

@@ -19,13 +19,6 @@
 #include "messages.h"
 #include "model_util.h"
 
-namespace devtools {
-namespace cdbg {
-
-// Number of attempts to register or approve a canary breakpoint before
-// failing the operation.
-static constexpr int kMaxAttempts = 3;
-
 // The "ApproveHealtyBreakpoints" method is called from worker thread every
 // cycle of "ListActiveBreakpoints", which is once every 40 seconds. The
 // constant of 35 seconds is deliberately a bit shorter than that so that the
@@ -35,6 +28,13 @@ ABSL_FLAG(
     "Time interval after which an enabled canary breakpoint is considered as "
     "safe for a global rollout (from this debuglet's perspective)");
 
+namespace devtools {
+namespace cdbg {
+
+// Number of attempts to register or approve a canary breakpoint before
+// failing the operation.
+static constexpr int kMaxAttempts = 3;
+
 CanaryControl::CanaryControl(
     CallbacksMonitor* callbacks_monitor,
     Bridge* bridge)
@@ -150,4 +150,3 @@ void CanaryControl::ApproveHealtyBreakpoints() {
 
 }  // namespace cdbg
 }  // namespace devtools
-

src/agent/capture_data_collector.cc

@@ -35,6 +35,21 @@ ABSL_FLAG(
 namespace devtools {
 namespace cdbg {
 
+// Local utility functions
+namespace {
+
+void MergeLabels(std::map<std::string, std::string>* existing_labels,
+                 std::map<std::string, std::string> agent_labels) {
+  // This will merge in the agent labels with the pre existing client labels
+  // that may already be present. In the event of a duplicate label the insert
+  // will favour the pre existing labels an not update the entry.  This
+  // generally should not be an issue as the pre existing client label names are
+  // chosen with care and there should be no conflicts.
+  existing_labels->insert(agent_labels.begin(), agent_labels.end());
+}
+
+}  // namespace
+
 CaptureDataCollector::CaptureDataCollector(JvmEvaluators* evaluators)
     : evaluators_(evaluators) {
   // Reserve "var_table_index" 0 for memory objects that we didn't capture
@@ -323,8 +338,9 @@ void CaptureDataCollector::Format(BreakpointModel* breakpoint) const {
     breakpoint->variable_table.push_back(std::move(object_variable));
   }
 
-  // Format the breakpoint labels.
-  breakpoint->labels = breakpoint_labels_provider_->Format();
+  // Format the breakpoint labels and merge them with the existing client
+  // labels.
+  MergeLabels(&breakpoint->labels, breakpoint_labels_provider_->Format());
 
   // Format the end user identity.
   if (absl::GetFlag(FLAGS_cdbg_capture_user_id)) {

src/agent/glob_data_visibility_policy.cc

@@ -15,8 +15,7 @@ namespace {
 // reports that methods and fields have their data hidden.
 class BlacklistedClassImpl : public DataVisibilityPolicy::Class {
  public:
-  BlacklistedClassImpl(const string& reason) :
-    reason_(reason) { }
+  explicit BlacklistedClassImpl(const string& reason) : reason_(reason) {}
 
   bool IsFieldVisible(const string& name, int32 field_modifiers) override {
     return true;

src/agent/internals/src/main/java/com/google/devtools/cdbg/debuglets/java/SafeCallerException.java

@@ -80,7 +80,6 @@ public static Throwable createNativeMethodNotAllowed(String method) {
 
   /** Determines the binary name of the caller omitting internally generated safe caller methods. */
   private static String getCallerName() {
-
     return "<unknown>";
   }
 }

src/agent/jvm_object_evaluator.h

@@ -53,8 +53,7 @@ class JvmObjectEvaluator : public ObjectEvaluator {
     bool pretty_print_stringable = true;
   };
 
-  JvmObjectEvaluator(
-      ClassMetadataReader* class_metadata_reader);
+  explicit JvmObjectEvaluator(ClassMetadataReader* class_metadata_reader);
 
   ~JvmObjectEvaluator() override;
 

src/agent/jvmti_globals.cc

@@ -24,6 +24,7 @@
 #include "jvm_internals.h"
 #include "jvmti_buffer.h"
 #include "statistician.h"
+
 #include "jvmti_agent.h"
 #include "glob_data_visibility_policy.h"
 #include "yaml_data_visibility_config_reader.h"
@@ -70,6 +71,7 @@ ABSL_FLAG(string, service_account_email, "",
 ABSL_FLAG(string, service_account_json_file, "",
           "Path to JSON file containing private key of the service account");
 #endif
+
 static devtools::cdbg::JvmtiAgent* g_instance = nullptr;
 
 static devtools::cdbg::JvmInternals* g_internals = nullptr;

src/agent/messages.h

@@ -215,8 +215,7 @@ constexpr char DynamicLogOutOfCallQuota[] =
 
 constexpr char CanaryBreakpointUnhealthy[] =
     "The snapshot canary has failed and the snapshot cancelled. Please try "
-    "again at a later time."
-    ;
+    "again at a later time.";
 
 #define INTERNAL_ERROR_MESSAGE (devtools::cdbg::FormatMessageModel { \
       "Internal error at $0:$1", \

src/agent/mutex.h

@@ -19,6 +19,7 @@
 
 #include <mutex>  // NOLINT
 #include "common.h"
+
 namespace absl {
 
 // Wrapper class for non-recursive mutex.