Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add debug logging messages on retry #549

Closed
wants to merge 11 commits into from
Closed

Conversation

ericphanson
Copy link
Member

Closes #514

Based on #548

The logging messages are structured so the string is a human-readable summary of the message, and then the fields are more structured for later analysis, e.g. retry::Bool, reason::String, etc. Additionally, the id of the log message will be useful for aggregation.

@ericphanson
Copy link
Member Author

Let's see if CI likes it:

bors try

bors bot added a commit that referenced this pull request May 11, 2022
src/utilities/request.jl Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors bot commented May 11, 2022

Copy link
Member

@christopher-dG christopher-dG left a comment

Choose a reason for hiding this comment

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

LGTM!

@ericphanson
Copy link
Member Author

ok great! I think we should probably first review/merge #548, then merge this on top.

test/issues.jl Outdated Show resolved Hide resolved
test/issues.jl Outdated Show resolved Hide resolved
Base automatically changed from eph/base-retry to master May 25, 2022 15:08
Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

Merge conflict.

ericphanson and others added 6 commits March 20, 2023 15:45
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Mar 20, 2023
549: Add debug logging messages on retry r=mattBrzezinski a=ericphanson

Closes #514

Based on #548

The logging messages are structured so the string is a human-readable summary of the message, and then the fields are  more structured for later analysis, e.g. `retry::Bool`, `reason::String`, etc. Additionally, the id of the log message will be useful for aggregation.

Co-authored-by: Eric Hanson <[email protected]>
Co-authored-by: Matt Brzezinski <[email protected]>
@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

Build failed:

@mattBrzezinski
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Mar 20, 2023
549: Add debug logging messages on retry r=mattBrzezinski a=ericphanson

Closes #514

Based on #548

The logging messages are structured so the string is a human-readable summary of the message, and then the fields are  more structured for later analysis, e.g. `retry::Bool`, `reason::String`, etc. Additionally, the id of the log message will be useful for aggregation.

Co-authored-by: Eric Hanson <[email protected]>
Co-authored-by: Matt Brzezinski <[email protected]>
@@ -110,6 +110,19 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str
return nothing
end

check = (s, e) -> begin
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
check = (s, e) -> begin
check =
(s, e) -> begin

@@ -110,6 +110,19 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str
return nothing
end

check = (s, e) -> begin
if isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500
@debug "AWS.jl Downloads inner retry for status >= 500" retry=true reason="status >= 500" status=_http_status(e) exception=e
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@debug "AWS.jl Downloads inner retry for status >= 500" retry=true reason="status >= 500" status=_http_status(e) exception=e
@debug "AWS.jl Downloads inner retry for status >= 500" retry = true reason = "status >= 500" status = _http_status(
e
) exception = e

@debug "AWS.jl Downloads inner retry for status >= 500" retry=true reason="status >= 500" status=_http_status(e) exception=e
return true
elseif isa(e, Downloads.RequestError)
@debug "AWS.jl Downloads inner retry for Downloads.RequestError" retry=true reason="Downloads.RequestError" exception=e
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@debug "AWS.jl Downloads inner retry for Downloads.RequestError" retry=true reason="Downloads.RequestError" exception=e
@debug "AWS.jl Downloads inner retry for Downloads.RequestError" retry =
true reason = "Downloads.RequestError" exception = e

@debug "AWS.jl Downloads inner retry for Downloads.RequestError" retry=true reason="Downloads.RequestError" exception=e
return true
else
@debug "AWS.jl Downloads inner retry declined" retry=false reason="Exception passed onwards" exception=e
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@debug "AWS.jl Downloads inner retry declined" retry=false reason="Exception passed onwards" exception=e
@debug "AWS.jl Downloads inner retry declined" retry = false reason = "Exception passed onwards" exception =
e

Comment on lines +159 to 164
check = (s, e) -> begin
# Pass on non-AWS exceptions.
if !(e isa AWSException)
@debug "AWS.jl declined to retry non-AWSException" retry=false reason="Non-AWSException" exception=e
return false
end
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
check = (s, e) -> begin
# Pass on non-AWS exceptions.
if !(e isa AWSException)
@debug "AWS.jl declined to retry non-AWSException" retry=false reason="Non-AWSException" exception=e
return false
end
check =
(s, e) -> begin
# Pass on non-AWS exceptions.
if !(e isa AWSException)
@debug "AWS.jl declined to retry non-AWSException" retry = false reason = "Non-AWSException" exception =
e
return false
end

if isa(e, error)
@debug "AWS.jl HTTP inner retry for $error" retry = true reason = "$error" exception =
e
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
return true
) exception = e
return true

Comment on lines 271 to 273
end
if (isa(e, HTTP.StatusError) && _http_status(e) >= 500)
@debug "AWS.jl HTTP inner retry for status >= 500" retry = true reason = "status >= 500" status = AWS._http_status(
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
end
if (isa(e, HTTP.StatusError) && _http_status(e) >= 500)
@debug "AWS.jl HTTP inner retry for status >= 500" retry = true reason = "status >= 500" status = AWS._http_status(
@debug "AWS.jl HTTP inner retry declined" retry = false reason = "Exception passed onwards" exception =

Comment on lines 275 to 276
) exception = e
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
) exception = e
return true
return false

Comment on lines 278 to 281
@debug "AWS.jl HTTP inner retry declined" retry = false reason = "Exception passed onwards" exception =
e
return false
end
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@debug "AWS.jl HTTP inner retry declined" retry = false reason = "Exception passed onwards" exception =
e
return false
end

Comment on lines +9 to +10
record.level == Logging.Debug &&
get(record.kwargs, :retry, nothing) == successful
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
record.level == Logging.Debug &&
get(record.kwargs, :retry, nothing) == successful
record.level == Logging.Debug && get(record.kwargs, :retry, nothing) == successful

@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

Build failed:

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 20, 2023
@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

try

Build failed:

@@ -147,52 +147,71 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers
if e isa HTTP.StatusError
e = AWSException(e, stream)
rethrow(e)
elseif !(e isa AWSException)
@debug "AWS.jl declined to retry non-AWSException" retry=false reason="Non-AWSException" exception=e
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@debug "AWS.jl declined to retry non-AWSException" retry=false reason="Non-AWSException" exception=e
@debug "AWS.jl declined to retry non-AWSException" retry = false reason = "Non-AWSException" exception =
e

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 20, 2023
@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

try

Build failed:

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 20, 2023
@@ -110,6 +110,19 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str
return nothing
end

check = (s, e) -> begin
if isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500
@debug "AWS.jl Downloads inner retry for status >= 500" retry=true reason="status >= 500" status=AWS._http_status(e) exception=e
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@debug "AWS.jl Downloads inner retry for status >= 500" retry=true reason="status >= 500" status=AWS._http_status(e) exception=e
@debug "AWS.jl Downloads inner retry for status >= 500" retry = true reason = "status >= 500" status = AWS._http_status(
e
) exception = e

Comment on lines +258 to +262
check = (s, e) -> begin
errors = (Sockets.DNSError, HTTP.ParseError, Base.IOError)
for error in errors
if isa(e, error)
@debug "AWS.jl HTTP inner retry for $error" retry=true reason="$error" exception=e
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
check = (s, e) -> begin
errors = (Sockets.DNSError, HTTP.ParseError, Base.IOError)
for error in errors
if isa(e, error)
@debug "AWS.jl HTTP inner retry for $error" retry=true reason="$error" exception=e
check =
(s, e) -> begin
errors = (Sockets.DNSError, HTTP.ParseError, Base.IOError)
for error in errors
if isa(e, error)
@debug "AWS.jl HTTP inner retry for $error" retry = true reason = "$error" exception =
e
return true
end
end
if (isa(e, HTTP.StatusError) && _http_status(e) >= 500)
@debug "AWS.jl HTTP inner retry for status >= 500" retry = true reason = "status >= 500" status = _http_status(
e
) exception = e

@debug "AWS.jl HTTP inner retry for $error" retry=true reason="$error" exception=e
return true
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
end
@debug "AWS.jl HTTP inner retry declined" retry = false reason = "Exception passed onwards" exception =
e
return false
end

Comment on lines +266 to +272
if (isa(e, HTTP.StatusError) && _http_status(e) >= 500)
@debug "AWS.jl HTTP inner retry for status >= 500" retry=true reason="status >= 500" status=_http_status(e) exception=e
return true
end
@debug "AWS.jl HTTP inner retry declined" retry=false reason="Exception passed onwards" exception=e
return false
end
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
if (isa(e, HTTP.StatusError) && _http_status(e) >= 500)
@debug "AWS.jl HTTP inner retry for status >= 500" retry=true reason="status >= 500" status=_http_status(e) exception=e
return true
end
@debug "AWS.jl HTTP inner retry declined" retry=false reason="Exception passed onwards" exception=e
return false
end

@mattBrzezinski
Copy link
Member

bors try

@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

try

Build failed:

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 21, 2023
@bors
Copy link
Contributor

bors bot commented Mar 21, 2023

try

Build failed:

@mattBrzezinski
Copy link
Member

Discussed offline, but we'll be revising how to properly introduce debugging here, closing this PR for now.

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.

Emit debug logs on retry?
4 participants