Skip to content

Conversation

@kinelski
Copy link
Contributor

@kinelski kinelski commented Mar 6, 2020

Fixes #10324

Event Hubs:

  • Renamed enqueued time attribute from x-opt-enqueued-time to enqueuedTime.
  • Removed the enqueued time attribute from the root processing activity and put it in the linked activity instead.
  • Created a new test. It's currently being ignored because it's very similar to RunPartitionProcessingAsyncCreatesScopeForEventProcessing, so it's expected to be flaky during nightly runs. It's already being tracked by [EventHubs] Event Processor tests failing during nightly runs (Track Two) #10067. No issues when running locally.

Core:

  • Updated the ClientDiagnosticListener test helper to track linked activities as well, not only their parent ids. I'm keeping the Links property since it's already being used in multiple tests across the repo.

@kinelski kinelski added Event Hubs Client This issue is related to a non-management package Azure.Core labels Mar 6, 2020
@kinelski kinelski added this to the [2020] April milestone Mar 6, 2020
@kinelski kinelski requested review from jsquire and pakrym March 6, 2020 21:19
@kinelski kinelski self-assigned this Mar 6, 2020
@pakrym
Copy link
Contributor

pakrym commented Mar 6, 2020

Change itself LGTM, the only test being ignored concerns me slightly.

@kinelski
Copy link
Contributor Author

kinelski commented Mar 6, 2020

A can enable it for the time being, but I believe it will be flaky during nightly runs for the same (unknown) reason similar tests are. I can put the Ignore attribute back later if that's the case.

Copy link
Member

@jsquire jsquire left a comment

Choose a reason for hiding this comment

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

LGTM.

Which test were we thinking would be a problem?

@kinelski
Copy link
Contributor Author

kinelski commented Mar 6, 2020

The new test: RunPartitionProcessingAsyncAddsAttributesToLinkedActivities.

It's basically the same as RunPartitionProcessingAsyncCreatesScopeForEventProcessing, but the assertions at the end are different. That's why I believe it will be flaky.

@kinelski kinelski merged commit 729051f into Azure:master Mar 6, 2020
@kinelski kinelski deleted the diag branch March 6, 2020 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core Client This issue is related to a non-management package Event Hubs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] EH SDK: Enqueued Time on root activity, not on the links

3 participants