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

api: add DOCKER_DEBUG_TRACE to return stack-traces in API error responses #43252

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

relates to #43249 (comment)

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:

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);

curl -s --unix-socket /var/run/docker.sock "http://localhost/v1.41/containers/nosuch/json" | jq -r .details
null

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

…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 <github@gone.nl>
@dlowe
Copy link

dlowe commented Apr 12, 2022

@thaJeztah I'd still love to see this merged so that #43249 can use it. Anything I can do?

@thaJeztah
Copy link
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)

@dlowe
Copy link

dlowe commented Apr 13, 2022

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants