Merge pull request #2020 from bugsnag/PLAT-12008/ndk-breadcrumb-count

NDK should obey Configuration.maxBreadcrumbs

.buildkite/pipeline.full.yml

@@ -11,6 +11,8 @@ steps:
     timeout_in_minutes: 5
     agents:
       queue: macos-14
+    env:
+      JAVA_VERSION: 17
     command: 'make example-app'
 
   - label: ':android: Build debug fixture APK'

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## TBD
+
+### Enhancements
+
+* `Configuration.maxBreadcrumbs` is now obeyed by `bugsnag-plugin-android-ndk`, so native events will include the correct number of breadcrumbs
+  []()
+
 ## 6.4.0 (2024-04-15)
 
 ### Enhancements

Makefile

@@ -64,7 +64,7 @@ fixture-debug: notifier
 example-app:
 	@./gradlew assembleRelease publishToMavenLocal -x check
 	# Build Android example app
-	@./examples/sdk-app-example/gradlew clean assembleRelease
+	@cd ./examples/sdk-app-example/ && ./gradlew clean assembleRelease
 
 bump:
 ifneq ($(shell git diff --staged),)

bugsnag-android-core/api/bugsnag-android-core.api

@@ -687,9 +687,10 @@ public final class com/bugsnag/android/StateEvent$Install : com/bugsnag/android/
 	public final field buildUuid Ljava/lang/String;
 	public final field consecutiveLaunchCrashes I
 	public final field lastRunInfoPath Ljava/lang/String;
+	public final field maxBreadcrumbs I
 	public final field releaseStage Ljava/lang/String;
 	public final field sendThreads Lcom/bugsnag/android/ThreadSendPolicy;
-	public fun <init> (Ljava/lang/String;ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILcom/bugsnag/android/ThreadSendPolicy;)V
+	public fun <init> (Ljava/lang/String;ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILcom/bugsnag/android/ThreadSendPolicy;I)V
 }
 
 public final class com/bugsnag/android/StateEvent$NotifyHandled : com/bugsnag/android/StateEvent {

bugsnag-android-core/detekt-baseline.xml

@@ -18,7 +18,7 @@
     <ID>LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, logger: Logger, breadcrumbs: MutableList&lt;Breadcrumb> = mutableListOf(), discardClasses: Set&lt;Pattern> = setOf(), errors: MutableList&lt;Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection&lt;String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList&lt;Thread> = mutableListOf(), user: User = User(), redactionKeys: Set&lt;Pattern>? = null )</ID>
     <ID>LongParameterList:EventStorageModule.kt$EventStorageModule$( contextModule: ContextModule, configModule: ConfigModule, dataCollectionModule: DataCollectionModule, bgTaskService: BackgroundTaskService, trackerModule: TrackerModule, systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState )</ID>
     <ID>LongParameterList:NativeStackframe.kt$NativeStackframe$( /** * The name of the method that was being executed */ var method: String?, /** * The location of the source file */ var file: String?, /** * The line number within the source file this stackframe refers to */ var lineNumber: Number?, /** * The address of the instruction where the event occurred. */ var frameAddress: Long?, /** * The address of the function where the event occurred. */ var symbolAddress: Long?, /** * The address of the library where the event occurred. */ var loadAddress: Long?, /** * Whether this frame identifies the program counter */ var isPC: Boolean?, /** * The type of the error */ var type: ErrorType? = null, /** * Identifies the exact build this frame originates from. */ var codeIdentifier: String? = null, )</ID>
-    <ID>LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int, @JvmField val sendThreads: ThreadSendPolicy )</ID>
+    <ID>LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int, @JvmField val sendThreads: ThreadSendPolicy, @JvmField val maxBreadcrumbs: Int )</ID>
     <ID>LongParameterList:ThreadState.kt$ThreadState$( allThreads: List&lt;JavaThread>, currentThread: JavaThread, exc: Throwable?, isUnhandled: Boolean, maxThreadCount: Int, threadCollectionTimeLimitMillis: Long, projectPackages: Collection&lt;String>, logger: Logger )</ID>
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$299</ID>
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$429</ID>

bugsnag-android-core/src/main/java/com/bugsnag/android/ClientObservable.kt

@@ -22,7 +22,8 @@ internal class ClientObservable : BaseObservable() {
                 conf.releaseStage,
                 lastRunInfoPath,
                 consecutiveLaunchCrashes,
-                conf.sendThreads
+                conf.sendThreads,
+                conf.maxBreadcrumbs
             )
         }
     }

bugsnag-android-core/src/main/java/com/bugsnag/android/StateEvent.kt

@@ -10,7 +10,8 @@ sealed class StateEvent { // JvmField allows direct field access optimizations
         @JvmField val releaseStage: String?,
         @JvmField val lastRunInfoPath: String,
         @JvmField val consecutiveLaunchCrashes: Int,
-        @JvmField val sendThreads: ThreadSendPolicy
+        @JvmField val sendThreads: ThreadSendPolicy,
+        @JvmField val maxBreadcrumbs: Int
     ) : StateEvent()
 
     object DeliverPending : StateEvent()

bugsnag-plugin-android-ndk/api/bugsnag-plugin-android-ndk.api

@@ -21,7 +21,7 @@ public final class com/bugsnag/android/ndk/NativeBridge : com/bugsnag/android/in
 	public final fun getCurrentNativeApiCallUsage ()Ljava/util/Map;
 	public final fun getSignalUnwindStackFunction ()J
 	public final fun initCallbackCounts (Ljava/util/Map;)V
-	public final fun install (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IZIZI)V
+	public final fun install (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IZIZII)V
 	public final fun notifyAddCallback (Ljava/lang/String;)V
 	public final fun notifyRemoveCallback (Ljava/lang/String;)V
 	public fun onStateChange (Lcom/bugsnag/android/StateEvent;)V

bugsnag-plugin-android-ndk/detekt-baseline.xml

@@ -4,7 +4,7 @@
   <CurrentIssues>
     <ID>CyclomaticComplexMethod:NativeBridge.kt$NativeBridge$override fun onStateChange(event: StateEvent)</ID>
     <ID>LongMethod:EventOnDiskTests.kt$EventOnDiskTests$@Test fun testEvent()</ID>
-    <ID>LongParameterList:NativeBridge.kt$NativeBridge$( apiKey: String, reportingDirectory: String, lastRunInfoPath: String, consecutiveLaunchCrashes: Int, autoDetectNdkCrashes: Boolean, apiLevel: Int, is32bit: Boolean, threadSendPolicy: Int )</ID>
+    <ID>LongParameterList:NativeBridge.kt$NativeBridge$( apiKey: String, reportingDirectory: String, lastRunInfoPath: String, consecutiveLaunchCrashes: Int, autoDetectNdkCrashes: Boolean, apiLevel: Int, is32bit: Boolean, threadSendPolicy: Int, maxBreadcrumbs: Int, )</ID>
     <ID>NestedBlockDepth:NativeBridge.kt$NativeBridge$private fun deliverPendingReports()</ID>
     <ID>TooManyFunctions:NativeBridge.kt$NativeBridge : StateObserver</ID>
     <ID>UseCheckOrError:ResourceUtils.kt$throw IllegalStateException("Failed to read JSON from $resourceName")</ID>

bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.kt

@@ -52,7 +52,8 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
         autoDetectNdkCrashes: Boolean,
         apiLevel: Int,
         is32bit: Boolean,
-        threadSendPolicy: Int
+        threadSendPolicy: Int,
+        maxBreadcrumbs: Int,
     )
 
     external fun startedSession(
@@ -226,7 +227,8 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
                     arg.autoDetectNdkCrashes,
                     Build.VERSION.SDK_INT,
                     is32bit,
-                    arg.sendThreads.ordinal
+                    arg.sendThreads.ordinal,
+                    arg.maxBreadcrumbs
                 )
                 installed.set(true)
             }

bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c

@@ -150,7 +150,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
     JNIEnv *env, jobject _this, jstring _api_key, jstring _event_path,
     jstring _last_run_info_path, jint consecutive_launch_crashes,
     jboolean auto_detect_ndk_crashes, jint _api_level, jboolean is32bit,
-    jint send_threads) {
+    jint send_threads, jint max_breadcrumbs) {
 
   if (!bsg_jni_cache_init(env)) {
     BUGSNAG_LOG("Could not init JNI jni_cache.");
@@ -165,6 +165,14 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
   bugsnag_env->send_threads = send_threads;
   bugsnag_env->handling_crash = ATOMIC_VAR_INIT(false);
 
+  bugsnag_env->next_event.max_crumb_count = max_breadcrumbs;
+  bugsnag_env->next_event.breadcrumbs =
+      calloc(max_breadcrumbs, sizeof(bugsnag_breadcrumb));
+
+  if (bugsnag_env->next_event.breadcrumbs == NULL) {
+    goto error;
+  }
+
   // copy event path to env struct
   const char *event_path = bsg_safe_get_string_utf_chars(env, _event_path);
   if (event_path == NULL) {

bugsnag-plugin-android-ndk/src/main/jni/event.c

@@ -353,13 +353,13 @@ void bugsnag_event_set_user(void *event_ptr, const char *id, const char *email,
 
 void bsg_event_add_breadcrumb(bugsnag_event *event, bugsnag_breadcrumb *crumb) {
   int crumb_index;
-  if (event->crumb_count < BUGSNAG_CRUMBS_MAX) {
+  if (event->crumb_count < event->max_crumb_count) {
     crumb_index = event->crumb_count;
     event->crumb_count++;
   } else {
     crumb_index = event->crumb_first_index;
     event->crumb_first_index =
-        (event->crumb_first_index + 1) % BUGSNAG_CRUMBS_MAX;
+        (event->crumb_first_index + 1) % event->max_crumb_count;
   }
 
   bsg_free_opaque_metadata(&(event->breadcrumbs[crumb_index].metadata));

bugsnag-plugin-android-ndk/src/main/jni/event.h

@@ -12,12 +12,6 @@
  */
 #define BUGSNAG_METADATA_MAX 128
 #endif
-#ifndef BUGSNAG_CRUMBS_MAX
-/**
- *  Max number of breadcrumbs in an event. Configures a default if not defined.
- */
-#define BUGSNAG_CRUMBS_MAX 50
-#endif
 #ifndef BUGSNAG_DEFAULT_EX_TYPE
 /**
  * Type assigned to exceptions. Configures a default if not defined.
@@ -34,7 +28,7 @@
 /**
  * Version of the bugsnag_event struct. Serialized to report header.
  */
-#define BUGSNAG_EVENT_VERSION 13
+#define BUGSNAG_EVENT_VERSION 14
 
 #ifdef __cplusplus
 extern "C" {
@@ -229,7 +223,9 @@ typedef struct {
   // Breadcrumbs are a ring; the first index moves as the
   // structure is filled and replaced.
   int crumb_first_index;
-  bugsnag_breadcrumb breadcrumbs[BUGSNAG_CRUMBS_MAX];
+  // the maximum number of breadcrumbs that can be placed in the buffer
+  int max_crumb_count;
+  bugsnag_breadcrumb *breadcrumbs;
 
   char context[64];
   bugsnag_severity severity;

bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_reader.c

@@ -2,14 +2,15 @@
 
 #include "../../event.h"
 #include "../string.h"
+#include "utils/logger.h"
 
 #include <fcntl.h>
 #include <malloc.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
-const int BSG_MIGRATOR_CURRENT_VERSION = 13;
+const int BSG_MIGRATOR_CURRENT_VERSION = 14;
 
 void bsg_read_feature_flags(int fd, bool expect_verification,
                             bsg_feature_flag **out_feature_flags,
@@ -21,6 +22,8 @@ void bsg_read_opaque_breadcrumb_metadata(int fd,
                                          bugsnag_breadcrumb *breadcrumbs,
                                          int crumb_count);
 
+bool bsg_read_breadcrumbs(int fd, bugsnag_event *event);
+
 bsg_report_header *bsg_report_header_read(int fd) {
   bsg_report_header *header = calloc(1, sizeof(bsg_report_header));
   ssize_t len = read(fd, header, sizeof(bsg_report_header));
@@ -55,10 +58,13 @@ bugsnag_event *bsg_read_event(char *filepath) {
 
   ssize_t len = read(fd, event, event_size);
   if (len != event_size) {
-    free(event);
-    return NULL;
+    goto error;
   }
 
+  // read the breadcrumbs
+  if (!bsg_read_breadcrumbs(fd, event)) {
+    goto error;
+  }
   // read the feature flags, if possible
   bsg_read_feature_flags(fd, true, &event->feature_flags,
                          &event->feature_flag_count);
@@ -67,6 +73,31 @@ bugsnag_event *bsg_read_event(char *filepath) {
                                       event->crumb_count);
 
   return event;
+error:
+  free(event);
+  return NULL;
+}
+
+bool bsg_read_breadcrumbs(int fd, bugsnag_event *event) {
+  bugsnag_breadcrumb *breadcrumbs =
+      calloc(event->max_crumb_count, sizeof(bugsnag_breadcrumb));
+  if (breadcrumbs == NULL) {
+    goto error;
+  }
+  const size_t bytes_to_read =
+      event->max_crumb_count * sizeof(bugsnag_breadcrumb);
+  const ssize_t read_count = read(fd, breadcrumbs, bytes_to_read);
+  if (read_count != bytes_to_read) {
+    goto error;
+  }
+
+  event->breadcrumbs = breadcrumbs;
+  return true;
+error:
+  event->breadcrumbs = NULL;
+  event->crumb_count = 0;
+  event->crumb_first_index = 0;
+  return false;
 }
 
 static char *read_string(int fd) {

bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c

@@ -8,11 +8,13 @@
 #include "buffered_writer.h"
 #include "utils/seqlock.h"
 
+bool bsg_write_breadcrumbs(bugsnag_event *event, bsg_buffered_writer *writer);
 bool bsg_write_feature_flags(bugsnag_event *event, bsg_buffered_writer *writer);
 
 bool bsg_write_opaque_metadata(bugsnag_event *event,
                                bsg_buffered_writer *writer);
 
+bool bsg_write_breadcrumbs(bugsnag_event *event, bsg_buffered_writer *writer);
 bool bsg_report_header_write(bsg_report_header *header, int fd) {
   ssize_t len = write(fd, header, sizeof(bsg_report_header));
 
@@ -30,7 +32,9 @@ bool bsg_event_write(bsg_environment *env) {
       bsg_report_header_write(&env->report_header, writer.fd) &&
       // add cached event info
       writer.write(&writer, &env->next_event, sizeof(bugsnag_event)) &&
-      // append feature flags after event structure
+      // append the breadcrumbs after the event structure
+      bsg_write_breadcrumbs(&env->next_event, &writer) &&
+      // append feature flags
       bsg_write_feature_flags(&env->next_event, &writer) &&
       // append opaque metadata after the feature flags
       bsg_write_opaque_metadata(&env->next_event, &writer);
@@ -85,6 +89,11 @@ static bool write_feature_flag(bsg_buffered_writer *writer,
   return true;
 }
 
+bool bsg_write_breadcrumbs(bugsnag_event *event, bsg_buffered_writer *writer) {
+  return writer->write(writer, event->breadcrumbs,
+                       sizeof(bugsnag_breadcrumb) * event->max_crumb_count);
+}
+
 extern bsg_seqlock_t bsg_feature_flag_lock;
 
 bool bsg_write_feature_flags(bugsnag_event *event,

bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/json_writer.c

@@ -365,7 +365,7 @@ void bsg_serialize_breadcrumbs(const bugsnag_event *event, JSON_Array *crumbs) {
                              bsg_crumb_type_string(breadcrumb.type));
       bsg_serialize_breadcrumb_metadata(breadcrumb.metadata, crumb);
       current_index++;
-      if (current_index == BUGSNAG_CRUMBS_MAX) {
+      if (current_index == event->max_crumb_count) {
         current_index = 0;
       }
     }

bugsnag-plugin-android-ndk/src/test/cpp/main.c

@@ -191,6 +191,8 @@ JNIEXPORT jstring JNICALL
 Java_com_bugsnag_android_ndk_BreadcrumbStateSerializationTest_run(JNIEnv *env,
                                                                   jobject thiz) {
   bugsnag_event *event = calloc(1, sizeof(bugsnag_event));
+  event->max_crumb_count = 50;
+  event->breadcrumbs = calloc(event->max_crumb_count, sizeof(bugsnag_breadcrumb));
   loadBreadcrumbsTestCase(event);
   JSON_Value *eventVal = json_value_init_array();
   JSON_Array *eventAry = json_value_get_array(eventVal);

bugsnag-plugin-android-ndk/src/test/cpp/migrations/EventOnDiskTests.cpp

@@ -41,7 +41,9 @@ static void *create_full_event() {
   event->app.version_code = 8139512718;
 
   // breadcrumbs
-  auto max = 50;
+  event->max_crumb_count = 50;
+  event->breadcrumbs = new bugsnag_breadcrumb[event->max_crumb_count];
+  auto max = event->max_crumb_count;
   event->crumb_first_index = 2; // test the circular buffer logic
   char name[30];
   for (int i = event->crumb_first_index; i < max; i++) {
@@ -132,7 +134,7 @@ static void *create_full_event() {
 static const char *write_event(JNIEnv *env, jstring temp_file,
                                   void *(event_generator)()) {
   auto event_ctx = (bsg_environment *)calloc(1, sizeof(bsg_environment));
-  event_ctx->report_header.version = 13;
+  event_ctx->report_header.version = BUGSNAG_EVENT_VERSION;
   const char *path = (*env).GetStringUTFChars(temp_file, nullptr);
   sprintf(event_ctx->next_event_path, "%s", path);
 
@@ -185,6 +187,7 @@ Java_com_bugsnag_android_ndk_migrations_EventOnDiskTests_generateAndStoreEvent(
     free(parsed_event->feature_flags[i].name);
     free(parsed_event->feature_flags[i].variant);
   }
+  free(parsed_event->breadcrumbs);
   free(parsed_event->feature_flags);
   free(parsed_event);
 

bugsnag-plugin-android-ndk/src/test/cpp/test_breadcrumbs.c

@@ -20,6 +20,9 @@ bugsnag_breadcrumb *init_breadcrumb(const char *name, const char *message, bugsn
 
 TEST test_add_breadcrumb(void) {
   bugsnag_event *event = calloc(1, sizeof(bugsnag_event));
+  event->max_crumb_count = 50;
+  event->breadcrumbs =
+      calloc(event->max_crumb_count, sizeof(bugsnag_breadcrumb));
   bugsnag_breadcrumb *crumb = init_breadcrumb("stroll", "this is a drill.", BSG_CRUMB_USER);
   bsg_event_add_breadcrumb(event, crumb);
   ASSERT_EQ(1, event->crumb_count);
@@ -39,13 +42,17 @@ TEST test_add_breadcrumb(void) {
   ASSERT(strcmp("message", event->breadcrumbs[1].metadata.values[0].name) == 0);
   ASSERT(strcmp("this is not a drill.", event->breadcrumbs[1].metadata.values[0].char_value) == 0);
 
+  free(event->breadcrumbs);
   free(event);
   free(crumb2);
   PASS();
 }
 
 TEST test_add_breadcrumbs_over_max(void) {
   bugsnag_event *event = calloc(1, sizeof(bugsnag_event));
+  event->max_crumb_count = 50;
+  event->breadcrumbs =
+      calloc(event->max_crumb_count, sizeof(bugsnag_breadcrumb));
   int breadcrumb_count = 64;
 
   for (int i=0; i < breadcrumb_count; i++) {
@@ -59,7 +66,7 @@ TEST test_add_breadcrumbs_over_max(void) {
   }
 
   // assertions assume that the crumb count is always 50
-  ASSERT_EQ(BUGSNAG_CRUMBS_MAX, event->crumb_count);
+  ASSERT_EQ(event->max_crumb_count, event->crumb_count);
   ASSERT_EQ(14, event->crumb_first_index);
 
   ASSERT_STR_EQ("crumb: 50", event->breadcrumbs[0].name);
@@ -71,6 +78,7 @@ TEST test_add_breadcrumbs_over_max(void) {
   ASSERT_STR_EQ("crumb: 14", event->breadcrumbs[14].name);
   ASSERT_STR_EQ("crumb: 15", event->breadcrumbs[15].name);
   ASSERT_STR_EQ("crumb: 16", event->breadcrumbs[16].name);
+  free(event->breadcrumbs);
   free(event);
   PASS();
 }