Skip to content

Commit 59f8637

Browse files
authored
fix: SDK refrains from setting tags automatically. (#2459)
1 parent 3cbbf93 commit 59f8637

File tree

7 files changed

+29
-102
lines changed

7 files changed

+29
-102
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Behavioural Changes
6+
7+
- The SDK no longer sets tags automatically. The promotion of contexts to tags now happens exclusively in Sentry. The `Unity` context got extended by `IsMainThread` for this. The tags `unity.device.unique_identifier` and `unity.gpu.supports_instancing` no longer exist and have no replacement. ([#2459](https://github.com/getsentry/sentry-unity/pull/2459))
8+
59
### Breaking Changes
610

711
- Renamed the option `AttachBreadcrumbsToEvents` to `AddBreadcrumbsWithStructuredLogs` to better reflect its purpose. The option controls if the SDK does BOTH: attach logs as breadcrumbs to events and send them as structured logs ([#2455](https://github.com/getsentry/sentry-unity/pull/2455))

src/Sentry.Unity/Integrations/UnityScopeIntegration.cs

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ public void ConfigureScope(Scope scope)
5656
PopulateUnity(unity);
5757
scope.Contexts.Add(Protocol.Unity.Type, unity);
5858

59-
PopulateTags(scope.SetTag);
6059
PopulateUser(scope);
6160
}
6261

@@ -158,30 +157,6 @@ private void PopulateUnity(Protocol.Unity unity)
158157
unity.RenderingThreadingMode = MainThreadData.RenderingThreadingMode;
159158
}
160159

161-
private void PopulateTags(Action<string, string> setTag)
162-
{
163-
// TODO revisit which tags we should be adding by default
164-
if (MainThreadData.InstallMode is { } installMode)
165-
{
166-
setTag("unity.install_mode", installMode);
167-
}
168-
169-
if (MainThreadData.SupportsDrawCallInstancing.HasValue)
170-
{
171-
setTag("unity.gpu.supports_instancing", MainThreadData.SupportsDrawCallInstancing.Value.ToTagValue());
172-
}
173-
174-
if (MainThreadData.DeviceType is { } deviceType)
175-
{
176-
setTag("unity.device.device_type", deviceType);
177-
}
178-
179-
if (_options.SendDefaultPii && MainThreadData.DeviceUniqueIdentifier is { } deviceUniqueIdentifier)
180-
{
181-
setTag("unity.device.unique_identifier", deviceUniqueIdentifier);
182-
}
183-
}
184-
185160
private void PopulateUser(Scope scope)
186161
{
187162
if (_options.DefaultUserId is not null)

src/Sentry.Unity/Protocol/Unity.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public sealed class Unity : ISentryJsonSerializable
1414
public const string Type = "unity";
1515

1616
/// <summary>
17-
/// The Unity editor version
17+
/// The Unity editor version.
1818
/// </summary>
1919
/// <example>
2020
/// 2019.4.40f1
@@ -29,6 +29,11 @@ public sealed class Unity : ISentryJsonSerializable
2929
/// </example>
3030
public string? InstallMode { get; set; }
3131

32+
/// <summary>
33+
/// Whether this was happening on the main thread or not.
34+
/// </summary>
35+
public bool? IsMainThread { get; set; }
36+
3237
/// <summary>
3338
/// Support for various copy texture cases.
3439
/// </summary>
@@ -53,14 +58,15 @@ public sealed class Unity : ISentryJsonSerializable
5358
public string? TargetFrameRate { get; set; }
5459

5560
/// <summary>
56-
/// The active scene's name
61+
/// The active scene's name.
5762
/// </summary>
5863
public string? ActiveSceneName { get; set; }
5964

6065
internal Unity Clone()
6166
=> new()
6267
{
6368
InstallMode = InstallMode,
69+
IsMainThread = IsMainThread,
6470
CopyTextureSupport = CopyTextureSupport,
6571
RenderingThreadingMode = RenderingThreadingMode,
6672
TargetFrameRate = TargetFrameRate,
@@ -83,6 +89,11 @@ public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger)
8389
writer.WriteString("install_mode", InstallMode);
8490
}
8591

92+
if (IsMainThread is not null)
93+
{
94+
writer.WriteBoolean("is_main_thread", IsMainThread.Value);
95+
}
96+
8697
if (!string.IsNullOrWhiteSpace(CopyTextureSupport))
8798
{
8899
writer.WriteString("copy_texture_support", CopyTextureSupport);
@@ -111,6 +122,7 @@ public static Unity FromJson(JsonElement json)
111122
{
112123
EditorVersion = json.GetPropertyOrNull("editor_version")?.GetString(),
113124
InstallMode = json.GetPropertyOrNull("install_mode")?.GetString(),
125+
IsMainThread = json.GetPropertyOrNull("is_main_thread")?.GetBoolean(),
114126
CopyTextureSupport = json.GetPropertyOrNull("copy_texture_support")?.GetString(),
115127
RenderingThreadingMode = json.GetPropertyOrNull("rendering_threading_mode")?.GetString(),
116128
TargetFrameRate = json.GetPropertyOrNull("target_frame_rate")?.GetString(),

src/Sentry.Unity/UnityEventProcessor.cs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ private void SetEventContext(IEventLike sentryEvent)
5959
// Populating the SDK Integrations here (for now) instead of UnityScopeIntegration because it cannot be guaranteed
6060
// that it got added last or that there was not an integration added at a later point
6161
PopulateSdkIntegrations(sentryEvent.Sdk);
62-
// TODO revisit which tags we should be adding by default
63-
sentryEvent.SetTag("unity.is_main_thread", MainThreadData.IsMainThread().ToTagValue());
6462
}
6563
catch (Exception exception)
6664
{
@@ -117,6 +115,8 @@ private void PopulateDevice(Device device)
117115

118116
private void PopulateUnity(Protocol.Unity unity)
119117
{
118+
unity.IsMainThread = MainThreadData.IsMainThread();
119+
120120
if (!MainThreadData.IsMainThread())
121121
{
122122
return;
@@ -137,8 +137,3 @@ private void PopulateSdkIntegrations(SdkVersion sdkVersion)
137137
}
138138
}
139139
}
140-
141-
internal static class TagValueNormalizer
142-
{
143-
internal static string ToTagValue(this bool value) => value ? "true" : "false";
144-
}

test/Sentry.Unity.Tests/IntegrationTests.cs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ public IEnumerator DebugLogError_OnMainThread_IsCapturedAndIsMainThreadIsTrue()
236236

237237
var triggeredEvent = _testHttpClientHandler.GetEvent(_identifyingEventValueAttribute, _eventReceiveTimeout);
238238
Assert.That(triggeredEvent, Does.Contain(_identifyingEventValueAttribute));
239-
Assert.That(triggeredEvent, Does.Contain("unity.is_main_thread\":\"true\""));
239+
Assert.That(triggeredEvent, Does.Contain("is_main_thread\":true"));
240240
}
241241

242242
[UnityTest]
@@ -259,24 +259,22 @@ public IEnumerator DebugLogError_InTask_IsCapturedAndIsMainThreadIsFalse()
259259

260260
var triggeredEvent = _testHttpClientHandler.GetEvent(_identifyingEventValueAttribute, _eventReceiveTimeout);
261261
Assert.That(triggeredEvent, Does.Contain(_identifyingEventValueAttribute));
262-
Assert.That(triggeredEvent, Does.Contain("unity.is_main_thread\":\"false\""));
262+
Assert.That(triggeredEvent, Does.Contain("is_main_thread\":false"));
263263
}
264264

265265
[UnityTest]
266266
public IEnumerator DebugLogException_OnMainThread_IsCapturedAndIsMainThreadIsTrue()
267267
{
268268
yield return SetupSceneCoroutine("1_BugFarm");
269269

270-
var expectedAttribute = CreateAttribute("unity.is_main_thread", "true");
271-
272270
using var _ = InitSentrySdk();
273271
var testBehaviour = new GameObject("TestHolder").AddComponent<TestMonoBehaviour>();
274272

275273
testBehaviour.gameObject.SendMessage(nameof(testBehaviour.DebugLogException), _eventMessage);
276274

277275
var triggeredEvent = _testHttpClientHandler.GetEvent(_identifyingEventValueAttribute, _eventReceiveTimeout);
278276
Assert.That(triggeredEvent, Does.Contain(_identifyingEventValueAttribute));
279-
Assert.That(triggeredEvent, Does.Contain(expectedAttribute));
277+
Assert.That(triggeredEvent, Does.Contain("is_main_thread\":true"));
280278
}
281279

282280
[UnityTest]
@@ -289,16 +287,14 @@ public IEnumerator DebugLogException_InTask_IsCapturedAndIsMainThreadIsFalse()
289287

290288
yield return SetupSceneCoroutine("1_BugFarm");
291289

292-
var expectedAttribute = CreateAttribute("unity.is_main_thread", "false");
293-
294290
using var _ = InitSentrySdk();
295291
var testBehaviour = new GameObject("TestHolder").AddComponent<TestMonoBehaviour>();
296292

297293
testBehaviour.gameObject.SendMessage(nameof(testBehaviour.DebugLogExceptionInTask), _eventMessage);
298294

299295
var triggeredEvent = _testHttpClientHandler.GetEvent(_identifyingEventValueAttribute, _eventReceiveTimeout);
300296
Assert.That(triggeredEvent, Does.Contain(_identifyingEventValueAttribute));
301-
Assert.That(triggeredEvent, Does.Contain(expectedAttribute));
297+
Assert.That(triggeredEvent, Does.Contain("is_main_thread\":false"));
302298
}
303299

304300
[UnityTest]

test/Sentry.Unity.Tests/Protocol/UnityTests.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
2020
var sut = new Unity.Protocol.Unity
2121
{
2222
InstallMode = "DeveloperBuild",
23+
IsMainThread = false,
2324
CopyTextureSupport = "Copy3D",
2425
RenderingThreadingMode = "MultiThreaded",
2526
TargetFrameRate = "30",
@@ -31,6 +32,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
3132
Assert.AreEqual(
3233
"{\"type\":\"unity\"," +
3334
"\"install_mode\":\"DeveloperBuild\"," +
35+
"\"is_main_thread\":false," +
3436
"\"copy_texture_support\":\"Copy3D\"," +
3537
"\"rendering_threading_mode\":\"MultiThreaded\"," +
3638
"\"target_frame_rate\":\"30\"," +
@@ -44,6 +46,7 @@ public void Clone_CopyValues()
4446
var sut = new Unity.Protocol.Unity
4547
{
4648
InstallMode = "DeveloperBuild",
49+
IsMainThread = false,
4750
CopyTextureSupport = "Copy3D",
4851
RenderingThreadingMode = "MultiThreaded",
4952
TargetFrameRate = "30",
@@ -53,6 +56,7 @@ public void Clone_CopyValues()
5356
var clone = sut.Clone();
5457

5558
Assert.AreEqual(sut.InstallMode, clone.InstallMode);
59+
Assert.AreEqual(sut.IsMainThread, clone.IsMainThread);
5660
Assert.AreEqual(sut.CopyTextureSupport, clone.CopyTextureSupport);
5761
Assert.AreEqual(sut.RenderingThreadingMode, clone.RenderingThreadingMode);
5862
Assert.AreEqual(sut.TargetFrameRate, clone.TargetFrameRate);
@@ -71,6 +75,7 @@ public void SerializeObject_TestCase_SerializesAsExpected((Unity.Protocol.Unity
7175
{
7276
new object[] { (new Unity.Protocol.Unity(), "{\"type\":\"unity\"}") },
7377
new object[] { (new Unity.Protocol.Unity { InstallMode = "Adhoc" }, "{\"type\":\"unity\",\"install_mode\":\"Adhoc\"}") },
78+
new object[] { (new Unity.Protocol.Unity { IsMainThread = false }, "{\"type\":\"unity\",\"is_main_thread\":false}") },
7479
new object[] { (new Unity.Protocol.Unity { CopyTextureSupport = "TextureToRT" }, "{\"type\":\"unity\",\"copy_texture_support\":\"TextureToRT\"}") },
7580
new object[] { (new Unity.Protocol.Unity { RenderingThreadingMode = "NativeGraphicsJobs" }, "{\"type\":\"unity\",\"rendering_threading_mode\":\"NativeGraphicsJobs\"}") },
7681
new object[] { (new Unity.Protocol.Unity { TargetFrameRate = "30" }, "{\"type\":\"unity\",\"target_frame_rate\":\"30\"}") },

test/Sentry.Unity.Tests/UnityEventScopeTests.cs

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -354,66 +354,6 @@ public void UserId_UnchangedIfNonEmpty()
354354
Assert.AreEqual(scope.User.Id, "bar");
355355
}
356356

357-
[Test]
358-
public void Tags_Set()
359-
{
360-
// arrange
361-
var systemInfo = new TestSentrySystemInfo
362-
{
363-
SupportsDrawCallInstancing = true,
364-
DeviceType = new(() => "test type"),
365-
DeviceUniqueIdentifier = new(() => "f810306c-68db-4ebe-89ba-13c457449339"),
366-
InstallMode = ApplicationInstallMode.Store.ToString()
367-
};
368-
MainThreadData.SentrySystemInfo = systemInfo;
369-
MainThreadData.CollectData();
370-
371-
var sentryOptions = new SentryUnityOptions { SendDefaultPii = true };
372-
var scopeUpdater = new UnityScopeUpdater(sentryOptions, _testApplication);
373-
var unityInfo = new TestUnityInfo { IL2CPP = true };
374-
var unityEventProcessor = new UnityEventProcessor(sentryOptions, unityInfo);
375-
var scope = new Scope(sentryOptions);
376-
var sentryEvent = new SentryEvent();
377-
var transaction = new SentryTransaction("name", "operation");
378-
379-
// act
380-
scopeUpdater.ConfigureScope(scope);
381-
scope.Apply(sentryEvent);
382-
scope.Apply(transaction);
383-
unityEventProcessor.Process(sentryEvent);
384-
unityEventProcessor.Process(transaction);
385-
386-
// assert
387-
AssertEventProcessorTags(systemInfo, sentryEvent.Tags);
388-
AssertEventProcessorTags(systemInfo, transaction.Tags);
389-
}
390-
391-
private void AssertEventProcessorTags(ISentrySystemInfo systemInfo, IReadOnlyDictionary<string, string> tags)
392-
{
393-
Assert.IsNotNull(tags);
394-
Assert.NotZero(tags.Count);
395-
396-
var unityInstallMode = tags.SingleOrDefault(t => t.Key == "unity.install_mode");
397-
Assert.NotNull(unityInstallMode);
398-
Assert.AreEqual(systemInfo.InstallMode, unityInstallMode.Value);
399-
400-
var supportsInstancing = tags.SingleOrDefault(t => t.Key == "unity.gpu.supports_instancing");
401-
Assert.NotNull(supportsInstancing);
402-
Assert.AreEqual(systemInfo.SupportsDrawCallInstancing, bool.Parse(supportsInstancing.Value));
403-
404-
var deviceType = tags.SingleOrDefault(t => t.Key == "unity.device.device_type");
405-
Assert.NotNull(deviceType);
406-
Assert.AreEqual(systemInfo.DeviceType!.Value, deviceType.Value);
407-
408-
var deviceUniqueIdentifier = tags.SingleOrDefault(t => t.Key == "unity.device.unique_identifier");
409-
Assert.NotNull(deviceUniqueIdentifier);
410-
Assert.AreEqual(systemInfo.DeviceUniqueIdentifier!.Value, deviceUniqueIdentifier.Value);
411-
412-
var isMainThread = tags.SingleOrDefault(t => t.Key == "unity.is_main_thread");
413-
Assert.NotNull(isMainThread);
414-
Assert.AreEqual("true", isMainThread.Value);
415-
}
416-
417357
[Test]
418358
public void OperatingSystemProtocol_Assigned()
419359
{

0 commit comments

Comments
 (0)