Skip to content

Conversation

@JoshLove-msft
Copy link
Member

@JoshLove-msft JoshLove-msft commented Dec 17, 2020

  • Add the same tweak to AmqpConnectionScope that was added for Event Hubs in [Event Hubs Client] Credential Refresh Buffer Tweak #17330
  • Add logging for the AuthorizationRequest and thread the Identifier through.
  • Add a delay after test resource creation to avoid flakiness in tests
  • Fix other flaky tests.

@JoshLove-msft
Copy link
Member Author

/azp run net - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

string identifier)
{
string uri = endpoint.AbsoluteUri;
ServiceBusEventSource.Log.RequestAuthorizationStart(identifier, uri);
Copy link
Member

Choose a reason for hiding this comment

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

This seems redundant to the existing AmqpLinkAuthorizationRefresh series (line 865). Am I overlooking something?

Copy link
Member Author

Choose a reason for hiding this comment

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

The difference is that the refresh event is only logged on refreshes, whereas the new logs are written on every request (including the initial one). The new event is also verbose as opposed to Informational.

}

[Event(RequestAuthorizationStartEvent, Level = EventLevel.Verbose, Message = "{0}: Requesting authorization to {1}")]
public virtual void RequestAuthorizationStart(string identifier, string endpoint)
Copy link
Member

Choose a reason for hiding this comment

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

I may be overlooking, but I'm not sure from usage how these differ from the AmqpLinkAuthorizationRefresh series (ids 39-41). Should we consider merging them?

# and the execution of the live tests. This allows RBAC to replicate and avoids flakiness in the first set
# of live tests that might otherwise start running before RBAC has replicated.

param (
Copy link
Member

Choose a reason for hiding this comment

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

Nice. I'm going to steal this after it merges...

@JoshLove-msft JoshLove-msft merged commit cc2262b into Azure:master Dec 17, 2020
annelo-msft pushed a commit to annelo-msft/azure-sdk-for-net that referenced this pull request Feb 17, 2021
* save

* Authorization refresh fix

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants