Using Xray with stscreds is difficult and awkward

[copumpkin]

The Xray integration with this SDK seems to rely a lot on passing Contexts around, which is easy in most cases, except when you need to go through AssumeRole with stscreds, where the type signature doesn't really give stscreds enough power to be able to call AssumeRoleWithContext on the underlying STS client.

It would be nice if there could be an endorsed way use stscreds with Xray, because if you just leave it alone right now, you get the following panic:

panic: failed to begin subsegment named 'sts': segment cannot be found.

In an ideal world, it seems like if an API call using the stscreds provider needs to refresh the credentials, then the AssumeRole should use the context of the API triggering the refresh. That would allow it to respect timeouts and such, as well as obviously letting Xray see into the bowels of a request that triggers an AssumeRole.

I'd welcome ideas for workarounds. Right now we've had to just make an AssumeRoler that looks into a global variable for the context to use, and then we have remember to set the global properly.

[varming]

I would go a little further and say that stscreds should use the context object passed into *WithContext methods when refreshing credentials.

[copumpkin]

It looks like @jasdel just recently added a GetWithContext to credentials that seemingly appears to help with this (assuming stscreds had an AssumeRolerWithContext or similar), except it has a comment on it saying that it can't pass the context further down. I don't quite follow the justification though:

aws-sdk-go/aws/credentials/credentials.go

Lines 228 to 235 in 6194da3

	func (c *Credentials) GetWithContext(ctx Context) (Value, error) {
	if curCreds := c.creds.Load(); !c.isExpired(curCreds) {
	return curCreds.(Value), nil
	}
	// Cannot pass context down to the actual retrieve, because the first
	// context would cancel the whole group when there is not direct
	// association of items in the group.

[skmcgrail]

The reason GetWithContext does not pass down the context to the providers' retrieve implementation is to prevent the cancellation of the context causing a failure to refresh the providers' credentials. The update that introduced GetWithContext introduced an underlying implementation change where the refreshing and waiting on credentials to be refreshed is no longer gated around a sync.Mutex lock, but now occurs using a singleflight.Group. The previous solution could cause issues with client side memory exhaustion and additionally the client could issue large numbers of credential refresh attempts outside the normal retry strategy.

The design change ensures that if a request to refresh credentials is currently in flight, all other calls to Get will wait on that same request, and will receive the same output or error response from said request. Thus to ensure a single context does not force the cancellation of a refresh for other Get requests we've opted not to pass in the original context down during retrieval. We are still able to honor a context cancellation though, and the individual call to Get will appropriately cancel and return from the Get regardless of whether there is an inflight refresh occurring.

[agacek]

Thanks Sean. In that case, what's the recommended way to use X-Ray in the Go SDK when using credentials from an AssumeRoler?