Skip to content

Optimize orchestration history scans for tracing performance - #799

Open
Chris Sheldrick (csheldrick) wants to merge 2 commits into
microsoft:mainfrom
csheldrick:main
Open

Optimize orchestration history scans for tracing performance#799
Chris Sheldrick (csheldrick) wants to merge 2 commits into
microsoft:mainfrom
csheldrick:main

Conversation

@csheldrick

Copy link
Copy Markdown

Summary

What changed?

  • Avoid repeated orchestration history scans performed only for tracing.
  • Skip orchestration tracing lookup/indexing work entirely when the Durable Task ActivitySource has no listeners.
  • Build a one-pass index over relevant PastEvents when tracing is enabled, allowing completion events to use O(1) lookups.
  • Preserve the existing lookup semantics:
    • SubOrchestrationInstanceCreated: first matching event wins.
    • TaskScheduled: last matching event wins.
  • Add unit tests covering duplicate event IDs, missing IDs, and ActivitySource listener detection.

Why is this change needed?

  • The tracing path previously rescanned PastEvents for each completed/failed task or sub-orchestration event.
  • For orchestration histories with many events, this can result in repeated O(N) scans and significant unnecessary CPU overhead.
  • The lookup work was also performed even when no tracing listener was registered.
  • This change reduces tracing-history processing to a single pass when tracing is active and avoids the work entirely when tracing is inactive, without intentionally changing tracing behavior.

Issues / work items

Project checklist

  • Release notes are not required for the next release
    • Otherwise: Notes added to release_notes.md
  • Backport is not required
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change? No
    • If yes:
      • Impact:
      • Migration guidance:

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): ChatGPT
  • AI-assisted areas/files:
    • src/Shared/Grpc/Tracing/TraceHelper.cs
    • src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs
    • src/Worker/Grpc/TracingHistoryEventIndex.cs
    • test/Worker/Grpc.Tests/TraceHelperTests.cs
    • test/Worker/Grpc.Tests/TracingHistoryEventIndexTests.cs
  • What you changed after AI output: I reviewed the generated implementation and tests against the existing tracing code and issue requirements. I verified that the optimization preserves the existing first/last lookup behavior, that the listener gate does not intentionally change tracing semantics, and that the final diff contains only the intended implementation and test changes.

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed
  • dotnet test test/Worker/Grpc.Tests/Worker.Grpc.Tests.csproj --configuration Release
  • Added coverage verifies:
    • first-match semantics for duplicate SubOrchestrationInstanceCreated IDs;
    • last-match semantics for duplicate TaskScheduled IDs;
    • missing event IDs return no match;
    • TraceHelper.HasListeners() detects a matching Durable Task ActivityListener and returns to its previous state after the listener is disposed.

Manual validation (only if runtime/behavior changed)

  • Environment: N/A
  • Steps + observed results:
    1. No separate manual runtime validation was performed because this is an internal tracing performance optimization with no intended externally observable behavior change.
    2. Existing behavior was compared directly against the previous implementation, and the significant lookup semantics were preserved explicitly and covered by automated tests.
  • Evidence (optional): Automated test coverage above.

Notes for reviewers

  • The history index intentionally has different duplicate-ID semantics for its two event types. This preserves the previous implementation exactly: sub-orchestration lookup used FirstOrDefault, while task-scheduled lookup used LastOrDefault.
  • The index is only created when the Durable Task tracing ActivitySource has listeners, so the normal no-listener path avoids both the previous scans and the new index allocation.

Copilot AI lite review requested due to automatic review settings September 1, 2026 04:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TracingHistoryEventIndex to build a single-pass index over relevant PastEvents and use it during NewEvents processing.
  • 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.

Comment on lines +15 to +18
ActivityListener listener = new()
{
ShouldListenTo = source => source.Name == "Microsoft.DurableTask",
};
Comment on lines +23 to +29
// Preserve the previous FirstOrDefault semantics for duplicate IDs.
if (!this.subOrchestrationCreatedEvents.ContainsKey(historyEvent.EventId))
{
this.subOrchestrationCreatedEvents.Add(historyEvent.EventId, historyEvent);
}

break;
Comment on lines +23 to +27
/// <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();
@csheldrick

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree [company="csheldrick"]

@csheldrick

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: avoid repeated orchestration-history scans for tracing

3 participants