api: add DOCKER_DEBUG_TRACE to return stack-traces in API error responses#43252
Draft
thaJeztah wants to merge 1 commit intomoby:masterfrom
Draft
api: add DOCKER_DEBUG_TRACE to return stack-traces in API error responses#43252thaJeztah wants to merge 1 commit intomoby:masterfrom
thaJeztah wants to merge 1 commit intomoby:masterfrom
Conversation
thaJeztah
added
area/api
thaJeztah
commented
Feb 17, 2022
…nses This introduces a DOCKER_DEBUG_TRACE which, when set, will propagate a new `details` field in error-responses, containing the error's stack-trace (if any). With this patch applied, the daemon can be started with `DOCKER_DEBUG_TRACE=1 dockerd` to propagate `.details` field if the error contains a stacktrace: ```bash docker container create --name foo busybox docker kill foo Error response from daemon: Cannot kill container: foo: Container 3045f49c661c85c4289e92cc471a357dc64e2cb1603269e8711dec0992fe9daa is not running curl -s -X POST --unix-socket /var/run/docker.sock http://localhost/v1.41/containers/foo/kill | jq -r .details Container 3045f49c661c85c4289e92cc471a357dc64e2cb1603269e8711dec0992fe9daa is not running Cannot kill container: foo github.com/docker/docker/api/server/router/container.(*containerRouter).postContainersKill /go/src/github.com/docker/docker/api/server/router/container/container_routes.go:267 github.com/docker/docker/api/server/middleware.ExperimentalMiddleware.WrapHandler.func1 /go/src/github.com/docker/docker/api/server/middleware/experimental.go:26 github.com/docker/docker/api/server/middleware.VersionMiddleware.WrapHandler.func1 /go/src/github.com/docker/docker/api/server/middleware/version.go:62 github.com/docker/docker/pkg/authorization.(*Middleware).WrapHandler.func1 /go/src/github.com/docker/docker/pkg/authorization/middleware.go:59 github.com/docker/docker/api/server/middleware.DebugRequestMiddleware.func1 /go/src/github.com/docker/docker/api/server/middleware/debug.go:53 github.com/docker/docker/api/server.(*Server).makeHTTPHandler.func1 /go/src/github.com/docker/docker/api/server/server.go:141 net/http.HandlerFunc.ServeHTTP /usr/local/go/src/net/http/server.go:2047 github.com/docker/docker/vendor/github.com/gorilla/mux.(*Router).ServeHTTP /go/src/github.com/docker/docker/vendor/github.com/gorilla/mux/mux.go:210 net/http.serverHandler.ServeHTTP /usr/local/go/src/net/http/server.go:2879 net/http.(*conn).serve /usr/local/go/src/net/http/server.go:1930 runtime.goexit /usr/local/go/src/runtime/asm_amd64.s:1581 ``` It's worth noting that many errors we currently generate do not (yet) contain a stack-trace, but we could consider updating the `errdefs` helpers to add these. (which could be optional as well, e.g., gated by the same env-var). For example, the `errdefs.NotExist()` does not include a stack-trace (unless the wrapped error is created with one); ```bash curl -s --unix-socket /var/run/docker.sock "http://localhost/v1.41/containers/nosuch/json" | jq -r .details null ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
api_error_trace
branch
from
9dec939 to
f84b3bf
Compare
|
@thaJeztah I'd still love to see this merged so that #43249 can use it. Anything I can do? |
Member
Author
|
Oh! I need to find some time. We discussed this in one of our maintainers meeting and there was a suggestion that it's ok to be able to control this through a header (apparently, it's not uncommon to do so (not sure if a "gate" is needed on the daemon side to allow disabling it, although if someone has access to the docker API, they could already to everything they want). There was also a suggestion from @tonistiigi, who pointed to some code in use by BuildKit that would allow returning a full trace even from grpc responses coming from containerd (I just realised I didn't post the link he provided, but I can ask him). I guess the latter one can be done as a follow up. If you're interested (or know someone who's interested, and a go developer), feel free to work in this (and feel free to take the code in this branch as a starting-point if it's useful) |
|
I'd be happy to get this branch ready to merge, but it's not clear what's missing. Is the header approach preferred? Or are env vars okay? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
relates to #43249 (comment)
This introduces a DOCKER_DEBUG_TRACE which, when set, will propagate a new
detailsfield in error-responses, containing the error's stack-trace (if any).With this patch applied, the daemon can be started with
DOCKER_DEBUG_TRACE=1 dockerdto propagate.detailsfield if the error contains a stacktrace:It's worth noting that many errors we currently generate do not (yet) contain a
stack-trace, but we could consider updating the
errdefshelpers to add these.(which could be optional as well, e.g., gated by the same env-var).
For example, the
errdefs.NotExist()does not include a stack-trace (unless thewrapped error is created with one);
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)