Skip to content

Conversation

@jdisanti
Copy link
Contributor

@jdisanti jdisanti commented Dec 5, 2022

Motivation and Context

This PR adds an info event for each time there is a credentials cache miss with the time it took to reload credentials, removes some duplicate provider spans, and adjusts some event/span levels.

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti marked this pull request as ready for review December 5, 2022 21:18
@jdisanti jdisanti requested review from a team as code owners December 5, 2022 21:18
@jdisanti jdisanti requested review from 82marbag and hlbarber December 5, 2022 21:18
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

if context.response().status().as_u16() == 404 =>
{
tracing::info!(
tracing::warn!(
Copy link
Contributor

Choose a reason for hiding this comment

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

users are gonna love this

let span = info_span!("lazy_load_credentials");
let future = Timeout::new(loader.provide_credentials(), timeout_future);
cache
let start_time = Instant::now();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should use tokio::time::Instant in this so that this is more controllable in tests. Maybe it would be worth testing cache miss time? IDK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using tokio::time::Instant in tests is fine, but I'm not as sold on using it in the actual code (without abstracting it in some way) since it requires the time feature, which pulls in Tokio's async sleep. I don't think this specific case really needs a test.

@jdisanti jdisanti enabled auto-merge (squash) December 7, 2022 23:48
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti merged commit d5bcd4f into main Dec 8, 2022
@jdisanti jdisanti deleted the jdisanti-credentials-tracing branch December 8, 2022 00:22
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.

2 participants