Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS native profiling #2930

Merged
merged 46 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
3f8f036
wip: cocoa profiling
vaind Nov 30, 2023
cd43974
use ISerializable in ITransactionProfiler
vaind Dec 1, 2023
fad8eff
fixup profile content
vaind Dec 1, 2023
c2adb5a
dispose stream
vaind Dec 1, 2023
0aa3bf1
fixup sentry.profiling
vaind Dec 1, 2023
ba4982a
build fixes
vaind Dec 1, 2023
fb13b03
Merge branch 'main' into feat/ios-native-profiling
vaind Dec 4, 2023
efa4552
feat: add ProfilesSampleRate
vaind Dec 4, 2023
4533cd9
Apply suggestions from code review
vaind Dec 5, 2023
4e18597
fixup naming
vaind Dec 5, 2023
1727f44
update verifier tests
vaind Dec 5, 2023
bf8e17d
update profiler tests
vaind Dec 5, 2023
e5383e2
build(deps): bump actions/setup-java from 3 to 4 (#2942)
dependabot[bot] Dec 4, 2023
221439c
workaround .net nativeAOT crash (#2943)
vaind Dec 4, 2023
cf734bd
release: 4.0.0-beta.4
getsentry-bot Dec 5, 2023
5bfcdf0
fix: gcp link (#2944)
bruno-garcia Dec 5, 2023
6d4f658
update sample
vaind Dec 5, 2023
8246932
chore: update changelog
vaind Dec 5, 2023
5d12ed0
release: 4.0.0-beta.4
getsentry-bot Dec 5, 2023
67c54fa
Merge remote-tracking branch 'origin/main' into feat/ios-native-profi…
vaind Dec 5, 2023
7fce737
fixup changelog
vaind Dec 5, 2023
f91846a
profiler hub tests
vaind Dec 8, 2023
5acfdc2
improve profiler collect() error handling
vaind Dec 8, 2023
e728541
Merge remote-tracking branch 'origin/main' into feat/ios-native-profi…
vaind Dec 8, 2023
c56da69
fixup device test script
vaind Dec 8, 2023
4a92fac
fix: hub tests
vaind Dec 8, 2023
6772bc1
Update CHANGELOG.md
vaind Dec 8, 2023
07eb383
Update src/Sentry.Profiling/ProfilingIntegration.cs
vaind Dec 8, 2023
37ca589
Update src/Sentry/Platforms/Cocoa/CocoaProfiler.cs
vaind Dec 8, 2023
6364fd0
Update src/Sentry/Platforms/Cocoa/SentrySdk.cs
vaind Dec 8, 2023
20807ab
review changes
vaind Dec 8, 2023
539cc8a
review changes
vaind Dec 8, 2023
1f93d19
profiler integration test
vaind Dec 8, 2023
bf11364
Update ProfilerTests.cs
vaind Dec 8, 2023
a17b3a0
test cleanup
vaind Dec 8, 2023
0c4ebc3
only set profiler factory if not previously set
vaind Dec 10, 2023
45617e7
trying to find stuck test
vaind Dec 10, 2023
f030857
improve logs
vaind Dec 10, 2023
24d1656
fixup device tests
vaind Dec 10, 2023
c2e63ec
include cocoa in device test app
vaind Dec 10, 2023
1dddef1
fix profiler test
vaind Dec 10, 2023
fb339d9
try disabling hub tests
vaind Dec 10, 2023
25099ba
try enabling a potential flaky test
vaind Dec 11, 2023
2b78e3b
try to enable a test case
vaind Dec 12, 2023
e5a069a
try enabling a test case
vaind Dec 12, 2023
ed6dd63
restore CI
vaind Dec 12, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/device-tests-ios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
- release/*
pull_request:
paths-ignore:
- '**.md'
- "**.md"

jobs:
build:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- iOS profiling support (alpha). ([#2930](https://github.com/getsentry/sentry-dotnet/pull/2930))
vaind marked this conversation as resolved.
Show resolved Hide resolved

### Fixes

- Stop Sentry for MacCatalyst from creating `default.profraw` in the app bundle using xcodebuild archive to build sentry-cocoa ([#2960](https://github.com/getsentry/sentry-dotnet/pull/2960))
Expand Down
4 changes: 2 additions & 2 deletions benchmarks/Sentry.Benchmarks/ProfilingBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public void StartProfiler()
public void StopProfiler()
{
_profiler?.Finish();
_profiler?.CollectAsync(new Transaction("", "")).Wait();
(_profiler as SamplingTransactionProfiler)?.CollectAsync(new Transaction("", "")).Wait();
_profiler = null;
_factory.Dispose();
_factory = null;
Expand Down Expand Up @@ -55,7 +55,7 @@ public long Transaction(int runtimeMs, bool collect)
var transaction = new Transaction(tt);
if (collect)
{
var collectTask = tt.TransactionProfiler.CollectAsync(transaction);
var collectTask = (tt.TransactionProfiler as SamplingTransactionProfiler).CollectAsync(transaction);
collectTask.Wait();
}
return result;
Expand Down
39 changes: 39 additions & 0 deletions samples/Sentry.Samples.Ios/AppDelegate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public override bool FinishedLaunching(UIApplication application, NSDictionary l
{
o.Debug = true;
o.Dsn = "https://eb18e953812b41c3aeb042e666fd3b5c@o447951.ingest.sentry.io/5428537";
o.TracesSampleRate = 1.0;
o.ProfilesSampleRate = 1.0;
});

// create a new window instance based on the screen size
Expand Down Expand Up @@ -50,6 +52,43 @@ public override bool FinishedLaunching(UIApplication application, NSDictionary l
// throw new Exception("Test Unhandled Managed Exception");
// SentrySdk.CauseCrash(CrashType.Native);

{
var tx = SentrySdk.StartTransaction("app", "run");
var count = 10;
for (var i = 0; i < count; i++)
{
FindPrimeNumber(100000);
}

tx.Finish();
}

return true;
}

private static long FindPrimeNumber(int n)
{
int count = 0;
long a = 2;
while (count < n)
{
long b = 2;
int prime = 1;// to check if found a prime
while (b * b <= a)
{
if (a % b == 0)
{
prime = 0;
break;
}
b++;
}
if (prime > 0)
{
count++;
}
a++;
}
return (--a);
}
}
8 changes: 6 additions & 2 deletions scripts/device-test.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Push-Location $PSScriptRoot/..
try
{
$tfm = 'net7.0-'
$arch = $(uname -m) -eq 'arm64' ? 'arm64' : 'x64'
$arch = (!$IsWindows -and $(uname -m) -eq 'arm64') ? 'arm64' : 'x64'
if ($Platform -eq 'android')
{
$tfm += 'android'
Expand Down Expand Up @@ -75,7 +75,11 @@ try
}
finally
{
scripts/parse-xunit2-xml.ps1 ./test_output/TestResults.xml | Out-File $env:GITHUB_STEP_SUMMARY
if ($CI)
{
scripts/parse-xunit2-xml.ps1 (Get-Item ./test_output/*.xml).FullName | Out-File $env:GITHUB_STEP_SUMMARY
}

}
}
}
Expand Down
12 changes: 7 additions & 5 deletions scripts/parse-xunit2-xml.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Set-StrictMode -Version Latest

if ([string]::IsNullOrEmpty($File) -or !(Test-Path($File)))
{
Write-Warning "Test output file was not found."
Write-Warning "Test output file was not found: '$File'."

# Return success exit code so that GitHub Actions highlights the failure in the test run, rather than in this script.
return
Expand All @@ -20,7 +20,7 @@ function ElementText([System.Xml.XmlElement] $element)
$summary = "## Summary`n`n"
$summary += "| Assembly | Passed | Failed | Skipped |`n"
$summary += "| -------- | -----: | -----: | ------: |`n"
$failures = ""
$failures = ''
foreach ($assembly in $xml.assemblies.assembly)
{
$summary += "| $($assembly.name) | $($assembly.passed) | $($assembly.failed) | $($assembly.skipped) |`n"
Expand All @@ -30,23 +30,25 @@ foreach ($assembly in $xml.assemblies.assembly)
$failures += "### $($assembly.name)`n"
foreach ($test in $assembly.collection.test)
{
if ($test.result -eq "Pass")
if ($test.result -eq 'Pass')
{
continue
}

$failures += "#### $($test.name.Replace('\"', '"'))"
if ($test.result -eq "Skip")
if ($test.result -eq 'Skip')
{
$failures += " - Skipped`n"
$failures += "$(ElementText $test.reason)"
}
else
{
$failures += " - $($test.result)ed`n"
if ($test.PSobject.Properties.name -match "output")
if ($test.PSobject.Properties.name -match 'output')
{
$failures += '```' + "`n"
$failures += "$(ElementText $test.output)`n"
$failures += '```'
}
$failures += '```' + "`n"
$failures += "$(ElementText $test.failure.message)`n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ public class ProfilingIntegration : ISdkIntegration
/// <inheritdoc/>
public void Register(IHub hub, SentryOptions options)
{
options.TransactionProfilerFactory = SamplingTransactionProfilerFactory.Create(options);
if (options.IsProfilingEnabled)
{
options.TransactionProfilerFactory ??= SamplingTransactionProfilerFactory.Create(options);
}
}
}
7 changes: 5 additions & 2 deletions src/Sentry.Profiling/SamplingTransactionProfiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ public void Finish()
}

/// <inheritdoc />
public async Task<ProfileInfo> CollectAsync(Transaction transaction)
public Protocol.Envelopes.ISerializable? Collect(Transaction transaction)
=> Protocol.Envelopes.AsyncJsonSerializable.CreateFrom(CollectAsync(transaction));

internal async Task<ProfileInfo> CollectAsync(Transaction transaction)
{
if (!_stopped)
{
throw new InvalidOperationException("Profiler.CollectAsync() called before Finish()");
throw new InvalidOperationException("Profiler.Collect() called before Finish()");
vaind marked this conversation as resolved.
Show resolved Hide resolved
}

// Wait for the last sample (<= _endTimeMs), or at most 1 second. The timeout shouldn't happen because
Expand Down
2 changes: 2 additions & 0 deletions src/Sentry/BindableSentryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ internal partial class BindableSentryOptions
public bool? EnableTracing { get; set; }
public double? TracesSampleRate { get; set; }
public List<string>? TracePropagationTargets { get; set; }
public double? ProfilesSampleRate { get; set; }
public StackTraceMode? StackTraceMode { get; set; }
public long? MaxAttachmentSize { get; set; }
public StartupTimeDetectionMode? DetectStartupTime { get; set; }
Expand Down Expand Up @@ -82,6 +83,7 @@ public void ApplyTo(SentryOptions options)
options.DefaultTags = DefaultTags ?? options.DefaultTags;
options.EnableTracing = EnableTracing ?? options.EnableTracing;
options.TracesSampleRate = TracesSampleRate ?? options.TracesSampleRate;
options.ProfilesSampleRate = ProfilesSampleRate ?? options.ProfilesSampleRate;
options.TracePropagationTargets = TracePropagationTargets?.Select(s => new SubstringOrRegexPattern(s)).ToList() ?? options.TracePropagationTargets;
options.StackTraceMode = StackTraceMode ?? options.StackTraceMode;
options.MaxAttachmentSize = MaxAttachmentSize ?? options.MaxAttachmentSize;
Expand Down
16 changes: 9 additions & 7 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ internal ITransactionTracer StartTransaction(
transaction.SampleRate = sampleRate;
}

if (transaction.IsSampled is true && _options.TransactionProfilerFactory is { } profilerFactory)
if (transaction.IsSampled is true &&
_options.TransactionProfilerFactory is { } profilerFactory &&
_randomValuesFactory.NextBool(_options.ProfilesSampleRate ?? 0.0))
bruno-garcia marked this conversation as resolved.
Show resolved Hide resolved
{
// TODO cancellation token based on Hub being closed?
transaction.TransactionProfiler = profilerFactory.Start(transaction, CancellationToken.None);
Expand Down Expand Up @@ -200,7 +202,7 @@ public SentryTraceHeader GetTraceHeader()

public BaggageHeader GetBaggage()
{
if (GetSpan() is TransactionTracer { DynamicSamplingContext: { IsEmpty: false } dsc } )
if (GetSpan() is TransactionTracer { DynamicSamplingContext: { IsEmpty: false } dsc })
{
return dsc.ToBaggageHeader();
}
Expand Down Expand Up @@ -528,11 +530,11 @@ public void Dispose()
#elif ANDROID
// TODO
#elif NET8_0_OR_GREATER
if (AotHelper.IsNativeAot)
{
_options?.LogDebug("Closing native SDK");
SentrySdk.CloseNativeSdk();
}
if (AotHelper.IsNativeAot)
{
_options?.LogDebug("Closing native SDK");
SentrySdk.CloseNativeSdk();
}
#endif
}

Expand Down
7 changes: 3 additions & 4 deletions src/Sentry/Internal/ITransactionProfiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ internal interface ITransactionProfiler
/// </summary>
void Finish();

/// <summary>
/// Process and collect the profile.
/// </summary>
Task<ProfileInfo> CollectAsync(Transaction transaction);
/// <summary>Process and collect the profile.</summary>
/// <returns>The collected profile.</returns>
ISerializable? Collect(Transaction transaction);
}
16 changes: 9 additions & 7 deletions src/Sentry/Internal/SentryStopwatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ internal struct SentryStopwatch
{
private static readonly double StopwatchTicksPerTimeSpanTick =
(double)Stopwatch.Frequency / TimeSpan.TicksPerSecond;
private static readonly double StopwatchTicksPerNs = (double)Stopwatch.Frequency / 1000000000.0;

private long _startTimestamp;
private DateTimeOffset _startDateTimeOffset;
Expand All @@ -21,14 +22,15 @@ internal struct SentryStopwatch
public DateTimeOffset StartDateTimeOffset => _startDateTimeOffset;
public DateTimeOffset CurrentDateTimeOffset => _startDateTimeOffset + Elapsed;

private long Diff() => Stopwatch.GetTimestamp() - _startTimestamp;

public TimeSpan Elapsed
{
get
{
var now = Stopwatch.GetTimestamp();
var diff = now - _startTimestamp;
var ticks = (long)(diff / StopwatchTicksPerTimeSpanTick);
return TimeSpan.FromTicks(ticks);
}
get => TimeSpan.FromTicks((long)(Diff() / StopwatchTicksPerTimeSpanTick));
}

public ulong ElapsedNanoseconds
{
get => (ulong)(Diff() / StopwatchTicksPerNs);
}
}
1 change: 0 additions & 1 deletion src/Sentry/Internal/SerializableExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using Sentry.Extensibility;
using Sentry.Infrastructure;
using Sentry.Protocol.Envelopes;
using ISerializable = Sentry.Protocol.Envelopes.ISerializable;

namespace Sentry.Internal;

Expand Down
62 changes: 62 additions & 0 deletions src/Sentry/Platforms/Cocoa/CocoaProfiler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using Sentry.Extensibility;
using Sentry.Internal;
using Sentry.Cocoa.Extensions;
using Sentry.Cocoa.Facades;

namespace Sentry.Cocoa;

internal class CocoaProfiler : ITransactionProfiler
{
private readonly SentryOptions _options;
private readonly SentryId _traceId;
private readonly CocoaSdk.SentryId _cocoaTraceId;
private readonly ulong _startTimeNs;
private ulong _endTimeNs;
private readonly SentryStopwatch _stopwatch;

public CocoaProfiler(SentryOptions options, ulong startTimeNs, SentryId traceId, CocoaSdk.SentryId cocoaTraceId)
{
_stopwatch = SentryStopwatch.StartNew();
_options = options;
_startTimeNs = startTimeNs;
_traceId = traceId;
_cocoaTraceId = cocoaTraceId;
_options.LogDebug("Trace {0} profile start timestamp: {1} ns", _traceId, _startTimeNs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to pass in the actual timestamp here and then turn this into TimeNs to be human readable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, but this is what the native profiler works with so when you're actually "debugging" based on the log output (that's why it's LogDebug()), you see what is actually passed. I was going to do a conversion to DateTime but it seems it doesn't support nanosecond precision.

}

/// <inheritdoc />
public void Finish()
{
if (_endTimeNs == 0)
{
_endTimeNs = _startTimeNs + (ulong)_stopwatch.ElapsedNanoseconds;
_options.LogDebug("Trace {0} profile end timestamp: {1} ns", _traceId, _endTimeNs);
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved
}
}

public ISerializable? Collect(Transaction transaction)
{
// TODO change return type of CocoaSDKs CollectProfileBetween to NSMutableDictionary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a ticket there not to forget this?

Copy link
Collaborator Author

@vaind vaind Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already made the change in sentry-cocoa and it's merged but I've missed last-week's release so I'm keeping as is with hopes this gets resolved before the PR is merged. If not, I'll create an issue

var payload = SentryCocoaHybridSdk.CollectProfileBetween(_startTimeNs, _endTimeNs, _cocoaTraceId)?.MutableCopy() as NSMutableDictionary;
if (payload is null)
{
_options.LogWarning("Trace {0} collected profile payload is null", _traceId);
return null;
}
_options.LogDebug("Trace {0} profile payload collected", _traceId);

var payloadTx = payload["transaction"]?.MutableCopy() as NSMutableDictionary;
if (payloadTx is null)
{
_options.LogWarning("Trace {0} collected profile payload doesn't have transaction information", _traceId);
return null;
}

payloadTx["id"] = transaction.EventId.ToString().ToNSString();
payloadTx["trace_id"] = _traceId.ToString().ToNSString();
payloadTx["name"] = transaction.Name.ToNSString();
payload["transaction"] = payloadTx;
payload["timestamp"] = transaction.StartTimestamp.ToString("o", CultureInfo.InvariantCulture).ToNSString();
return new SerializableNSObject(payload);
}
}
22 changes: 22 additions & 0 deletions src/Sentry/Platforms/Cocoa/CocoaProfilerFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
using Sentry.Internal;
using Sentry.Cocoa.Extensions;

namespace Sentry.Cocoa;

internal class CocoaProfilerFactory : ITransactionProfilerFactory
{
private readonly SentryOptions _options;

internal CocoaProfilerFactory(SentryOptions options)
{
_options = options;
}

/// <inheritdoc />
public ITransactionProfiler? Start(ITransactionTracer tracer, CancellationToken cancellationToken)
{
var traceId = tracer.TraceId.ToCocoaSentryId();
var startTime = SentryCocoaHybridSdk.StartProfilerForTrace(traceId);
return new CocoaProfiler(_options, startTime, tracer.TraceId, traceId);
}
}
2 changes: 2 additions & 0 deletions src/Sentry/Platforms/Cocoa/Extensions/CocoaExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ internal static class CocoaExtensions

public static NSDate ToNSDate(this DateTimeOffset timestamp) => (NSDate)timestamp.UtcDateTime;

public static NSString ToNSString(this string str) => new NSString(str);

public static string? ToJsonString(this NSObject? obj, IDiagnosticLogger? logger = null)
{
using var data = obj.ToJsonData(logger);
Expand Down
Loading