From bf1811a50284329598d480426b62c2c90dd3df61 Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Sat, 7 Jun 2025 15:05:02 -0400 Subject: [PATCH 1/5] fix tracing client-server --- .../AndroidUploader.cs | 8 +- .../AutoUploader.cs | 2 +- src/SymbolCollector.Android/MainActivity.cs | 58 ++++++------ .../ConsoleUploader.cs | 4 +- src/SymbolCollector.Core/Client.cs | 90 +++++++++++-------- .../SymbolCollector.Core.Tests/ClientTests.cs | 4 +- 6 files changed, 96 insertions(+), 70 deletions(-) diff --git a/src/SymbolCollector.Android.Library/AndroidUploader.cs b/src/SymbolCollector.Android.Library/AndroidUploader.cs index 7967eeb..7de89a0 100644 --- a/src/SymbolCollector.Android.Library/AndroidUploader.cs +++ b/src/SymbolCollector.Android.Library/AndroidUploader.cs @@ -15,7 +15,7 @@ public AndroidUploader(Client client, ILogger logger) _logger = logger; } - public Task StartUpload(string friendlyName, CancellationToken token) => + public Task StartUpload(string friendlyName, ISpan uploadSpan, CancellationToken token) => Task.Run(async () => { var paths = new[] { "/system/lib", "/system/lib64", "/system/", "/vendor/lib" }; @@ -23,17 +23,21 @@ public Task StartUpload(string friendlyName, CancellationToken token) => _logger.LogInformation("Using friendly name: {friendlyName} and paths: {paths}", friendlyName, paths); + var androidUploaderSpan = uploadSpan.StartChild("androidUpload", "uploading all paths async"); try { - await _client.UploadAllPathsAsync(friendlyName, BatchType.Android, paths, token); + await _client.UploadAllPathsAsync(friendlyName, BatchType.Android, paths, androidUploaderSpan, token); + androidUploaderSpan.Finish(); } catch (OperationCanceledException) { + androidUploaderSpan.Finish(SpanStatus.Cancelled); } catch (Exception e) { _logger.LogError(e, "Failed uploading {friendlyName} paths: {paths}", friendlyName, paths); + androidUploaderSpan.Finish(e); // Make sure event is flushed and rethrow await SentrySdk.FlushAsync(TimeSpan.FromSeconds(3)); throw; diff --git a/src/SymbolCollector.Android.Library/AutoUploader.cs b/src/SymbolCollector.Android.Library/AutoUploader.cs index 404a3c0..44e5992 100644 --- a/src/SymbolCollector.Android.Library/AutoUploader.cs +++ b/src/SymbolCollector.Android.Library/AutoUploader.cs @@ -67,7 +67,7 @@ public void Run(Context context) s.Contexts.OperatingSystem.KernelVersion = uname.Release; } }); - var uploadTask = uploader.StartUpload(friendlyName, source.Token); + var uploadTask = uploader.StartUpload(friendlyName, tran, source.Token); uploadTask.ContinueWith(t => { tran.Finish(t.IsCompletedSuccessfully ? SpanStatus.Ok : SpanStatus.UnknownError); diff --git a/src/SymbolCollector.Android/MainActivity.cs b/src/SymbolCollector.Android/MainActivity.cs index af2a8d8..bfab59f 100644 --- a/src/SymbolCollector.Android/MainActivity.cs +++ b/src/SymbolCollector.Android/MainActivity.cs @@ -21,7 +21,7 @@ public class MainActivity : Activity private string _friendlyName = null!; // set on OnCreate private IHost _host = null!; // set on OnCreate private IServiceProvider _serviceProvider = null!; // set on OnCreate - private ITransactionTracer _startupTransaction = null!; // set on OnCreate + private SentryTraceHeader _parentTraceHeader = null!; protected override void OnCreate(Bundle? savedInstanceState) { @@ -35,10 +35,12 @@ protected override void OnCreate(Bundle? savedInstanceState) _serviceProvider = _host.Services; // It's set in Host.Init above - SentrySdk.ConfigureScope(s => _startupTransaction = s.Transaction!); + ITransactionTracer startupTransaction = null!; + SentrySdk.ConfigureScope(s => startupTransaction = s.Transaction!); + _parentTraceHeader = startupTransaction.GetTraceHeader(); AddSentryContext(); - var span = _startupTransaction.StartChild("OnCreate"); + var onCreateSpan = startupTransaction.StartChild("OnCreate"); try { base.OnCreate(savedInstanceState); @@ -71,10 +73,23 @@ protected override void OnCreate(Bundle? savedInstanceState) uploadButton.Click += OnUploadButtonOnClick; cancelButton.Click += OnCancelButtonOnClick; + onCreateSpan.Finish(SpanStatus.Ok); + startupTransaction.Finish(SpanStatus.Ok); + return; + + void OnCancelButtonOnClick(object? sender, EventArgs args) + { + SentrySdk.AddBreadcrumb("OnCancelButtonOnClick", category: "ui.event"); + Unfocus(); + source.Cancel(); + } + async void OnUploadButtonOnClick(object? sender, EventArgs args) { // The scope tracks the overall app transaction while this is only batch uploading - var uploadTransaction = SentrySdk.StartTransaction("BatchUpload", "batch.upload", _startupTransaction.GetTraceHeader()); + var uploadTransaction = SentrySdk.StartTransaction("BatchUpload", "batch.upload", _parentTraceHeader); + // startup transaction has completed at this point, this starts when the user pressed Upload so make this transaction global: + SentrySdk.ConfigureScope(s => s.Transaction = uploadTransaction); try { @@ -89,10 +104,11 @@ async void OnUploadButtonOnClick(object? sender, EventArgs args) uploadButton.Enabled = false; source = new CancellationTokenSource(); - var uploadTask = uploader.StartUpload(_friendlyName, source.Token); + var uploadTask = uploader.StartUpload(_friendlyName, uploadTransaction, source.Token); var updateUiTask = StartUiUpdater(source.Token, metrics); - await UploadAsync(uploadTask, updateUiTask, metrics, cancelButton, uploadButton, uploadTransaction, source); + // Successful completion (or timeout) of the transaction is handled internally. + await AwaitOnUploadAndCancellationTasks(uploadTask, updateUiTask, metrics, cancelButton, uploadButton, uploadTransaction, source); } catch (Exception e) { @@ -100,21 +116,11 @@ async void OnUploadButtonOnClick(object? sender, EventArgs args) throw; } } - - void OnCancelButtonOnClick(object? sender, EventArgs args) - { - SentrySdk.AddBreadcrumb("OnCancelButtonOnClick", category: "ui.event"); - Unfocus(); - source.Cancel(); - } - - span.Finish(SpanStatus.Ok); - _startupTransaction.Finish(SpanStatus.Ok); } catch (Exception e) { - span.Finish(e); - _startupTransaction.Finish(e); + onCreateSpan.Finish(e); + startupTransaction.Finish(e); throw; } } @@ -128,13 +134,13 @@ private void Unfocus() } } - private async Task UploadAsync( + private async Task AwaitOnUploadAndCancellationTasks( Task uploadTask, Task updateUiTask, ClientMetrics metrics, View cancelButton, View uploadButton, - ISpan span, + ISpan uploadTransaction, CancellationTokenSource source) { var container = base.FindViewById(Resource.Id.metrics_container)!; @@ -159,28 +165,28 @@ private async Task UploadAsync( ranForContainer.Visibility = ViewStates.Visible; ranForLabel.Text = metrics.RanFor.ToString(); - span.Finish(SpanStatus.Ok); + uploadTransaction.Finish(SpanStatus.Ok); } else if (uploadTask.IsFaulted) { await ShowError(uploadTask.Exception); - span.Finish(SpanStatus.InternalError); + uploadTransaction.Finish(SpanStatus.InternalError); } else { cancelButton.Enabled = false; uploadButton.Enabled = true; - span.Finish(SpanStatus.Cancelled); + uploadTransaction.Finish(SpanStatus.Cancelled); } } catch (Exception e) { await ShowError(e); - span.Finish(e); + uploadTransaction.Finish(e); } finally { - source.Cancel(); + await source.CancelAsync(); } } @@ -278,8 +284,6 @@ private void AddSentryContext() SentrySdk.ConfigureScope(s => { - s.Transaction = _startupTransaction; - if (uname is { }) { s.Contexts["uname"] = new diff --git a/src/SymbolCollector.Console/ConsoleUploader.cs b/src/SymbolCollector.Console/ConsoleUploader.cs index 920ff83..6cbe135 100644 --- a/src/SymbolCollector.Console/ConsoleUploader.cs +++ b/src/SymbolCollector.Console/ConsoleUploader.cs @@ -59,7 +59,7 @@ public async Task StartUploadSymbols(IEnumerable paths, string bundleId, var type = batchType ?? DeviceBatchType(); _logger.LogInformation("Uploading bundle {bundleId} of type {type} and paths: {paths}", bundleId, type, paths); - await _client.UploadAllPathsAsync(bundleId, type, paths, token); + await _client.UploadAllPathsAsync(bundleId, type, paths, transaction, token); transaction.Finish(SpanStatus.Ok); } catch (Exception e) @@ -87,4 +87,4 @@ static BatchType DeviceBatchType() throw new InvalidOperationException("No BatchType available for the current device."); } -} \ No newline at end of file +} diff --git a/src/SymbolCollector.Core/Client.cs b/src/SymbolCollector.Core/Client.cs index 828b7c5..63e1acb 100644 --- a/src/SymbolCollector.Core/Client.cs +++ b/src/SymbolCollector.Core/Client.cs @@ -30,59 +30,75 @@ public Client( SentrySdk.ConfigureScope(s => s.SetExtra(nameof(Metrics), Metrics)); } - public async Task UploadAllPathsAsync( - string friendlyName, + public async Task UploadAllPathsAsync(string friendlyName, BatchType type, IEnumerable topLevelPaths, + ISpan span, CancellationToken cancellationToken) { - var groupsSpan = SentrySdk.GetSpan()?.StartChild("group.get", "Get the group of directories to search in parallel"); + List> groups; var counter = 0; - var groups = - (from topPath in topLevelPaths - from lookupDirectory in SafeGetDirectories(topPath) - where _blockListedPaths?.Contains(lookupDirectory) != true - let c = counter++ - group lookupDirectory by c / ParallelTasks - into grp - select grp.ToList()).ToList(); - groupsSpan?.Finish(); - - var startSpan = SentrySdk.GetSpan()?.StartChild("batch.start"); - Guid batchId; - try { - batchId = await _symbolClient.Start(friendlyName, type, cancellationToken); - startSpan?.Finish(SpanStatus.Ok); + var groupsSpan = span.StartChild("group.get", "Get the group of directories to search in parallel"); + groups = + (from topPath in topLevelPaths + from lookupDirectory in SafeGetDirectories(topPath) + where _blockListedPaths?.Contains(lookupDirectory) != true + let c = counter++ + group lookupDirectory by c / ParallelTasks + into grp + select grp.ToList()).ToList(); + groupsSpan.Finish(); } - catch (Exception e) + + Guid batchId; { - startSpan?.Finish(e); - throw; + var startSpan = span.StartChild("batch.start"); + try + { + batchId = await _symbolClient.Start(friendlyName, type, cancellationToken); + startSpan.Finish(SpanStatus.Ok); + } + catch (Exception e) + { + startSpan.Finish(e); + throw; + } } - var uploadSpan = SentrySdk.GetSpan()?.StartChild("batch.upload"); - uploadSpan?.SetTag("groups", groups.Count.ToString()); - uploadSpan?.SetTag("total_items", counter.ToString()); - try { - foreach (var group in groups) + var uploadSpan = span.StartChild("batch.upload", "concurrent batch upload"); + uploadSpan.SetData("groups", groups.Count.ToString()); + uploadSpan.SetData("total_items", counter.ToString()); + + // use this as parent to all outgoing HTTP requests now: + SentrySdk.ConfigureScope(s => s.Span = uploadSpan); + int i = 0; + try { - await UploadParallel(batchId, group, cancellationToken); + foreach (var group in groups) + { + if (i++ == 3) break; + await UploadParallel(batchId, group, cancellationToken); + } + uploadSpan.Finish(SpanStatus.Ok); + } + catch (Exception e) + { + _logger.LogError(e, "Failed processing files for {batchId}. Rethrowing and leaving the batch open.", + batchId); + uploadSpan.Finish(e); + throw; } - uploadSpan?.Finish(SpanStatus.Ok); } - catch (Exception e) + { - uploadSpan?.Finish(e); - _logger.LogError(e, "Failed processing files for {batchId}. Rethrowing and leaving the batch open.", - batchId); - throw; + var stopSpan = span.StartChild("batch.close"); + await _symbolClient.Close(batchId, cancellationToken); + stopSpan.Finish(SpanStatus.Ok); } - var stopSpan = SentrySdk.GetSpan()?.StartChild("batch.close"); - await _symbolClient.Close(batchId, cancellationToken); - stopSpan?.Finish(SpanStatus.Ok); + return; IEnumerable SafeGetDirectories(string path) { @@ -122,6 +138,8 @@ private async Task UploadParallel(Guid batchId, IEnumerable paths, Cance { tasks.Add(UploadFilesAsync(batchId, path, cancellationToken)); Metrics.JobsInFlightAdd(1); + // TODO: Remove me. Keeping it short to test e2e + // return; } else { diff --git a/test/SymbolCollector.Core.Tests/ClientTests.cs b/test/SymbolCollector.Core.Tests/ClientTests.cs index 28cc50c..f61ae4c 100644 --- a/test/SymbolCollector.Core.Tests/ClientTests.cs +++ b/test/SymbolCollector.Core.Tests/ClientTests.cs @@ -70,7 +70,7 @@ public async Task UploadAllPathsAsync_TestFilesDirectory_FilesDetected() new HttpClient(_fixture.HttpMessageHandler)); var sut = _fixture.GetSut(); - await sut.UploadAllPathsAsync("friendly name", BatchType.IOS, new[] {"TestFiles"}, CancellationToken.None); + await sut.UploadAllPathsAsync("friendly name", BatchType.IOS, new[] {"TestFiles"}, SentrySdk.StartTransaction("test", "test-op"), CancellationToken.None); // number of valid test files in TestFiles Assert.Equal(12, counter); @@ -84,7 +84,7 @@ public async Task UploadAllPathsAsync_TestFilesDirectory_FileCorrectlySent() Substitute.For>(), new FatBinaryReader()); var sut = _fixture.GetSut(); - await sut.UploadAllPathsAsync("friendly name", BatchType.IOS, new[] {"TestFiles"}, CancellationToken.None); + await sut.UploadAllPathsAsync("friendly name", BatchType.IOS, new[] {"TestFiles"}, SentrySdk.StartTransaction("test", "test-op"), CancellationToken.None); // Make sure all valid test files were picked up var testFiles = new ObjectFileResultTestCases() From 841344bf354c2bde03efbee0a205d94953c55467 Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Sat, 7 Jun 2025 16:42:17 -0400 Subject: [PATCH 2/5] parent-child http trace fixed --- src/Directory.Packages.props | 2 +- src/SymbolCollector.Core/Client.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index cfbec33..f1c2211 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -1,7 +1,7 @@ true - 5.9.0 + 5.11.0-alpha.1 diff --git a/src/SymbolCollector.Core/Client.cs b/src/SymbolCollector.Core/Client.cs index 63e1acb..5dac01c 100644 --- a/src/SymbolCollector.Core/Client.cs +++ b/src/SymbolCollector.Core/Client.cs @@ -78,7 +78,7 @@ into grp { foreach (var group in groups) { - if (i++ == 3) break; + if (i++ == 2) break; await UploadParallel(batchId, group, cancellationToken); } uploadSpan.Finish(SpanStatus.Ok); From 92999f0aa9324bf3ee3fd6afb64c6d6fa2e7b897 Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Sat, 7 Jun 2025 20:15:43 -0400 Subject: [PATCH 3/5] Update src/SymbolCollector.Core/Client.cs --- src/SymbolCollector.Core/Client.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SymbolCollector.Core/Client.cs b/src/SymbolCollector.Core/Client.cs index 5dac01c..57be480 100644 --- a/src/SymbolCollector.Core/Client.cs +++ b/src/SymbolCollector.Core/Client.cs @@ -78,7 +78,6 @@ into grp { foreach (var group in groups) { - if (i++ == 2) break; await UploadParallel(batchId, group, cancellationToken); } uploadSpan.Finish(SpanStatus.Ok); From a6fc9d34153ca06199d5b6ae137293fd62abe499 Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Sat, 7 Jun 2025 20:16:06 -0400 Subject: [PATCH 4/5] Update src/SymbolCollector.Core/Client.cs --- src/SymbolCollector.Core/Client.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/SymbolCollector.Core/Client.cs b/src/SymbolCollector.Core/Client.cs index 57be480..4739c60 100644 --- a/src/SymbolCollector.Core/Client.cs +++ b/src/SymbolCollector.Core/Client.cs @@ -137,8 +137,6 @@ private async Task UploadParallel(Guid batchId, IEnumerable paths, Cance { tasks.Add(UploadFilesAsync(batchId, path, cancellationToken)); Metrics.JobsInFlightAdd(1); - // TODO: Remove me. Keeping it short to test e2e - // return; } else { From 8a0fdb2594c3028f63f180b9b56b1efea1cbce34 Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Sat, 7 Jun 2025 20:32:39 -0400 Subject: [PATCH 5/5] Update src/SymbolCollector.Core/Client.cs --- src/SymbolCollector.Core/Client.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SymbolCollector.Core/Client.cs b/src/SymbolCollector.Core/Client.cs index 4739c60..1ea4bf3 100644 --- a/src/SymbolCollector.Core/Client.cs +++ b/src/SymbolCollector.Core/Client.cs @@ -73,7 +73,6 @@ into grp // use this as parent to all outgoing HTTP requests now: SentrySdk.ConfigureScope(s => s.Span = uploadSpan); - int i = 0; try { foreach (var group in groups)