PR feedback: more specific categories

Author: cgillum

src/Worker/Core/Logs.cs

@@ -11,10 +11,16 @@ namespace Microsoft.DurableTask
     /// </summary>
     static partial class Logs
     {
-        /// <summary>
-        /// The common category name for worker logs.
-        /// </summary>
-        internal const string WorkerCategoryName = "Microsoft.DurableTask.Worker";
+        internal static ILogger CreateWorkerLogger(ILoggerFactory loggerFactory, params string[] subcategories)
+        {
+            string categoryName = "Microsoft.DurableTask.Worker";
+            if (subcategories.Length > 0)
+            {
+                categoryName += "." + string.Join(".", subcategories);
+            }
+
+            return loggerFactory.CreateLogger(categoryName);
+        }
 
         [LoggerMessage(EventId = 15, Level = LogLevel.Error, Message = "Unhandled exception in entity operation {entityInstanceId}/{operationName}.")]
         public static partial void OperationError(this ILogger logger, Exception ex, EntityInstanceId entityInstanceId, string operationName);

src/Worker/Core/Shims/TaskActivityShim.cs

@@ -29,7 +29,7 @@ public TaskActivityShim(
         TaskName name,
         ITaskActivity implementation)
     {
-        this.logger = Check.NotNull(loggerFactory).CreateLogger(Logs.WorkerCategoryName);
+        this.logger = Logs.CreateWorkerLogger(Check.NotNull(loggerFactory), "Activities");
         this.dataConverter = Check.NotNull(dataConverter);
         this.name = Check.NotDefault(name);
         this.implementation = Check.NotNull(implementation);
@@ -50,10 +50,10 @@ public TaskActivityShim(
         {
             object? output = await this.implementation.RunAsync(contextWrapper, deserializedInput);
 
-            this.logger.ActivityCompleted(instanceId, this.name);
-
             // Return the output (if any) as a serialized string.
             string? serializedOutput = this.dataConverter.Serialize(output);
+            this.logger.ActivityCompleted(instanceId, this.name);
+
             return serializedOutput;
         }
         catch (Exception e)

src/Worker/Core/Shims/TaskOrchestrationShim.cs

@@ -34,13 +34,11 @@ public TaskOrchestrationShim(
         this.invocationContext = Check.NotNull(invocationContext);
         this.implementation = Check.NotNull(implementation);
 
-        this.logger = this.invocationContext.LoggerFactory.CreateLogger(Logs.WorkerCategoryName);
+        this.logger = Logs.CreateWorkerLogger(this.invocationContext.LoggerFactory, "Orchestrations");
     }
 
     DataConverter DataConverter => this.invocationContext.Options.DataConverter;
 
-    ILoggerFactory LoggerFactory => this.invocationContext.LoggerFactory;
-
     /// <inheritdoc/>
     public override async Task<string?> Execute(OrchestrationContext innerContext, string rawInput)
     {

src/Worker/Grpc/Logs.cs

@@ -8,9 +8,6 @@ namespace Microsoft.DurableTask.Worker.Grpc
     /// <summary>
     /// Log messages.
     /// </summary>
-    /// <remarks>
-    /// NOTE: Trying to make logs consistent with https://github.com/Azure/durabletask/blob/main/src/DurableTask.Core/Logging/LogEvents.cs.
-    /// </remarks>
     static partial class Logs
     {
         [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Durable Task gRPC worker starting and connecting to {endpoint}.")]

test/Grpc.IntegrationTests/IntegrationTestBase.cs

@@ -90,9 +90,9 @@ protected IReadOnlyCollection<LogEntry> GetLogs()
         return logs;
     }
 
-    protected IReadOnlyCollection<LogEntry> GetLogs(string categoryName)
+    protected IReadOnlyCollection<LogEntry> GetLogs(string category)
     {
-        this.logProvider.TryGetLogs(categoryName, out IReadOnlyCollection<LogEntry> logs);
+        this.logProvider.TryGetLogs(category, out IReadOnlyCollection<LogEntry> logs);
         return logs ?? [];
     }
 

test/Grpc.IntegrationTests/OrchestrationErrorHandling.cs

@@ -76,7 +76,7 @@ void MyActivityImpl(TaskActivityContext ctx) =>
         Assert.Equal(typeof(CustomException).FullName, failureDetails.InnerFailure.InnerFailure!.ErrorType);
         Assert.Equal("Inner!", failureDetails.InnerFailure.InnerFailure.ErrorMessage);
 
-        IReadOnlyCollection<LogEntry> workerLogs = this.GetLogs("Microsoft.DurableTask.Worker");
+        IReadOnlyCollection<LogEntry> workerLogs = this.GetLogs(category: "Microsoft.DurableTask.Worker");
         Assert.NotEmpty(workerLogs);
 
         // Check that the orchestrator and activity logs are present

test/TestHelpers/Logging/TestLogProvider.cs

@@ -7,27 +7,21 @@
 
 namespace Microsoft.DurableTask.Tests.Logging;
 
-public sealed class TestLogProvider : ILoggerProvider
+public sealed class TestLogProvider(ITestOutputHelper output) : ILoggerProvider
 {
-    readonly ITestOutputHelper output;
-    readonly ConcurrentDictionary<string, TestLogger> loggers;
-
-    public TestLogProvider(ITestOutputHelper output)
-    {
-        this.output = output ?? throw new ArgumentNullException(nameof(output));
-        this.loggers = new ConcurrentDictionary<string, TestLogger>(StringComparer.OrdinalIgnoreCase);
-    }
+    readonly ITestOutputHelper output = output ?? throw new ArgumentNullException(nameof(output));
+    readonly ConcurrentDictionary<string, TestLogger> loggers = new(StringComparer.OrdinalIgnoreCase);
 
     public bool TryGetLogs(string category, out IReadOnlyCollection<LogEntry> logs)
     {
-        if (this.loggers.TryGetValue(category, out TestLogger? logger))
-        {
-            logs = logger.GetLogs();
-            return true;
-        }
-
-        logs = Array.Empty<LogEntry>();
-        return false;
+        // Get all logs for loggers that are prefixed with the category name
+        // (e.g. "Microsoft.DurableTask.Worker" will return all logs for "Microsoft.DurableTask.Worker.*")
+        logs = this.loggers
+            .Where(kvp => kvp.Key.StartsWith(category, StringComparison.OrdinalIgnoreCase))
+            .SelectMany(kvp => kvp.Value.GetLogs())
+            .OrderBy(log => log.Timestamp)
+            .ToList();
+        return logs.Count > 0;
     }
 
     public void Clear()