Optimize orchestration history scans for tracing performance - #799
Optimize orchestration history scans for tracing performance#799Chris Sheldrick (csheldrick) wants to merge 3 commits into
Conversation
perf: avoid repeated orchestration history scans for tracing
There was a problem hiding this comment.
Pull request overview
This PR optimizes the gRPC worker’s orchestration tracing path by avoiding repeated scans of orchestration history and skipping all tracing lookup/indexing work when the Durable Task ActivitySource has no listeners. It introduces a one-pass index over relevant past history events to enable O(1) lookups for completed/failed task and sub-orchestration tracing, while preserving prior “first/last match” semantics.
Changes:
- Add a listener gate (
TraceHelper.HasListeners()) so history lookup/indexing work is skipped when tracing is inactive. - Introduce
TracingHistoryEventIndexto build a single-pass index over relevantPastEventsand use it duringNewEventsprocessing. - Add unit tests covering duplicate IDs, missing IDs, and listener detection behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Shared/Grpc/Tracing/TraceHelper.cs | Adds a listener-detection helper used to skip tracing work when no listeners exist. |
| src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs | Uses the listener gate and the new history index to avoid repeated history scans during tracing. |
| src/Worker/Grpc/TracingHistoryEventIndex.cs | New helper to index past history events for O(1) tracing lookups while preserving first/last semantics. |
| test/Worker/Grpc.Tests/TraceHelperTests.cs | Adds unit test for listener detection behavior. |
| test/Worker/Grpc.Tests/TracingHistoryEventIndexTests.cs | Adds unit tests validating duplicate-ID and missing-ID lookup semantics. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| ActivityListener listener = new() | ||
| { | ||
| ShouldListenTo = source => source.Name == "Microsoft.DurableTask", | ||
| }; |
| // Preserve the previous FirstOrDefault semantics for duplicate IDs. | ||
| if (!this.subOrchestrationCreatedEvents.ContainsKey(historyEvent.EventId)) | ||
| { | ||
| this.subOrchestrationCreatedEvents.Add(historyEvent.EventId, historyEvent); | ||
| } | ||
|
|
||
| break; |
| /// <summary> | ||
| /// Gets whether any listener is subscribed to Durable Task tracing activities. | ||
| /// </summary> | ||
| /// <returns><see langword="true"/> when tracing work can produce activities; otherwise, <see langword="false"/>.</returns> | ||
| public static bool HasListeners() => ActivityTraceSource.HasListeners(); |
|
@microsoft-github-policy-service agree [company="csheldrick"] |
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
🟢 Approval recommended
The change is a scoped, well-tested internal performance optimization that preserves existing tracing lookup semantics and avoids work when tracing is inactive.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
What changed?
Why is this change needed?
Issues / work items
⸻
Project checklist
⸻
AI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
AI verification (required if AI was used):
⸻
Testing
Automated tests
Manual validation (only if runtime/behavior changed)
⸻
Notes for reviewers