diff --git a/CHANGELOG.md b/CHANGELOG.md index 08dd24178d..055ed0a59a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ ### Features ✨ - feat: Add exponential backoff and log deduplication to Spotlight transport by @mattico in [#5025](https://github.com/getsentry/sentry-dotnet/pull/5025) +- feat: The `Environment` set on the `Scope` now gets synchronized to the native layers (`sentry-cocoa` and `sentry-native`) by @bitsandfoxes [#5365](https://github.com/getsentry/sentry-dotnet/pull/5365) + +### Fixes + +- fix: When setting `Environment` and `Release` on the `Scope` these are now also getting applied on emitted Metrics and Logs by bitsandfoxes [#5376](https://github.com/getsentry/sentry-dotnet/pull/5376) ## 6.6.0 diff --git a/src/Sentry/IScopeObserver.cs b/src/Sentry/IScopeObserver.cs index 870a160a30..3d9dcdce90 100644 --- a/src/Sentry/IScopeObserver.cs +++ b/src/Sentry/IScopeObserver.cs @@ -35,6 +35,11 @@ public interface IScopeObserver /// public void SetTrace(SentryId traceId, SpanId parentSpanId); + /// + /// Sets the environment. + /// + public void SetEnvironment(string? environment); + /// /// Adds an attachment. /// diff --git a/src/Sentry/Internal/ScopeObserver.cs b/src/Sentry/Internal/ScopeObserver.cs index 28f4676f84..0ba44c98a5 100644 --- a/src/Sentry/Internal/ScopeObserver.cs +++ b/src/Sentry/Internal/ScopeObserver.cs @@ -95,6 +95,15 @@ public void SetTrace(SentryId traceId, SpanId parentSpanId) public abstract void SetTraceImpl(SentryId traceId, SpanId parentSpanId); + public void SetEnvironment(string? environment) + { + _options.DiagnosticLogger?.Log(SentryLevel.Debug, + "{0} Scope Sync - Setting Environment e:\"{1}\"", null, _name, environment); + SetEnvironmentImpl(environment); + } + + public abstract void SetEnvironmentImpl(string? environment); + public void AddAttachment(SentryAttachment attachment) { _options.DiagnosticLogger?.Log(SentryLevel.Debug, diff --git a/src/Sentry/Platforms/Android/AndroidScopeObserver.cs b/src/Sentry/Platforms/Android/AndroidScopeObserver.cs index e18b215562..c4d3e91640 100644 --- a/src/Sentry/Platforms/Android/AndroidScopeObserver.cs +++ b/src/Sentry/Platforms/Android/AndroidScopeObserver.cs @@ -112,6 +112,18 @@ public void SetTrace(SentryId traceId, SpanId parentSpanId) } } + public void SetEnvironment(string? environment) + { + try + { + // TODO: Missing corresponding scope-level functionality on the Android SDK + } + finally + { + _innerObserver?.SetEnvironment(environment); + } + } + public void AddAttachment(SentryAttachment attachment) { try diff --git a/src/Sentry/Platforms/Cocoa/CocoaScopeObserver.cs b/src/Sentry/Platforms/Cocoa/CocoaScopeObserver.cs index f92156a4be..46f8aebf5e 100644 --- a/src/Sentry/Platforms/Cocoa/CocoaScopeObserver.cs +++ b/src/Sentry/Platforms/Cocoa/CocoaScopeObserver.cs @@ -120,6 +120,18 @@ public void SetTrace(SentryId traceId, SpanId parentSpanId) } } + public void SetEnvironment(string? environment) + { + try + { + SentryCocoaSdk.ConfigureScope(scope => scope.SetEnvironment(environment)); + } + finally + { + _innerObserver?.SetEnvironment(environment); + } + } + public void AddAttachment(SentryAttachment attachment) { try diff --git a/src/Sentry/Platforms/Native/CFunctions.cs b/src/Sentry/Platforms/Native/CFunctions.cs index 5faa149720..72105155e8 100644 --- a/src/Sentry/Platforms/Native/CFunctions.cs +++ b/src/Sentry/Platforms/Native/CFunctions.cs @@ -251,6 +251,9 @@ internal static string GetCacheDirectory(SentryOptions options) [DllImport("sentry-native")] internal static extern void sentry_set_trace(string traceId, string parentSpanId); + [DllImport("sentry-native")] + internal static extern void sentry_set_environment(string? environment); + internal static Dictionary LoadDebugImages(IDiagnosticLogger? logger) { // It only makes sense to load them once because they're cached on the native side anyway. We could force diff --git a/src/Sentry/Platforms/Native/NativeScopeObserver.cs b/src/Sentry/Platforms/Native/NativeScopeObserver.cs index f9034df998..1d6956a9f4 100644 --- a/src/Sentry/Platforms/Native/NativeScopeObserver.cs +++ b/src/Sentry/Platforms/Native/NativeScopeObserver.cs @@ -43,6 +43,9 @@ public override void SetUserImpl(SentryUser user) public override void SetTraceImpl(SentryId traceId, SpanId parentSpanId) => C.sentry_set_trace(traceId.ToString(), parentSpanId.ToString()); + public override void SetEnvironmentImpl(string? environment) => + C.sentry_set_environment(environment); + public override void AddAttachmentImpl(SentryAttachment attachment) { // TODO: Missing corresponding functionality on the Native SDK diff --git a/src/Sentry/Protocol/SentryAttributes.cs b/src/Sentry/Protocol/SentryAttributes.cs index 711860e480..71b967b3d1 100644 --- a/src/Sentry/Protocol/SentryAttributes.cs +++ b/src/Sentry/Protocol/SentryAttributes.cs @@ -97,17 +97,18 @@ internal void SetAttribute(string key, int value) this[key] = new SentryAttribute(value, "integer"); } - internal void SetDefaultAttributes(SentryOptions options, SdkVersion sdk) + internal void SetDefaultAttributes(SentryOptions options, Scope? scope, SdkVersion? sdk = null) { - var environment = options.SettingLocator.GetEnvironment(); + var environment = scope?.Environment ?? options.SettingLocator.GetEnvironment(); SetAttribute("sentry.environment", environment); - var release = options.SettingLocator.GetRelease(); + var release = scope?.Release ?? options.SettingLocator.GetRelease(); if (release is not null) { SetAttribute("sentry.release", release); } + sdk ??= scope?.Sdk ?? SdkVersion.Instance; if (sdk.Name is { } name) { SetAttribute("sentry.sdk.name", name); diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 62b926cb7f..86bea1754f 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -144,8 +144,35 @@ public SentryUser User /// public string? Distribution { get; set; } + private string? _environment; + /// - public string? Environment { get; set; } + public string? Environment + { + get => _environment; + set + { + if (_environment == value) + { + return; + } + + if (value is null) + { + Options.LogWarning("Environment cannot be null. Reverting to default value from the options."); + _environment = Options.Environment; + } + else + { + _environment = value; + } + + if (Options is { EnableScopeSync: true, ScopeObserver: { } observer }) + { + observer.SetEnvironment(_environment); + } + } + } // TransactionName is kept for legacy purposes because // SentryEvent still makes use of it. @@ -288,6 +315,10 @@ internal Scope(SentryOptions? options, SentryPropagationContext? propagationCont { Options = options ?? new SentryOptions(); PropagationContext = new SentryPropagationContext(propagationContext); + + // Skip scope sync with backing field. The native SDKs already received the environment set on the options. + _environment = Options.Environment; + Release = Options.Release; } // For testing. Should explicitly require SentryOptions. @@ -404,9 +435,9 @@ public void Clear() Request = new(); Contexts.Clear(); User = new(); - Release = default; + Release = Options.Release; Distribution = default; - Environment = default; + Environment = Options.Environment; // Restore to keep in sync with native TransactionName = default; Transaction = default; Fingerprint = Array.Empty(); diff --git a/src/Sentry/SentryLog.cs b/src/Sentry/SentryLog.cs index 8e74be21e8..4bea633e35 100644 --- a/src/Sentry/SentryLog.cs +++ b/src/Sentry/SentryLog.cs @@ -136,8 +136,7 @@ public bool TryGetAttribute(string key, [NotNullWhen(true)] out object? value) = internal void SetDefaultAttributes(SentryOptions options, Scope? scope, SdkVersion? sdk = null) { // Core Attributes - sdk ??= scope?.Sdk ?? SdkVersion.Instance; - Attributes.SetDefaultAttributes(options, sdk); + Attributes.SetDefaultAttributes(options, scope, sdk); // Server Attributes if (!string.IsNullOrEmpty(options.ServerName)) diff --git a/src/Sentry/SentryMetric.Factory.cs b/src/Sentry/SentryMetric.Factory.cs index 7c9621ce84..3cc51b655d 100644 --- a/src/Sentry/SentryMetric.Factory.cs +++ b/src/Sentry/SentryMetric.Factory.cs @@ -20,7 +20,7 @@ private static SentryMetric CreateCore(IHub hub, SentryOptions options, IS }; scope ??= hub.GetScope(); - metric.Attributes.SetDefaultAttributes(options, scope?.Sdk ?? SdkVersion.Instance); + metric.Attributes.SetDefaultAttributes(options, scope); return metric; } diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs index a6e31c57af..b8b4b993f9 100644 --- a/src/Sentry/SentrySdk.cs +++ b/src/Sentry/SentrySdk.cs @@ -63,6 +63,10 @@ internal static IHub InitHub(SentryOptions options) #pragma warning restore 0162 #pragma warning restore CS0162 // Unreachable code detected + // This happens before the native SDKs get initialized + options.Environment = options.SettingLocator.GetEnvironment(); + options.Release = options.SettingLocator.GetRelease(); + // Initialize native platform SDKs here if (options.InitNativeSdks) { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt index 4b2ecc6f40..024203f121 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt @@ -220,6 +220,7 @@ namespace Sentry void AddAttachment(Sentry.SentryAttachment attachment); void AddBreadcrumb(Sentry.Breadcrumb breadcrumb); void ClearAttachments(); + void SetEnvironment(string? environment); void SetExtra(string key, object? value); void SetTag(string key, string value); void SetTrace(Sentry.SentryId traceId, Sentry.SpanId parentSpanId); diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index 4b2ecc6f40..024203f121 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -220,6 +220,7 @@ namespace Sentry void AddAttachment(Sentry.SentryAttachment attachment); void AddBreadcrumb(Sentry.Breadcrumb breadcrumb); void ClearAttachments(); + void SetEnvironment(string? environment); void SetExtra(string key, object? value); void SetTag(string key, string value); void SetTrace(Sentry.SentryId traceId, Sentry.SpanId parentSpanId); diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt index 4b2ecc6f40..024203f121 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt @@ -220,6 +220,7 @@ namespace Sentry void AddAttachment(Sentry.SentryAttachment attachment); void AddBreadcrumb(Sentry.Breadcrumb breadcrumb); void ClearAttachments(); + void SetEnvironment(string? environment); void SetExtra(string key, object? value); void SetTag(string key, string value); void SetTrace(Sentry.SentryId traceId, Sentry.SpanId parentSpanId); diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index 211c1e6ce2..8394533269 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt @@ -208,6 +208,7 @@ namespace Sentry void AddAttachment(Sentry.SentryAttachment attachment); void AddBreadcrumb(Sentry.Breadcrumb breadcrumb); void ClearAttachments(); + void SetEnvironment(string? environment); void SetExtra(string key, object? value); void SetTag(string key, string value); void SetTrace(Sentry.SentryId traceId, Sentry.SpanId parentSpanId); @@ -2029,4 +2030,4 @@ public static class SentryExceptionExtensions public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary data) { } public static void AddSentryTag(this System.Exception ex, string name, string value) { } public static void SetSentryMechanism(this System.Exception ex, string type, string? description = null, bool? handled = default, bool? terminal = default) { } -} \ No newline at end of file +} diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index ccb1b277bf..ff0c9b5a17 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -426,6 +426,29 @@ public void Clear_SetsPropertiesToDefaultValues() } } + [Fact] + public void Clear_OptionsHaveEnvironmentAndRelease_RestoresToOptionsValues() + { + // Arrange + var options = new SentryOptions + { + Environment = "options-environment", + Release = "options-release", + }; + var sut = new Scope(options); + sut.ApplyFakeValues(); + + // Act + sut.Clear(); + + // Assert + using (new AssertionScope()) + { + sut.Environment.Should().Be("options-environment"); + sut.Release.Should().Be("options-release"); + } + } + [Fact] public void Clear_ResetsPropagationContext() { @@ -696,6 +719,82 @@ public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumbIfEnabled(bool obs observer.Received(expectedCount).AddBreadcrumb(Arg.Is(breadcrumb)); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void SetEnvironment_ObserverExist_ObserverSetsEnvironmentIfEnabled(bool observerEnable) + { + // Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); + const string expectedEnvironment = "staging"; + var expectedCount = observerEnable ? 1 : 0; + + // Act + scope.Environment = expectedEnvironment; + + // Assert + observer.Received(expectedCount).SetEnvironment(Arg.Is(expectedEnvironment)); + } + + [Fact] + public void SetEnvironment_Null_EnvironmentSetToOptionEnvironment() + { + // Arrange + const string expectedEnvironment = "staging"; + var scope = new Scope(new SentryOptions { Environment = expectedEnvironment }); + + // Act + scope.Environment = null; + + // Assert + scope.Environment.Should().Be(expectedEnvironment); + } + + [Fact] + public void SetEnvironment_Null_ObserverReceivesOptionEnvironment() + { + // Arrange + const string expectedEnvironment = "staging"; + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions + { + ScopeObserver = observer, + EnableScopeSync = true, + Environment = expectedEnvironment + }); + observer.ClearReceivedCalls(); + + // Act + scope.Environment = null; + + // Assert + observer.Received(1).SetEnvironment(Arg.Is(expectedEnvironment)); + } + + [Fact] + public void SetEnvironment_SameValue_ObserverNotifiedOnce() + { + // Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions + { + ScopeObserver = observer, + EnableScopeSync = true + }); + + // Act + scope.Environment = "staging"; + scope.Environment = "staging"; + + // Assert + observer.Received(1).SetEnvironment(Arg.Is("staging")); + } + [Fact] public void Filtered_tags_are_not_set() { diff --git a/test/Sentry.Tests/SentryLogTests.cs b/test/Sentry.Tests/SentryLogTests.cs index c6481be437..f828d3c659 100644 --- a/test/Sentry.Tests/SentryLogTests.cs +++ b/test/Sentry.Tests/SentryLogTests.cs @@ -76,6 +76,43 @@ public void Protocol_Default_VerifyAttributes() log.Attributes.ShouldNotContain("not-found"); } + [Fact] + public void SetDefaultAttributes_ScopeOverridesOptions_UsesScopeValues() + { + var options = new SentryOptions + { + Environment = "options-environment", + Release = "options-release", + }; + var scope = new Scope(options) + { + Environment = "scope-environment", + Release = "scope-release", + }; + var log = new SentryLog(Timestamp, TraceId, SentryLogLevel.Info, "message"); + + log.SetDefaultAttributes(options, scope); + + log.Attributes.ShouldContain("sentry.environment", "scope-environment"); + log.Attributes.ShouldContain("sentry.release", "scope-release"); + } + + [Fact] + public void SetDefaultAttributes_NullScope_FallsBackToOptionsSettings() + { + var options = new SentryOptions + { + Environment = "options-environment", + Release = "options-release", + }; + var log = new SentryLog(Timestamp, TraceId, SentryLogLevel.Info, "message"); + + log.SetDefaultAttributes(options, scope: null); + + log.Attributes.ShouldContain("sentry.environment", options.SettingLocator.GetEnvironment()); + log.Attributes.ShouldContain("sentry.release", options.SettingLocator.GetRelease()); + } + [Fact] public void SetDefaultAttributes_OptionsServerName_SetsServerAddress() { diff --git a/test/Sentry.Tests/SentryMetricTests.cs b/test/Sentry.Tests/SentryMetricTests.cs index d369324cb7..ef7c366897 100644 --- a/test/Sentry.Tests/SentryMetricTests.cs +++ b/test/Sentry.Tests/SentryMetricTests.cs @@ -44,10 +44,13 @@ public void Protocol_Default_VerifyAttributes() Environment = "my-environment", Release = "my-release", }; - var sdk = new SdkVersion + var scope = new Scope(options) { - Name = "Sentry.Test.SDK", - Version = "1.2.3-test+Sentry", + Sdk = + { + Name = "Sentry.Test.SDK", + Version = "1.2.3-test+Sentry" + } }; var metric = new SentryMetric(Timestamp, TraceId, SentryMetricType.Counter, "sentry_tests.sentry_metric_tests.counter", 1) @@ -56,7 +59,7 @@ public void Protocol_Default_VerifyAttributes() Unit = "test_unit", }; metric.SetAttribute("attribute", "value"); - metric.Attributes.SetDefaultAttributes(options, sdk); + metric.Attributes.SetDefaultAttributes(options, scope); metric.Timestamp.Should().Be(Timestamp); metric.TraceId.Should().Be(TraceId); @@ -69,11 +72,48 @@ public void Protocol_Default_VerifyAttributes() metric.Attributes.ShouldContain("attribute", "value"); metric.Attributes.ShouldContain("sentry.environment", options.Environment); metric.Attributes.ShouldContain("sentry.release", options.Release); - metric.Attributes.ShouldContain("sentry.sdk.name", sdk.Name); - metric.Attributes.ShouldContain("sentry.sdk.version", sdk.Version); + metric.Attributes.ShouldContain("sentry.sdk.name", scope.Sdk.Name); + metric.Attributes.ShouldContain("sentry.sdk.version", scope.Sdk.Version); metric.Attributes.ShouldNotContain("not-found"); } + [Fact] + public void SetDefaultAttributes_ScopeOverridesOptions_UsesScopeValues() + { + var options = new SentryOptions + { + Environment = "options-environment", + Release = "options-release", + }; + var scope = new Scope(options) + { + Environment = "scope-environment", + Release = "scope-release", + }; + var metric = new SentryMetric(Timestamp, TraceId, SentryMetricType.Counter, "sentry_tests.sentry_metric_tests.counter", 1); + + metric.Attributes.SetDefaultAttributes(options, scope); + + metric.Attributes.ShouldContain("sentry.environment", "scope-environment"); + metric.Attributes.ShouldContain("sentry.release", "scope-release"); + } + + [Fact] + public void SetDefaultAttributes_NullScope_FallsBackToOptionsSettings() + { + var options = new SentryOptions + { + Environment = "options-environment", + Release = "options-release", + }; + var metric = new SentryMetric(Timestamp, TraceId, SentryMetricType.Counter, "sentry_tests.sentry_metric_tests.counter", 1); + + metric.Attributes.SetDefaultAttributes(options, scope: null); + + metric.Attributes.ShouldContain("sentry.environment", options.SettingLocator.GetEnvironment()); + metric.Attributes.ShouldContain("sentry.release", options.SettingLocator.GetRelease()); + } + [Fact] public void WriteTo_Envelope_MinimalSerializedSentryMetric() { @@ -84,7 +124,7 @@ public void WriteTo_Envelope_MinimalSerializedSentryMetric() }; var metric = new SentryMetric(Timestamp, TraceId, SentryMetricType.Counter, "sentry_tests.sentry_metric_tests.counter", 1); - metric.Attributes.SetDefaultAttributes(options, new SdkVersion()); + metric.Attributes.SetDefaultAttributes(options, new Scope(options)); var envelope = Envelope.FromMetric(new TraceMetric([metric])); @@ -162,7 +202,7 @@ public void WriteTo_EnvelopeItem_MaximalSerializedSentryMetric() metric.SetAttribute("boolean-attribute", true); metric.SetAttribute("integer-attribute", 3); metric.SetAttribute("double-attribute", 4.4); - metric.Attributes.SetDefaultAttributes(options, new SdkVersion { Name = "Sentry.Test.SDK", Version = "1.2.3-test+Sentry" }); + metric.Attributes.SetDefaultAttributes(options, new Scope(options) { Sdk = { Name = "Sentry.Test.SDK", Version = "1.2.3-test+Sentry" } }); var envelope = EnvelopeItem.FromMetric(new TraceMetric([metric]));