Merge pull request #5 from getsentry/design/api

Microsoft.Extension.Logging integration keeps scope as state

samples/Sentry.Samples.ME.Logging/Program.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using Microsoft.Extensions.Logging;
 
 namespace Sentry.Samples.ME.Logging
@@ -29,19 +30,41 @@ static void App()
                 .AddSentry(o =>
                 {
                     // The default values are:
-                    o.MinimumBreadcrumbLevel = LogLevel.Information;
-                    o.MinimumEventLevel = LogLevel.Error;
-                    o.MaxLogBreadcrumbs = 100;
+                    o.MinimumBreadcrumbLevel = LogLevel.Information; // It requires at least this level to store breadcrumb
+                    o.MinimumEventLevel = LogLevel.Error; // This level or above will result in event sent to Sentry
+
                 })
                 .AddConsole())
             {
                 var logger = loggerFactory.CreateLogger<Program>();
 
-                logger.LogTrace("By default this log level is no-op");
+                logger.LogTrace("1 - By *default* this log level is ignored by Sentry.");
+
+                logger.LogInformation("2 -Information messages are stored as Breadcrumb, sent with the next event.");
+
+                logger.LogError("3 - This generates an event, captured by sentry and includes breadcrumbs (2) tracked in this transaction.");
+
+                using (logger.BeginScope(new Dictionary<string, string>
+                {
+                    {"A - some context", "some value"},
+                    {"B - more info on this", "more value"},
+                }))
+                {
+                    logger.LogWarning("4 - Breadcrumb that only exists inside this scope");
+
+                    logger.LogError("5 - An event that includes the scoped key-value (A, B) above and also the breadcrumbs: (2, 4)");
+
+                    using (logger.BeginScope("C - Inner most scope, with single string state"))
+                    {
+                        logger.LogInformation("6 - Inner most breadcrumb");
+
+                        logger.LogError("7 - An event that includes the scope key-value (A, B, C) and also the breadcrumbs: (2, 4, 6)");
+                    }
 
-                logger.LogInformation("This should only store a Breadcrumb");
+                    logger.LogError("8 - Includes Scope (A, B) and breadcrumbs: (2, 4)");
+                }
 
-                logger.LogError("This generates an event captured by sentry which includes the message above.");
+                logger.LogError("9 - No scope data, breadcrumb: 2");
 
             } // Disposing the logger won't affect Sentry: The lifetime is managed externally (call CloseAndFlush)
         }

src/Sentry.Extensions.Logging/SentryLogger.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
 using Microsoft.Extensions.Logging;
 using Sentry.Extensibility;
@@ -45,15 +44,17 @@ internal SentryLogger(
 
         public IDisposable BeginScope<TState>(TState state)
         {
-            var guard = _sdk.PushScope();
+            var guard = _sdk.PushScope(state);
 
             // TODO: store state within Scope to be read later when (if) event is sent
 
             return guard;
         }
 
-
-        public bool IsEnabled(LogLevel logLevel) => _sdk.IsEnabled && logLevel >= _options.MinimumBreadcrumbLevel;
+        public bool IsEnabled(LogLevel logLevel) => _sdk.IsEnabled
+                                                    && logLevel != LogLevel.None
+                                                    && (logLevel >= _options.MinimumBreadcrumbLevel
+                                                    || logLevel >= _options.MinimumEventLevel);
 
         public void Log<TState>(
             LogLevel logLevel,
@@ -68,20 +69,8 @@ public void Log<TState>(
             }
 
             var message = formatter?.Invoke(state, exception);
-
-            // If it's enabled, level is configured to at least store event as Breadcrumb
-            if (logLevel < _options.MinimumEventLevel)
-            {
-                _sdk.ConfigureScope(
-                    s => s.AddBreadcrumb(
-                        _clock,
-                        message,
-                        "logger",
-                        CategoryName,
-                        eventId.ToTupleOrNull(),
-                        logLevel.ToBreadcrumbLevel()));
-            }
-            else
+            if (_options.MinimumEventLevel != LogLevel.None
+                && logLevel >= _options.MinimumEventLevel)
             {
                 var @event = new SentryEvent(exception)
                 {
@@ -97,6 +86,18 @@ public void Log<TState>(
 
                 _sdk.CaptureEvent(@event);
             }
+            else if (_options.MinimumBreadcrumbLevel != LogLevel.None
+                     && logLevel >= _options.MinimumBreadcrumbLevel)
+            {
+                _sdk.ConfigureScope(
+                    s => s.AddBreadcrumb(
+                        _clock,
+                        message,
+                        "logger",
+                        CategoryName,
+                        eventId.ToTupleOrNull(),
+                        logLevel.ToBreadcrumbLevel()));
+            }
         }
     }
 }

src/Sentry.Extensions.Logging/SentryLoggerProvider.cs

@@ -11,7 +11,7 @@ internal class SentryLoggerProvider : ILoggerProvider
         private IDisposable _scope;
 
         public SentryLoggerProvider(SentryLoggingOptions options)
-            : this(StaticSentryScopeManagement.Instance, options)
+            : this(SentryCoreAdapter.Instance, options)
         { }
 
         internal SentryLoggerProvider(

src/Sentry.Extensions.Logging/SentryLoggingOptions.cs

@@ -8,20 +8,6 @@ namespace Sentry.Extensions.Logging
     /// </summary>
     public class SentryLoggingOptions
     {
-        /// <summary>
-        /// Gets or sets the maximum log breadcrumbs.
-        /// </summary>
-        /// <remarks>
-        /// Events with level higher than <see cref="MinimumBreadcrumbLevel"/>
-        /// will be logged as <see cref="Breadcrumb"/>
-        /// When the number of events reach this configuration value,
-        /// older breadcrumbs start dropping to make room for new ones.
-        /// </remarks>
-        /// <value>
-        /// The maximum log breadcrumbs.
-        /// </value>
-        public int MaxLogBreadcrumbs { get; set; } = 100;
-
         /// <summary>
         /// Gets or sets the minimum breadcrumb level.
         /// </summary>

src/Sentry/Extensibility/ISdk.cs

@@ -12,6 +12,7 @@ public interface ISdk
         // Scope stuff:
         void ConfigureScope(Action<Scope> configureScope);
         IDisposable PushScope();
+        IDisposable PushScope<TState>(TState state);
 
         // Client or Client/Scope stuff:
         SentryResponse CaptureEvent(SentryEvent evt);

src/Sentry/ISentryScopeManagement.cs → src/Sentry/Extensibility/ISentryScopeManagement.cs

@@ -1,7 +1,7 @@
 using System;
 using Sentry.Protocol;
 
-namespace Sentry
+namespace Sentry.Extensibility
 {
     /// <summary>
     /// Scope management
@@ -23,5 +23,7 @@ public interface ISentryScopeManagement
         /// <returns>A disposable which removes the scope
         /// from the environment when invoked</returns>
         IDisposable PushScope();
+
+        IDisposable PushScope<TState>(TState state);
     }
 }

src/Sentry/Extensibility/SentryCoreAdapter.cs

@@ -6,10 +6,16 @@
 namespace Sentry.Extensibility
 {
     /// <summary>
-    /// Allows testing classes which depend on static <see cref="SentryCore"/>
+    /// An implementation of <see cref="ISdk" /> which forwards any call to <see cref="SentryCore" />
     /// </summary>
-    /// <seealso cref="Sentry.Extensibility.ISdk" />
-    public sealed class SentryCoreAdapter : ISdk
+    /// <remarks>
+    /// Allows testing classes which otherwise would need to depend on static <see cref="SentryCore" />
+    /// by having them depend on ISdk instead, which can be mocked.
+    /// </remarks>
+    /// <seealso cref="ISdk" />
+    /// <inheritdoc cref="ISdk" />
+    /// <inheritdoc cref="ISentryScopeManagement" />
+    public sealed class SentryCoreAdapter : ISdk, ISentryScopeManagement
     {
         /// <summary>
         /// The single instance which forwards all calls to <see cref="SentryCore"/>
@@ -18,7 +24,7 @@ public sealed class SentryCoreAdapter : ISdk
 
         private SentryCoreAdapter() { }
 
-        public bool IsEnabled => SentryCore.IsEnabled;
+        public bool IsEnabled { [DebuggerStepThrough] get => SentryCore.IsEnabled; }
 
         [DebuggerStepThrough]
         public void ConfigureScope(Action<Scope> configureScope)
@@ -28,6 +34,10 @@ public void ConfigureScope(Action<Scope> configureScope)
         public IDisposable PushScope()
             => SentryCore.PushScope();
 
+        [DebuggerStepThrough]
+        public IDisposable PushScope<TState>(TState state)
+            => SentryCore.PushScope(state);
+
         [DebuggerStepThrough]
         public SentryResponse CaptureEvent(SentryEvent evt)
             => SentryCore.CaptureEvent(evt);

src/Sentry/HttpSentryClient.cs

@@ -1,4 +1,6 @@
 using System;
+using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Sentry.Protocol;
@@ -18,7 +20,36 @@ public Task<SentryResponse> CaptureEventAsync(SentryEvent @event, Scope scope, C
             => Task.FromResult(new SentryResponse(false));
 
         ///
-        public SentryResponse CaptureEvent(SentryEvent @event, Scope scope) => new SentryResponse(false);
+        public SentryResponse CaptureEvent(SentryEvent @event, Scope scope)
+        {
+            // TODO: Consider multiple events being sent with the same scope:
+            // Wherever this code will end up, it should evaluate only once
+            if (scope.States != null)
+            {
+                foreach (var state in scope.States)
+                {
+                    if (state is string scopeSring)
+                    {
+                        @event.AddTag("scope", scopeSring);
+                    }
+                    else if (state is IEnumerable<KeyValuePair<string, string>> keyValStringString)
+                    {
+                        @event.AddTags(keyValStringString);
+                    }
+                    else if (state is IEnumerable<KeyValuePair<string, object>> keyValStringObject)
+                    {
+                        @event.AddTags(keyValStringObject.Select(k => new KeyValuePair<string, string>(k.Key, k.Value.ToString())));
+                    }
+                    else
+                    {
+                        // TODO: possible callback invocation here
+                        @event.AddExtra("State of unknown type", state.GetType().ToString());
+                    }
+                }
+            }
+
+            return new SentryResponse(false);
+        }
 
         ///
         public void Dispose()

src/Sentry/IScopeOptions.cs

@@ -0,0 +1,7 @@
+namespace Sentry
+{
+    public interface IScopeOptions
+    {
+        int MaxBreadcrumbs { get; }
+    }
+}

src/Sentry/Internals/DisabledSdk.cs

@@ -17,6 +17,7 @@ private DisabledSdk() { }
         public void ConfigureScope(Action<Scope> configureScope) { }
 
         public IDisposable PushScope() => this;
+        public IDisposable PushScope<TState>(TState state) => this;
 
         public bool IsEnabled => false;
 

src/Sentry/Internals/IInternalScopeManagement.cs

@@ -1,3 +1,4 @@
+using Sentry.Extensibility;
 using Sentry.Protocol;
 
 namespace Sentry.Internals

src/Sentry/Internals/Sdk.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Diagnostics;
 using System.Threading.Tasks;
 using Sentry.Extensibility;
 using Sentry.Protocol;
@@ -20,7 +19,7 @@ public Sdk(SentryOptions options)
 
             // Create proper client based on Options
             _client = new HttpSentryClient(options);
-            ScopeManagement = new SentryScopeManagement();
+            ScopeManagement = new SentryScopeManagement(options);
         }
 
         private void CurrentDomain_UnhandledException(object sender, UnhandledExceptionEventArgs e)
@@ -35,6 +34,12 @@ private void CurrentDomain_UnhandledException(object sender, UnhandledExceptionE
 
         public bool IsEnabled => true;
 
+        public void ConfigureScope(Action<Scope> configureScope) => ScopeManagement.ConfigureScope(configureScope);
+
+        public IDisposable PushScope() => ScopeManagement.PushScope();
+
+        public IDisposable PushScope<TState>(TState state) => ScopeManagement.PushScope(state);
+
         public SentryResponse CaptureEvent(SentryEvent evt)
             => WithClientAndScope((client, scope)
                 => client.CaptureEvent(evt, scope));
@@ -63,12 +68,6 @@ public async Task<SentryResponse> CaptureEventAsync(Func<Task<SentryEvent>> even
             return await _client.CaptureEventAsync(@event, ScopeManagement.GetCurrent());
         }
 
-        [DebuggerStepThrough]
-        public void ConfigureScope(Action<Scope> configureScope) => ScopeManagement.ConfigureScope(configureScope);
-
-        [DebuggerStepThrough]
-        public IDisposable PushScope() => ScopeManagement.PushScope();
-
         public void Dispose()
         {
             AppDomain.CurrentDomain.UnhandledException -= CurrentDomain_UnhandledException;

src/Sentry/Internals/SentryScopeManagement.cs

@@ -8,14 +8,17 @@ namespace Sentry.Internals
 {
     internal class SentryScopeManagement : IInternalScopeManagement
     {
+        private readonly IScopeOptions _options;
         private readonly AsyncLocal<ImmutableStack<Scope>> _asyncLocalScope = new AsyncLocal<ImmutableStack<Scope>>();
 
         internal ImmutableStack<Scope> ScopeStack
         {
-            get => _asyncLocalScope.Value ?? (_asyncLocalScope.Value = ImmutableStack.Create(new Scope()));
+            get => _asyncLocalScope.Value ?? (_asyncLocalScope.Value = ImmutableStack.Create(new Scope(_options)));
             set => _asyncLocalScope.Value = value;
         }
 
+        public SentryScopeManagement(IScopeOptions options) => _options = options;
+
         public Scope GetCurrent() => ScopeStack.Peek();
 
         public void ConfigureScope(Action<Scope> configureScope)
@@ -25,10 +28,12 @@ public void ConfigureScope(Action<Scope> configureScope)
         }
 
         // TODO: Microsoft.Extensions.Logging calls its equivalent method: BeginScope()
-        public IDisposable PushScope()
+        public IDisposable PushScope() => PushScope<object>(null);
+
+        public IDisposable PushScope<TState>(TState state)
         {
             var currentScopeStack = ScopeStack;
-            var clonedScope = currentScopeStack.Peek().Clone();
+            var clonedScope = currentScopeStack.Peek().Clone(state);
             var scopeSnapshot = new ScopeSnapshot(currentScopeStack, this);
             ScopeStack = currentScopeStack.Push(clonedScope);