Open
Conversation
- Remove duplicate BenchmarkDotNet 0.14.0 entry in Directory.Packages.props (fixes NU1506) - Upgrade ExportHistoryWebApp from net6.0 to net8.0 (fixes NETSDK1138) - Remove 'TODO' prefix from exception message in TaskHubGrpcServer
…chestrator - Add LoggerMessage methods for entity client operations (SignalingEntity, GettingEntity, QueryingEntities, CleaningEntityStorage) in Logs.cs - Wire logging calls into GrpcDurableEntityClient replacing TODO comment - Add replay-safe logging to ExecuteScheduleOperationOrchestrator with operation start/complete/error messages - Update orchestrator tests to mock CreateReplaySafeLogger
YunchuWang
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds source-generated and replay-safe logging to improve observability for entity client operations and schedule-operation orchestrations, and includes a couple of small hygiene updates (exception text + package/version cleanup).
Changes:
- Added
LoggerMessage-based logs forGrpcDurableEntityCliententity operations (signal/get/query/clean). - Added replay-safe orchestrator logging in
ExecuteScheduleOperationOrchestratorand updated its unit tests to mockCreateReplaySafeLogger. - Minor maintenance updates: cleaned an exception message, removed a duplicate package version entry, and updated a sample to
net8.0.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/ScheduledTasks.Tests/Orchestrations/ExecuteScheduleOperationOrchestratorTests.cs | Mocks CreateReplaySafeLogger<T>() to support new replay-safe logging in orchestrator tests. |
| src/ScheduledTasks/Orchestrations/ExecuteScheduleOperationOrchestrator.cs | Adds replay-safe start/success/error logs around the schedule entity call. |
| src/InProcessTestHost/Sidecar/Grpc/TaskHubGrpcServer.cs | Replaces a TODO:-prefixed exception message with a user-facing message. |
| src/Client/Grpc/Logs.cs | Introduces new client-side LoggerMessage methods for entity operations. |
| src/Client/Grpc/GrpcDurableEntityClient.cs | Wires the new entity-operation logs into the gRPC entity client methods. |
| samples/ExportHistoryWebApp/ExportHistoryWebApp.csproj | Updates sample target framework from net6.0 to net8.0. |
| Directory.Packages.props | Removes duplicate BenchmarkDotNet version entry. |
src/ScheduledTasks/Orchestrations/ExecuteScheduleOperationOrchestrator.cs
Outdated
Show resolved
Hide resolved
…estrator.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
test/ScheduledTasks.Tests/Orchestrations/ExecuteScheduleOperationOrchestratorTests.cs
Outdated
Show resolved
Hide resolved
…ionOrchestratorTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
src/Client/Grpc/Logs.cs
Outdated
| [LoggerMessage(EventId = 48, Level = LogLevel.Information, Message = "Getting entity '{instanceId}'.")] | ||
| public static partial void GettingEntity(this ILogger logger, string instanceId); | ||
|
|
||
| [LoggerMessage(EventId = 50, Level = LogLevel.Information, Message = "Querying entities with filter: {{ StartsWith = {startsWith}, LastModifiedFrom = {lastModifiedFrom}, LastModifiedTo = {lastModifiedTo} }}")] |
There was a problem hiding this comment.
EventIds 49 and 51 are skipped here but are already in use by other components (InProcessTestHost/Sidecar/Logs.cs). While this is acceptable since they're in different namespaces, consider whether EventId 49 should be used for GetAllEntitiesAsync (currently using EventId 50) to maintain sequential numbering where possible. EventIds 47, 48, 50, 52 work correctly but the gap at 49 and 51 could be confusing for future maintainers.
Suggested change
| [LoggerMessage(EventId = 50, Level = LogLevel.Information, Message = "Querying entities with filter: {{ StartsWith = {startsWith}, LastModifiedFrom = {lastModifiedFrom}, LastModifiedTo = {lastModifiedTo} }}")] | |
| [LoggerMessage(EventId = 49, Level = LogLevel.Information, Message = "Querying entities with filter: {{ StartsWith = {startsWith}, LastModifiedFrom = {lastModifiedFrom}, LastModifiedTo = {lastModifiedTo} }}")] |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
kaibocai
approved these changes
Feb 27, 2026
…//github.com/microsoft/durabletask-dotnet into wangbill/add-entity-and-orchestrator-logging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed?
GrpcDurableEntityClientfor all entity client operations (SignalEntityAsync,GetEntityAsync,GetAllEntitiesAsync,CleanEntityStorageAsync), replacing the existing// TODO this.logger.LogSomethingcomment.LoggerMessagesource-generated methods insrc/Client/Grpc/Logs.cs(EventIds 47, 48, 50, 52) for entity operations.ExecuteScheduleOperationOrchestratorusingcontext.CreateReplaySafeLogger<T>(), with operation start, completion, and error messages. Removed// TODO: loggingcomment.ExecuteScheduleOperationOrchestratorTeststo mockCreateReplaySafeLoggerwithNullLogger.Instance.Why is this change needed?
GrpcDurableEntityClient) had a logger field injected but never used — there was an explicit// TODOto add logging. Without logging, entity operations were invisible in diagnostics, making it harder to troubleshoot issues.ExecuteScheduleOperationOrchestratorhad a// TODO: loggingcomment with zero observability into schedule entity operations executed through it.Issues / work items
// TODO this.logger.LogSomethinginGrpcDurableEntityClient.cs// TODO: logginginExecuteScheduleOperationOrchestrator.csProject checklist
release_notes.mdAI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
src/Client/Grpc/Logs.cs— new LoggerMessage definitionssrc/Client/Grpc/GrpcDurableEntityClient.cs— logging call wiringsrc/ScheduledTasks/Orchestrations/ExecuteScheduleOperationOrchestrator.cs— replay-safe loggingtest/ScheduledTasks.Tests/Orchestrations/ExecuteScheduleOperationOrchestratorTests.cs— mock setup updateAI verification (required if AI was used):
Testing
Automated tests
Manual validation (only if runtime/behavior changed)
Notes for reviewers
LoggerMessagesource generator pattern used inGrpcDurableTaskClient(orchestration client). EventIds 47, 48, 50, 52 were chosen from the available gap after existing client EventIds 40–46.ScheduleOperationInfo(EventId 113) andScheduleOperationError(EventId 115) methods fromLogs.Entity.cs, consistent with how theScheduleentity itself logs operations.