diff --git a/CHANGELOG.md b/CHANGELOG.md index 24cdd16f3d..6e9de0ad41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Unable to load DLL sentry-native or one of its dependencies - On mobile devices, the SDK no longer throws a `FormatException` when trying to report native events ([#3485](https://github.com/getsentry/sentry-dotnet/pull/3485)) - Race condition in `SentryMessageHandler` ([#3477](https://github.com/getsentry/sentry-dotnet/pull/3477)) - Decrease runtime diagnostics circular buffer when profiling, reducing memory usage ([#3491](https://github.com/getsentry/sentry-dotnet/pull/3491)) +- The InstallationId is now resolved only once per application execution and any issues are logged as warnings instead of errors ([#3529](https://github.com/getsentry/sentry-dotnet/pull/3529)) - DisplayInfo now captured correctly on iOS and Mac Catalyst on non-UI threads ([#3521](https://github.com/getsentry/sentry-dotnet/pull/3521)) ### Dependencies diff --git a/src/Sentry/InstallationIdHelper.cs b/src/Sentry/Internal/InstallationIdHelper.cs similarity index 73% rename from src/Sentry/InstallationIdHelper.cs rename to src/Sentry/Internal/InstallationIdHelper.cs index 70b0ff9435..c3159d8d93 100644 --- a/src/Sentry/InstallationIdHelper.cs +++ b/src/Sentry/Internal/InstallationIdHelper.cs @@ -1,20 +1,12 @@ using Sentry.Extensibility; using Sentry.Internal.Extensions; -namespace Sentry; +namespace Sentry.Internal; -internal class InstallationIdHelper +internal class InstallationIdHelper(SentryOptions options) { private readonly object _installationIdLock = new(); private string? _installationId; - private readonly SentryOptions _options; - private readonly string? _persistenceDirectoryPath; - - public InstallationIdHelper(SentryOptions options) - { - _options = options; - _persistenceDirectoryPath = options.CacheDirectoryPath ?? options.TryGetDsnSpecificCacheDirectoryPath(); - } public string? TryGetInstallationId() { @@ -42,11 +34,11 @@ public InstallationIdHelper(SentryOptions options) if (!string.IsNullOrWhiteSpace(id)) { - _options.LogDebug("Resolved installation ID '{0}'.", id); + options.LogDebug("Resolved installation ID '{0}'.", id); } else { - _options.LogDebug("Failed to resolve installation ID."); + options.LogDebug("Failed to resolve installation ID."); } return _installationId = id; @@ -57,13 +49,13 @@ public InstallationIdHelper(SentryOptions options) { try { - var rootPath = _persistenceDirectoryPath ?? + var rootPath = options.CacheDirectoryPath ?? Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); - var directoryPath = Path.Combine(rootPath, "Sentry", _options.Dsn!.GetHashString()); + var directoryPath = Path.Combine(rootPath, "Sentry", options.Dsn!.GetHashString()); Directory.CreateDirectory(directoryPath); - _options.LogDebug("Created directory for installation ID file ({0}).", directoryPath); + options.LogDebug("Created directory for installation ID file ({0}).", directoryPath); var filePath = Path.Combine(directoryPath, ".installation"); @@ -72,20 +64,20 @@ public InstallationIdHelper(SentryOptions options) { return File.ReadAllText(filePath); } - _options.LogDebug("File containing installation ID does not exist ({0}).", filePath); + options.LogDebug("File containing installation ID does not exist ({0}).", filePath); // Generate new installation ID and store it in a file var id = Guid.NewGuid().ToString(); File.WriteAllText(filePath, id); - _options.LogDebug("Saved installation ID '{0}' to file '{1}'.", id, filePath); + options.LogDebug("Saved installation ID '{0}' to file '{1}'.", id, filePath); return id; } // If there's no write permission or the platform doesn't support this, we handle // and let the next installation id strategy kick in catch (Exception ex) { - _options.LogError(ex, "Failed to resolve persistent installation ID."); + options.LogError(ex, "Failed to resolve persistent installation ID."); return null; } } @@ -105,7 +97,7 @@ public InstallationIdHelper(SentryOptions options) if (string.IsNullOrWhiteSpace(installationId)) { - _options.LogError("Failed to find an appropriate network interface for installation ID."); + options.LogError("Failed to find an appropriate network interface for installation ID."); return null; } @@ -113,7 +105,7 @@ public InstallationIdHelper(SentryOptions options) } catch (Exception ex) { - _options.LogError(ex, "Failed to resolve hardware installation ID."); + options.LogError(ex, "Failed to resolve hardware installation ID."); return null; } } diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index c69cb62acf..d354d253e3 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -41,9 +41,8 @@ public class SentryOptions /// internal IScopeStackContainer? ScopeStackContainer { get; set; } - private Lazy LazyInstallationId => new(() - => new InstallationIdHelper(this).TryGetInstallationId()); - internal string? InstallationId => LazyInstallationId.Value; + private readonly Lazy _lazyInstallationId; + internal string? InstallationId => _lazyInstallationId.Value; #if __MOBILE__ private bool _isGlobalModeEnabled = true; @@ -1182,6 +1181,7 @@ public bool JsonPreserveReferences public SentryOptions() { SettingLocator = new SettingLocator(this); + _lazyInstallationId = new(() => new InstallationIdHelper(this).TryGetInstallationId()); TransactionProcessorsProviders = new() { () => TransactionProcessors ?? Enumerable.Empty() diff --git a/test/Sentry.Tests/Internals/InstallationIdHelperTests.cs b/test/Sentry.Tests/Internals/InstallationIdHelperTests.cs index dbe2c81739..8162aa8ca3 100644 --- a/test/Sentry.Tests/Internals/InstallationIdHelperTests.cs +++ b/test/Sentry.Tests/Internals/InstallationIdHelperTests.cs @@ -6,13 +6,13 @@ private class Fixture : IDisposable { private readonly TempDirectory _cacheDirectory; - public InMemoryDiagnosticLogger Logger { get; } + public IDiagnosticLogger Logger { get; } public SentryOptions Options { get; } public Fixture(Action configureOptions = null) { - Logger = new InMemoryDiagnosticLogger(); + Logger = Substitute.For(); var fileSystem = new FakeFileSystem(); _cacheDirectory = new TempDirectory(fileSystem); @@ -40,9 +40,6 @@ public Fixture(Action configureOptions = null) [Fact] public void GetMachineNameInstallationId_Hashed() { - // Arrange - var sut = _fixture.GetSut(); - // Act var installationId = InstallationIdHelper.GetMachineNameInstallationId(); @@ -54,9 +51,6 @@ public void GetMachineNameInstallationId_Hashed() [Fact] public void GetMachineNameInstallationId_Idempotent() { - // Arrange - var sut = _fixture.GetSut(); - // Act var installationIds = Enumerable .Range(0, 10) @@ -66,4 +60,29 @@ public void GetMachineNameInstallationId_Idempotent() // Assert installationIds.Distinct().Should().ContainSingle(); } + + [Fact] + public void TryGetInstallationId_CachesInstallationId() + { + // Arrange + _fixture.Logger.IsEnabled(Arg.Any()).Returns(true); + var installationIdHelper = _fixture.GetSut(); + + // Act + var installationId1 = installationIdHelper.TryGetInstallationId(); + + // Assert + installationId1.Should().NotBeNullOrWhiteSpace(); + _fixture.Logger.Received(1).Log(SentryLevel.Debug, "Resolved installation ID '{0}'.", null, Arg.Any()); + + // Arrange + _fixture.Logger.ClearReceivedCalls(); + + // Act + var installationId2 = installationIdHelper.TryGetInstallationId(); + + // Assert + installationId2.Should().Be(installationId1); + _fixture.Logger.Received(0).Log(SentryLevel.Debug, "Resolved installation ID '{0}'.", null, Arg.Any()); + } } diff --git a/test/Sentry.Tests/SentryOptionsTests.cs b/test/Sentry.Tests/SentryOptionsTests.cs index 2c1167a819..1f5789f45d 100644 --- a/test/Sentry.Tests/SentryOptionsTests.cs +++ b/test/Sentry.Tests/SentryOptionsTests.cs @@ -738,4 +738,34 @@ public void Integrations_Includes_MajorSystemPrefixes(string expected) var sut = new SentryOptions(); Assert.Contains(sut.InAppExclude!, e => e.ToString() == expected); } + + [Fact] + public void CachesInstallationId() + { + // Arrange + var logger = Substitute.For(); + logger.IsEnabled(Arg.Any()).Returns(true); + var sut = new SentryOptions + { + Debug = true, + DiagnosticLogger = logger + }; + + // Act + var installationId1 = sut.InstallationId; + + // Assert + installationId1.Should().NotBeNullOrWhiteSpace(); + logger.Received(1).Log(SentryLevel.Debug, "Resolved installation ID '{0}'.", null, Arg.Any()); + + // Arrange + logger.ClearReceivedCalls(); + + // Act + var installationId2 = sut.InstallationId; + + // Assert + installationId2.Should().Be(installationId1); + logger.Received(0).Log(SentryLevel.Debug, "Resolved installation ID '{0}'.", null, Arg.Any()); + } }