[Storage] [WebJobs] LogScan new blobs between the start of polling and the LMT of the last LogScan

[amnguye]

Attempt to resolve issue #51030

Where LogScan fails to detect newly created blobs when the List Blobs/Get Blobs operation requires multiple requests. If a blob is added or modified after LogScan begins and during the later portion of the listing, any blobs changed between the listing start time and the last modified timestamp of blobs found later will not be flagged as new in the subsequent LogScan.

[AartBluestoke]

basically the same change as was proposed at Azure/azure-webjobs-sdk#3014 :)
if you are on the first page of data (ie, no continuation token), you can still safely advance the timestamp as high as you want., this will completely preserve existing behaviour for those people unaffected by this bug

1. list blob api call is issued (first page only, no continuation token)
2. blob on page 1 is edited
3. blob on page 1 is edited.
4. response includes blobs edited at 2 and 3.

if it is possible for storage to return blob 3 but not 2, then as written it is correct, we can't advance timepoint past 1.
if storage only returns either none, just 2, or both 2 and 3, then advancing the high water mark to timepoints 2 and 3 to avoid them being also returned in the next call is best (potential double handling is benign as etag filtering will silently drop them).

[amnguye]

TODO: unit tests, integrated tests, changelog

[amnguye]

basically the same change as was proposed at Azure/azure-webjobs-sdk#3014 :) if you are on the first page of data (ie, no continuation token), you can still safely advance the timestamp as high as you want., this will completely preserve existing behaviour for those people unaffected by this bug

1. list blob api call is issued (first page only, no continuation token)
2. blob on page 1 is edited
3. blob on page 1 is edited.
4. response includes blobs edited at 2 and 3.

if it is possible for storage to return blob 3 but not 2, then as written it is correct, we can't advance timepoint past 1. if storage only returns either none, just 2, or both 2 and 3, then advancing the high water mark to timepoints 2 and 3 to avoid them being also returned in the next call is best (potential double handling is benign as etag filtering will silently drop them).

Ah nice! Sorry that I didn't see that PR. Looks like it's in the Azure/azure-webjobs-sdk repo and should have be in this repo! Looks like that didn't fall under my radar cause I don't work in that repo. But thanks for the contribution!

[amnguye]

Probably not my favorite changelog message, any rewording suggestions is welcomed here.

[amnguye]

@mathewc @fabiocav Could I get a review on this?

[Copilot]

Pull request overview

This PR fixes a bug in blob polling logic where newly created or modified blobs could be missed during multi-page blob listing operations. The fix introduces a PollingStartTime timestamp to ensure only blobs modified before polling began are considered in the current scan cycle, preventing gaps in blob detection when listings span multiple pages.

Key Changes:

• Added PollingStartTime property to track when each polling cycle begins
• Modified blob detection logic to filter blobs based on the polling start time
• Added comprehensive test coverage for the multi-page continuation token scenario

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
ContainerScanInfo.cs	Adds PollingStartTime property to track when polling began
ScanBlobScanLogHybridPollingStrategy.cs	Updates polling logic to set PollingStartTime and filter blobs based on this timestamp
ScanBlobScanLogHybridPollingStrategyTests.cs	Adds test for continuation token scenario and updates existing test timestamp type
TestAsyncPageableWithContinuationToken.cs	New test helper class for simulating paged blob listing with continuation tokens
CHANGELOG.md	Documents the bug fix

[Copilot]

Using Thread.Sleep in tests makes them slower and potentially flaky. Since you're setting up blob timestamps in the future (line 449 uses UtcNow.AddSeconds(5)), the test is relying on actual time passing to validate the behavior. Consider redesigning the test to use deterministic timestamps that don't require waiting, or use a time provider abstraction that can be mocked. For example, you could set blob timestamps relative to a fixed point in time rather than DateTimeOffset.UtcNow.

[amnguye]

I might actually look into this, we may have to use Reflection(?) to set the exact start polling time. It will make this test more resilient. If not, we'd have to refactor the strategy itself to make it more testable.

[Copilot]

[nitpick] The bug description is verbose and could be simplified for better readability. Consider rephrasing to: "Fixed a bug where polling for new blobs fails to detect blobs modified between the start of a multi-page blob listing and the last modified timestamp of blobs found in later pages."

Suggested change

- Fixed a bug where polling for new blobs fails to detect newly created blobs when the List Blobs/Get Blobs operation requires multiple requests. If a blob is added or modified after LogScan begins, and is listed during the later portion of the listing, any blobs changed between the listing start time and the last modified timestamp of blobs found later will not be flagged as new in the subsequent LogScan.

- Fixed a bug where polling for new blobs fails to detect blobs modified between the start of a multi-page blob listing and the last modified timestamp of blobs found in later pages.

[amnguye]

I don't think this fix details best how this issue is exposed. But if others would rather see it shortened to this, I can do that.