Skip to content

Commit

Permalink
Fix: InstallationId now gets evaluated only once per application exec…
Browse files Browse the repository at this point in the history
…ution (#3529)
  • Loading branch information
jamescrosswell committed Aug 13, 2024
1 parent 5d00295 commit b2d6162
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
{
Expand Down Expand Up @@ -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;
Expand All @@ -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");

Expand All @@ -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;
}
}
Expand All @@ -105,15 +97,15 @@ 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;
}

return installationId;
}
catch (Exception ex)
{
_options.LogError(ex, "Failed to resolve hardware installation ID.");
options.LogError(ex, "Failed to resolve hardware installation ID.");
return null;
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/Sentry/SentryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ public class SentryOptions
/// </remarks>
internal IScopeStackContainer? ScopeStackContainer { get; set; }

private Lazy<string?> LazyInstallationId => new(()
=> new InstallationIdHelper(this).TryGetInstallationId());
internal string? InstallationId => LazyInstallationId.Value;
private readonly Lazy<string?> _lazyInstallationId;
internal string? InstallationId => _lazyInstallationId.Value;

#if __MOBILE__
private bool _isGlobalModeEnabled = true;
Expand Down Expand Up @@ -1182,6 +1181,7 @@ public bool JsonPreserveReferences
public SentryOptions()
{
SettingLocator = new SettingLocator(this);
_lazyInstallationId = new(() => new InstallationIdHelper(this).TryGetInstallationId());

TransactionProcessorsProviders = new() {
() => TransactionProcessors ?? Enumerable.Empty<ISentryTransactionProcessor>()
Expand Down
35 changes: 27 additions & 8 deletions test/Sentry.Tests/Internals/InstallationIdHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<SentryOptions> configureOptions = null)
{
Logger = new InMemoryDiagnosticLogger();
Logger = Substitute.For<IDiagnosticLogger>();

var fileSystem = new FakeFileSystem();
_cacheDirectory = new TempDirectory(fileSystem);
Expand Down Expand Up @@ -40,9 +40,6 @@ public Fixture(Action<SentryOptions> configureOptions = null)
[Fact]
public void GetMachineNameInstallationId_Hashed()
{
// Arrange
var sut = _fixture.GetSut();

// Act
var installationId = InstallationIdHelper.GetMachineNameInstallationId();

Expand All @@ -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)
Expand All @@ -66,4 +60,29 @@ public void GetMachineNameInstallationId_Idempotent()
// Assert
installationIds.Distinct().Should().ContainSingle();
}

[Fact]
public void TryGetInstallationId_CachesInstallationId()
{
// Arrange
_fixture.Logger.IsEnabled(Arg.Any<SentryLevel>()).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<string>());

// 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<string>());
}
}
30 changes: 30 additions & 0 deletions test/Sentry.Tests/SentryOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IDiagnosticLogger>();
logger.IsEnabled(Arg.Any<SentryLevel>()).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<string>());

// 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<string>());
}
}

0 comments on commit b2d6162

Please sign in to comment.