Perf: avoid debug-only logging work when Debug logging is disabled#787
Open
berndverst wants to merge 1 commit into
Open
Perf: avoid debug-only logging work when Debug logging is disabled#787berndverst wants to merge 1 commit into
berndverst wants to merge 1 commit into
Conversation
Gate the debug-only action-list computation (GetActionsListForLogging) and UTF-8 byte-count computations in GrpcDurableTaskWorker.Processor.cs behind this.Logger.IsEnabled(LogLevel.Debug) checks, so this work is skipped entirely when Debug logging is disabled. When Debug logging is enabled, log content is unchanged. Fixes #770 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c145c88d-13ab-421d-91df-184375e4ae51
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the gRPC worker’s hot-path performance by avoiding debug-only logging argument computation when Debug logging is disabled, while preserving identical debug log output when Debug is enabled.
Changes:
- Guard Debug-level orchestrator-response logging to avoid eagerly building the actions list when Debug is off.
- Guard Debug-level activity request/response logging to avoid UTF-8 byte counting when Debug is off.
- Add targeted tests covering both Debug-enabled and Debug-disabled behavior for activity and orchestrator paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/Worker/Grpc.Tests/GrpcDurableTaskWorkerDebugLoggingTests.cs | Adds tests validating Debug logging guards and expected log content when enabled. |
| src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs | Wraps expensive debug-only log argument computations in Logger.IsEnabled(LogLevel.Debug) checks. |
Comment on lines
+339
to
+342
| public void Dispose() | ||
| { | ||
| } | ||
|
|
Comment on lines
+195
to
+198
| static GrpcDurableTaskWorker CreateActivityWorker( | ||
| GrpcDurableTaskWorkerOptions grpcOptions, | ||
| DurableTaskWorkerOptions workerOptions, | ||
| ILoggerFactory loggerFactory) |
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.
Description
Fixes #770.
Avoids debug-only logging work in GrpcDurableTaskWorker.Processor when Debug logging is disabled:
SendingOrchestratorResponselog.ReceivedActivityRequest(input size) andSendingActivityResponse(output size) logs.These computations were previously evaluated eagerly as method arguments to [LoggerMessage]-generated methods, even though the source generator itself only formats/emits the log when the level is enabled. Wrapping each call site in
if (this.Logger.IsEnabled(LogLevel.Debug))skips the extra work entirely when Debug logging is off, while producing byte-for-byte identical log content when Debug logging is on.Scope / non-goals
traceActivity?.SetStatus(...)remains unconditional (it does not depend on Debug logging).Testing
test/Worker/Grpc.Tests/GrpcDurableTaskWorkerDebugLoggingTests.cswith targeted tests covering both the activity path and the orchestrator path:test/Worker/Grpc.Testssuite (141 tests) — all passing.src/Worker/Grpcbuilds cleanly across all target frameworks.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com
Copilot-Session: c145c88d-13ab-421d-91df-184375e4ae51