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

Jump to bottom

x/crypto/acme: send req with external context to RetryBackoff function #30183

Open
rekby opened this issue Feb 12, 2019 · 1 comment
Open

x/crypto/acme: send req with external context to RetryBackoff function #30183

rekby opened this issue Feb 12, 2019 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@rekby
Copy link

rekby commented Feb 12, 2019

What did you do?

client = &acme.Client{
			Key:          config.PrivateKey,
			DirectoryURL: *ServerApiURL,
			RetryBackoff: func(n int, r *http.Request, res *http.Response) time.Duration {
				ctx := r.Context()
				retryLogger := zc.L(ctx) // get logger from context https://github.com/rekby/zapcontext
				if retryLogger == nil {
					retryLogger = logger.With(zap.String("request_context", "no-logger"))
				}
                                retryLogger.Info("Test")
                       }
}

Send request to Lets encrypt with retrieval error.
_, err = client.Accept(ctx, httpchallenge)

What did you expect to see?

logs with my logger from ctx, and without request_context" = "no-logger"

What did you see instead?

logs with "request_context" = "no-logger" and without my logger from context ctx.

I suggest to send req with external context to function. Now it receive clear context (but http call do with external context).
My patch for self: https://github.com/rekby/crypto/commit/9874cac870c3253304772cee7f13a87c56040e88
@gopherbot gopherbot added this to the Unreleased milestone Feb 12, 2019
@FiloSottile FiloSottile changed the title x/crypto: send req with external context to RetryBackoff function x/crypto/acme: send req with external context to RetryBackoff function Feb 13, 2019
@bcmills
Copy link
Contributor

bcmills commented Mar 1, 2019

CC @FiloSottile @agl for x/crypto

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@bcmills @rekby @gopherbot