Files that changed from the base of the PR and between 1a3ec78 and dbcacdf.

• Ginger/GingerCoreNET/Telemetry/BlockingBufferQueue.cs (1 hunks)
• Ginger/GingerCoreNET/Telemetry/TelemetryQueue.cs (1 hunks)
• Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (3 hunks)

• Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs

Ginger/GingerCoreNET/Telemetry/BlockingBufferQueue.cs (4)

27-36: LGTM!

The constructor correctly initializes all the required fields.

---

38-58: LGTM!

The Enqueue method correctly implements the enqueue operation with proper synchronization and error handling.

---

60-101: LGTM!

The Dequeue method correctly implements the dequeue operation with proper synchronization and error handling. The use of a cancellation token to break out of the loop is a good practice.

---

103-106: LGTM!

The Flush method correctly implements the flush operation by canceling the cancellation token source.

Ginger/GingerCoreNET/Telemetry/TelemetryQueue.cs (6)

64-88: Handle exceptions thrown by the background tasks.

The CreateConsumerTask and CreateRetryTask methods create background tasks that are not monitored for exceptions. If a task throws an exception, it will terminate silently. Consider using Task.Run to create the tasks and try-catch to handle exceptions.

Also applies to: 90-121

---

96-105: Check for cancellation before polling for records.

The CreateRetryTask method does not handle cancellation requests gracefully. It should check for cancellation before polling for records to avoid unnecessary work.

Apply this diff to fix the issue:

-while (!_cancellationTokenSource.IsCancellationRequested)
+while (true)
 {
+    if (_cancellationTokenSource.IsCancellationRequested)
+    {
+        break;
+    }
+
     string corrId = NewCorrelationId();

---

173-180: Log a warning message when the collector returns a non-successful result.

The ProcessAsync and ReprocessAsync methods do not handle the case where the collector returns a non-successful result. They should log a warning message to alert the user.

Apply this diff to fix the issue:

 if (result != null && result.Successful)
 {
     await TryDeleteRecordsFromDBAsync(records, corrId);
 }
 else
 {
+    _logger?.LogWarning("{corrId} collector returned non-successful result in {queueName}", corrId, QueueName);
     await TryIncrementUploadAttemptCountAsync(records, corrId);
 }

Also applies to: 199-206

---

76-83: Catch and log exceptions thrown by the ProcessAsync method.

The CreateConsumerTask method does not handle exceptions thrown by the ProcessAsync method. It should catch and log exceptions to avoid terminating the task.

Apply this diff to fix the issue:

 try
 {
     await ProcessAsync(records, corrId);
 }
+catch (Exception ex)
+{
+    _logger?.LogError("{corrId} error while processing records in {queueName}\n{ex}", corrId, QueueName, ex);
+}

---

198-207: Catch and log exceptions thrown by the TrySendToCollectorAsync and TryDeleteRecordsFromDBAsync methods.

The ReprocessAsync method does not handle exceptions thrown by the TrySendToCollectorAsync and TryDeleteRecordsFromDBAsync methods. It should catch and log exceptions to avoid terminating the method.

Apply this diff to fix the issue:

+try
+{
     ITelemetryCollector<TRecord>.AddResult? result = await TrySendToCollectorAsync(records, corrId);
+}
+catch (Exception ex)
+{
+    _logger?.LogError("{corrId} error while sending records to collector in {queueName}\n{ex}", corrId, QueueName, ex);
+}
+
+try
+{
     if (result != null && result.Successful)
     {
         await TryDeleteRecordsFromDBAsync(records, corrId);
     }
 }
+catch (Exception ex)
+{
+    _logger?.LogError("{corrId} error while deleting records from db in {queueName}\n{ex}", corrId, QueueName, ex);
+}

---

165-180: Catch and log exceptions thrown by the TryAddToDBAsync, TrySendToCollectorAsync, and TryDeleteRecordsFromDBAsync methods.

The ProcessAsync method does not handle exceptions thrown by the TryAddToDBAsync, TrySendToCollectorAsync, and TryDeleteRecordsFromDBAsync methods. It should catch and log exceptions to avoid terminating the method.

Apply this diff to fix the issue:

 try
 {
     await TryAddToDBAsync(records, corrId);
 }
+catch (Exception ex)
+{
+    _logger?.LogError("{corrId} error while adding records to db in {queueName}\n{ex}", corrId, QueueName, ex);
+}
+
+try
+{
     ITelemetryCollector<TRecord>.AddResult? result = await TrySendToCollectorAsync(records, corrId);
+}
+catch (Exception ex)
+{
+    _logger?.LogError("{corrId} error while sending records to collector in {queueName}\n{ex}", corrId, QueueName, ex);
+}
+
+try
+{
     if (result != null && result.Successful)
     {
         await TryDeleteRecordsFromDBAsync(records, corrId);
     }
 }
+catch (Exception ex)
+{
+    _logger?.LogError("{corrId} error while deleting records from db in {queueName}\n{ex}", corrId, QueueName, ex);
+}

Files that changed from the base of the PR and between dbcacdf and be7cdc5.

• Ginger/GingerCoreNET/Telemetry/Protos/V1/feature.proto (1 hunks)
• Ginger/GingerCoreNET/Telemetry/Protos/V1/log.proto (1 hunks)
• Ginger/GingerCoreNET/Telemetry/TelemetryBaseRecord.cs (1 hunks)
• Ginger/GingerCoreNET/Telemetry/TelemetryCollector.cs (1 hunks)
• Ginger/GingerCoreNET/Telemetry/TelemetryQueueManager.cs (1 hunks)

• Ginger/GingerCoreNET/Telemetry/Protos/V1/log.proto
• Ginger/GingerCoreNET/Telemetry/TelemetryBaseRecord.cs
• Ginger/GingerCoreNET/Telemetry/TelemetryCollector.cs
• Ginger/GingerCoreNET/Telemetry/TelemetryQueueManager.cs

Ginger/GingerCoreNET/Telemetry/Protos/V1/feature.proto (3)

5-15: LGTM!

The FeatureRecord message is well-structured and captures relevant information for feature telemetry. The field names are descriptive, and the field types are appropriate.

---

22-24: LGTM!

The AddFeaturesResponse message is well-structured and provides confirmation of the number of feature records added.

---

26-28: LGTM!

The FeatureCollector service and its Collect method provide a clean and straightforward interface for collecting feature telemetry data.