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

feat: seperate build & runtime scriptable options configure() #1046

Merged
merged 31 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3647638
feat: seperate build & runtime scriptable options configure()
vaind Oct 25, 2022
a6ad3e6
Format code
getsentry-bot Oct 25, 2022
030e98e
fix: OptionsConfigurationDotNet::SetScript()
vaind Nov 4, 2022
bf3849b
fix: sample CustomOptionsConfiguration missing asset
vaind Nov 4, 2022
2bfae0b
feat: don't throw in default CustomScriptableOptions
vaind Nov 7, 2022
1819e95
feat: configure CliOptions through ConfigureAtBuild()
vaind Nov 7, 2022
c31abdd
fix: ScriptableOptionsConfiguration must not have nullable args or Un…
vaind Nov 7, 2022
f07d055
fix: PerformanceAutoInstrumentation shouldn't load scripts unless bui…
vaind Nov 7, 2022
a31d4bb
test: fix test regression
vaind Nov 8, 2022
40f79d2
test: move integration test build-time options to SmokeTestOptions.cs
vaind Nov 8, 2022
f11f3bc
chore: minor code deduplication
vaind Nov 8, 2022
68a081e
Merge branch 'main' into rework/scriptable-options
vaind Nov 8, 2022
08aaeec
fix: update PerformanceAutoInstrumentation after merge
vaind Nov 8, 2022
3aecf9c
chore: add Enabled suffix to PerformanceAutoInstrumentation option
vaind Nov 8, 2022
ac7b784
chore: update changelog
vaind Nov 8, 2022
1027341
test: fix integration test; improve custom configuration template
vaind Nov 8, 2022
728d3cb
test: fail on flaky retry test
vaind Nov 8, 2022
febda42
test: fix run-smoke-test.ps1
vaind Nov 8, 2022
df9cc9f
chore: improve logs
vaind Nov 8, 2022
bd0834a
test: improve integration test scripts
vaind Nov 8, 2022
0bc9d06
test: debug logging in smoke test
vaind Nov 8, 2022
df4b4e6
test: fix SmokeTestOptions
vaind Nov 8, 2022
1a37cd6
test: integration test script fixes
vaind Nov 9, 2022
26300ae
refactor: split runtime and builtime configuration scripts
vaind Nov 9, 2022
3169393
ci: reenable android smoke test
vaind Nov 9, 2022
f48dbaa
test: fix smoke test
vaind Nov 9, 2022
ba515c8
Merge branch 'main' into rework/scriptable-options
vaind Nov 9, 2022
c141697
chore: improve integration test script
vaind Nov 9, 2022
b9196df
Merge branch 'main' into rework/scriptable-options
vaind Nov 10, 2022
eb36ddc
fix: update asset name created in OptionsConfigurationTab
vaind Nov 10, 2022
ba040e7
chore: update sample in the package
vaind Nov 10, 2022
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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ jobs:
disk-size: 4096M # Some runs have out of storage error when installing the smoke test.
emulator-options: -no-snapshot-save -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none -accel on
disable-animations: true
script: pwsh ./scripts/smoke-test-droid.ps1 -IsIntegrationTest
script: pwsh ./scripts/smoke-test-droid.ps1 -IsIntegrationTest -WarnIfFlaky

- name: Kill emulator if AVD failed.
if: ${{ steps.smoke-test.outputs.status != 'success' }}
Expand Down Expand Up @@ -527,7 +527,7 @@ jobs:
path: samples/IntegrationTest/Build

- name: Build iOS package
run: ./Scripts/smoke-test-ios.ps1 Build -IsIntegrationTest -UnityVersion "${{ matrix.unity-version }}"
run: ./scripts/smoke-test-ios.ps1 Build -IsIntegrationTest -UnityVersion "${{ matrix.unity-version }}"

- name: Upload iOS test app for smoke test.
uses: actions/upload-artifact@v3
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
### Features

- Added Unity version to event context ([#1072](https://github.com/getsentry/sentry-unity/pull/1072))
- Add build-time `ScriptableOptionsConfiguration` scripting interface to support changing settings for native integrations and CLI ([#1046](https://github.com/getsentry/sentry-unity/pull/1046))

### Fixes

- Auto Instrumentation now correctly resolves prebuilt assemblies ([#1066](https://github.com/getsentry/sentry-unity/pull/1066))
- Newly created `ScriptableOptionsConfiguration` script not being set in editor window UI ([#1046](https://github.com/getsentry/sentry-unity/pull/1046))

## 0.25.1

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
%YAML 1.1
vaind marked this conversation as resolved.
Show resolved Hide resolved
%TAG !u! tag:unity3d.com,2011:
--- !u!114 &11400000
MonoBehaviour:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
m_GameObject: {fileID: 0}
m_Enabled: 1
m_EditorHideFlags: 0
m_Script: {fileID: 11500000, guid: cf00f463fb8394bd4aaa371f7db39685, type: 3}
m_Name: CustomOptionsConfiguration
m_EditorClassIdentifier:

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ MonoBehaviour:
<CaptureInEditor>k__BackingField: 1
<EnableLogDebouncing>k__BackingField: 0
<TracesSampleRate>k__BackingField: 0
<AutoInstrumentPerformance>k__BackingField: 0
<PerformanceAutoInstrumentationEnabled>k__BackingField: 1
<AutoSessionTracking>k__BackingField: 1
<AutoSessionTrackingInterval>k__BackingField: 30000
<ReleaseOverride>k__BackingField:
Expand Down Expand Up @@ -47,7 +47,7 @@ MonoBehaviour:
<MacosNativeSupportEnabled>k__BackingField: 1
<LinuxNativeSupportEnabled>k__BackingField: 1
<Il2CppLineNumberSupportEnabled>k__BackingField: 1
<OptionsConfiguration>k__BackingField: {fileID: 11400000, guid: bda1a724b0875436aade31393b0129c0,
<OptionsConfiguration>k__BackingField: {fileID: 11400000, guid: 0f38fa91024b747cebbd0d10e603e363,
type: 2}
<Debug>k__BackingField: 1
<DebugOnlyInEditor>k__BackingField: 0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
using Sentry.Unity;
using UnityEngine;

[CreateAssetMenu(fileName = "Assets/Resources/Sentry/CustomOptionsConfiguration", menuName = "Sentry/CustomOptionsConfiguration", order = 999)]
public class CustomOptionsConfiguration : ScriptableOptionsConfiguration
[CreateAssetMenu(fileName = "Assets/Resources/Sentry/CustomOptionsConfiguration.cs", menuName = "Sentry/CustomOptionsConfiguration", order = 999)]
public class CustomOptionsConfiguration : Sentry.Unity.ScriptableOptionsConfiguration
{
// This method gets called when you instantiated the scriptable object and added it to the configuration window
/// See base class for documentation.
/// Learn more at https://docs.sentry.io/platforms/unity/configuration/options/#programmatic-configuration
public override void Configure(SentryUnityOptions options)
{
// NOTE: Changes to the options object done here will not affect native crashes. The native SDKs only take
// options defined in the Sentry editor configuration window.
// Learn more at: https://docs.sentry.io/platforms/unity/native-support/configuration/
Debug.Log("CustomOptionsConfiguration::Configure() called");

/// BeforeSend is only relevant at runtime. It wouldn't hurt to be set at build time, just wouldn't do anything.
options.BeforeSend = sentryEvent =>
{
if (sentryEvent.Tags.ContainsKey("SomeTag"))
Expand All @@ -21,5 +21,7 @@ public override void Configure(SentryUnityOptions options)

return sentryEvent;
};

Debug.Log("CustomOptionsConfiguration::Configure() finished");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ GraphicsSettings:
- {fileID: 16000, guid: 0000000000000000f000000000000000, type: 0}
- {fileID: 16001, guid: 0000000000000000f000000000000000, type: 0}
- {fileID: 17000, guid: 0000000000000000f000000000000000, type: 0}
- {fileID: 16003, guid: 0000000000000000f000000000000000, type: 0}
- {fileID: 16002, guid: 0000000000000000f000000000000000, type: 0}
m_PreloadedShaders: []
m_SpritesDefaultMaterial: {fileID: 10754, guid: 0000000000000000f000000000000000,
type: 0}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ PlayerSettings:
incrementalIl2cppBuild: {}
suppressCommonWarnings: 1
allowUnsafeCode: 0
additionalIl2CppArgs:
additionalIl2CppArgs: ' --emit-source-mapping'
scriptingRuntimeVersion: 1
gcIncremental: 0
assemblyVersionValidation: 1
Expand Down
5 changes: 2 additions & 3 deletions scripts/pack.ps1
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Remove-Item "package-release" -Recurse -ErrorAction SilentlyContinue
Remove-Item "package-release" -Recurse -ErrorAction SilentlyContinue
New-Item "package-release" -ItemType Directory

$exclude = @(
Expand All @@ -23,8 +23,7 @@ Copy-Item "LICENSE.md" -Destination "package-release/LICENSE.md"

# Copy samples
Copy-Item "samples/unity-of-bugs/Assets/Scenes" -Destination "package-release/Samples~/unity-of-bugs/Scenes" -Recurse
Copy-Item "samples/unity-of-bugs/Assets/Scripts" -Destination "package-release/Samples~/unity-of-bugs/Scripts" -Recurse `
-Exclude "SmokeTestOptions.cs*"
Copy-Item "samples/unity-of-bugs/Assets/Scripts" -Destination "package-release/Samples~/unity-of-bugs/Scripts" -Recurse

# Create zip
Compress-Archive "package-release/*" -DestinationPath "package-release.zip" -Force
20 changes: 13 additions & 7 deletions scripts/smoke-test-droid.ps1
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
param (
[Parameter(Position = 0)]
[Switch] $IsIntegrationTest
[Switch] $IsIntegrationTest,
[Switch] $WarnIfFlaky
)

. $PSScriptRoot/../test/Scripts.Integration.Test/common.ps1
Expand Down Expand Up @@ -216,7 +217,7 @@ function ExitNow([string] $status, [string] $message)
{
Write-Host $message -ForegroundColor Green
}
elseif ($status -ieq "flaky")
elseif ($status -ieq "flaky" -and $WarnIfFlaky)
{
Write-Warning $message
}
Expand Down Expand Up @@ -259,6 +260,8 @@ If (-not (Test-Path -Path "$ApkPath/$ApkFileName" ))
# Test
foreach ($device in $DeviceList)
{
adb -s $device logcat -c

$deviceApi = "$(adb -s $device shell getprop ro.build.version.sdk)".Trim()
$deviceSdk = "$(adb -s $device shell getprop ro.build.version.release)".Trim()
Write-Host "`nChecking device $device with SDK '$deviceSdk' and API '$deviceApi'"
Expand All @@ -283,7 +286,7 @@ foreach ($device in $DeviceList)
do
{
Write-Host "Installing test app..."
$stdout = (adb -s $device install -r $ApkPath/$ApkFileName)
$stdout = (adb -s $device install -r $ApkPath/$ApkFileName 2>&1)

if ($stdout.Contains("Broken pipe"))
{
Expand All @@ -299,12 +302,15 @@ foreach ($device in $DeviceList)
ExitNow "failed" "Failed to Install APK: $stdout."
}

function RunTest([string] $Name, [string] $SuccessString, [string] $FailureString)
function RunTest([string] $Name, [string] $SuccessString, [string] $FailureString, [switch] $PreserveLogcat)
{
Write-Host "::group::Test: '$name'"

Write-Host "Clearing logcat from $device."
adb -s $device logcat -c
if (!$PreserveLogcat)
{
Write-Host "Clearing logcat from $device."
adb -s $device logcat -c
}

adb -s $device shell am start -n $TestActivityName -e test $Name
#despite calling start, the app might not be started yet.
Expand Down Expand Up @@ -411,7 +417,7 @@ foreach ($device in $DeviceList)
# Note: mobile apps post the crash on the second app launch, so we must run both as part of the "CrashTestWithServer"
CrashTestWithServer -SuccessString "POST /api/12345/envelope/ HTTP/1.1`" 200 -b'1f8b08000000000000" -CrashTestCallback {
RunTest -Name "crash" -SuccessString "CRASH TEST: Issuing a native crash" -FailureString "CRASH TEST: FAIL"
RunTest -Name "has-crashed"
RunTest -Name "has-crashed" -PreserveLogcat
}
}
catch
Expand Down
20 changes: 9 additions & 11 deletions src/Sentry.Unity.Editor.iOS/BuildPostProcess.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.IO;
using Sentry.Extensibility;
using Sentry.Unity.Editor.ConfigurationWindow;
using UnityEditor;
using UnityEditor.Callbacks;

Expand All @@ -16,9 +17,7 @@ public static void OnPostProcessBuild(BuildTarget target, string pathToProject)
return;
}

var options = SentryScriptableObject
.Load<ScriptableSentryUnityOptions>(ScriptableSentryUnityOptions.GetConfigPath())
?.ToSentryUnityOptions(BuildPipeline.isBuildingPlayer);
var (options, cliOptions) = SentryScriptableObject.ConfiguredBuildtimeOptions();
var logger = options?.DiagnosticLogger ?? new UnityLogger(new SentryUnityOptions());

try
Expand All @@ -37,32 +36,31 @@ public static void OnPostProcessBuild(BuildTarget target, string pathToProject)

if (options is null)
{
logger.LogWarning("Native support disabled. " +
"Sentry has not been configured. You can do that through the editor: Tools -> Sentry");
logger.LogWarning("iOS native support disabled because Sentry has not been configured. " +
"You can do that through the editor: {0}", SentryWindow.EditorMenuPath);
return;
}

if (!options.IsValid())
{
logger.LogWarning("Native support disabled.");
logger.LogWarning("iOS native support disabled.");
return;
}

if (!options.IosNativeSupportEnabled)
{
logger.LogDebug("iOS Native support disabled through the options.");
logger.LogDebug("iOS native support disabled through the options.");
return;
}

sentryXcodeProject.AddNativeOptions(options);
sentryXcodeProject.AddSentryToMain(options);

var sentryCliOptions = SentryScriptableObject.CreateOrLoad<SentryCliOptions>(SentryCliOptions.GetConfigPath());
if (sentryCliOptions.IsValid(logger))
if (cliOptions?.IsValid(logger, EditorUserBuildSettings.development) is true)
{
SentryCli.CreateSentryProperties(pathToProject, sentryCliOptions, options);
SentryCli.CreateSentryProperties(pathToProject, cliOptions, options);
SentryCli.AddExecutableToXcodeProject(pathToProject, logger);
sentryXcodeProject.AddBuildPhaseSymbolUpload(logger, sentryCliOptions);
sentryXcodeProject.AddBuildPhaseSymbolUpload(logger, cliOptions);
}
else if (options.Il2CppLineNumberSupportEnabled)
{
Expand Down
19 changes: 8 additions & 11 deletions src/Sentry.Unity.Editor/Android/AndroidManifestConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Text;
using System.Xml;
using Sentry.Extensibility;
using Sentry.Unity.Editor.ConfigurationWindow;
using UnityEditor;
using UnityEditor.Android;
using UnityEngine;
Expand All @@ -25,24 +26,20 @@ public class AndroidManifestConfiguration : IPostGenerateGradleAndroidProject

public AndroidManifestConfiguration()
: this(
() => SentryScriptableObject.Load<ScriptableSentryUnityOptions>(ScriptableSentryUnityOptions.GetConfigPath())
?.ToSentryUnityOptions(BuildPipeline.isBuildingPlayer),
() => SentryScriptableObject.Load<SentryCliOptions>(SentryCliOptions.GetConfigPath()),
() => SentryScriptableObject.ConfiguredBuildtimeOptions(),
vaind marked this conversation as resolved.
Show resolved Hide resolved
isDevelopmentBuild: EditorUserBuildSettings.development,
scriptingImplementation: PlayerSettings.GetScriptingBackend(BuildTargetGroup.Android))
{
}

// Testing
internal AndroidManifestConfiguration(
Func<SentryUnityOptions?> getOptions,
Func<SentryCliOptions?> getSentryCliOptions,
Func<(SentryUnityOptions?, SentryCliOptions?)> getOptions,
bool isDevelopmentBuild,
ScriptingImplementation scriptingImplementation,
IUnityLoggerInterceptor? interceptor = null)
{
_options = getOptions();
_sentryCliOptions = getSentryCliOptions();
(_options, _sentryCliOptions) = getOptions();
_logger = _options?.DiagnosticLogger ?? new UnityLogger(_options ?? new SentryUnityOptions(), interceptor);

_isDevelopmentBuild = isDevelopmentBuild;
Expand Down Expand Up @@ -71,18 +68,18 @@ internal void ModifyManifest(string basePath)
var disableAutoInit = false;
if (_options is null)
{
_logger.LogWarning("Android Native support disabled. " +
"Sentry has not been configured. You can do that through the editor: Tools -> Sentry");
_logger.LogWarning("Android native support disabled because Sentry has not been configured. " +
"You can do that through the editor: {0}", SentryWindow.EditorMenuPath);
disableAutoInit = true;
}
else if (!_options.IsValid())
{
_logger.LogDebug("Native support disabled.");
_logger.LogDebug("Android native support disabled.");
disableAutoInit = true;
}
else if (!_options.AndroidNativeSupportEnabled)
{
_logger.LogDebug("Android Native support disabled through the options.");
_logger.LogDebug("Android native support disabled through the options.");
disableAutoInit = true;
}

Expand Down
Loading