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

No output in the body when using Django dev server #16

Closed
fabMrc opened this issue Mar 9, 2021 · 17 comments
Closed

No output in the body when using Django dev server #16

fabMrc opened this issue Mar 9, 2021 · 17 comments

Comments

@fabMrc
Copy link

fabMrc commented Mar 9, 2021

I am trying runitor and I notice log attachment is not working in the last 0.7.0
I succeed in attaching logs with curl command
I have a self provisioned hosted instance of healtcheck.io

 m=$(/bin/sh /data/scripts/pgsql_backup.sh -r sftp:backup@backup.openstack.local:/backup/restic/cachet/dev -p /data/cachet/dump -c cachet-db)
curl -fsS -m 10 --retry 5 --data "$m" https://healthcheck.openstack.local/ping/0b34abd2-1e0b-4620-acc0-1255a087ebf5

is working

but :

./runitor -uuid 0b34abd2-1e0b-4620-acc0-1255a087ebf5 -api-url https://healthcheck.openstack.local/ping -- /bin/sh /data/scripts/pgsql_backup.sh -r sftp:backup@backup.openstack.local:/backup/restic/cachet/dev -p /data/cachet/dump -c cachet-db

does not attach anything

@lakemike
Copy link

lakemike commented Mar 9, 2021

I have the same issue since v0.7 beta, also with a self-hosted healthcheck.io instance. So far, I am back to v0.6 stable.

@fabMrc
Copy link
Author

fabMrc commented Mar 9, 2021

Yes I read your issue yesterday
0.6.0 is working nice

@bdd
Copy link
Owner

bdd commented Mar 9, 2021

Yes I read your issue yesterday

0.6.0 is working nice

If you read #14, then you saw the simple web server pasted to debug the operation. Can you by any chance run it to see if runitor is sensing the body?

Do you have an option to dump the entire HTTP session on the receiver side (the instance that handles requests to healthchecks.openstack.local)

I tried to reproduce the issue running a hosted HC instance locally in a container, and then on another machine in the same network, but couldn't reproduce the behavior you and @lakemike report.

Maybe we can ping @cuu508 to give us some hot debugging tips.

@cuu508
Copy link
Contributor

cuu508 commented Mar 9, 2021

Looks like I can reproduce it when testing against a development version of Healthchecks running locally (manage.py runserver).

runitor 0.6.0 sends the Content-Length header but runitor 0.7.0 sends Transfer-Encoding: chunked.

I suspect the Django development server doesn't like "chunked"

0.6.0:

image

0.7.0:

image

@lakemike, @fabMrc what Dockerfile are you using?

@cuu508
Copy link
Contributor

cuu508 commented Mar 9, 2021

Pretty sure it has something to do with the chunked encoding.

Command:

runitor-v0.7.0-linux-amd64 -uuid 3b0aabaa-9c33-4974-963a-c334dee68c66 --api-url http://localhost:8000/ping  -no-start-ping -- echo hi

If I use "Follow TCP stream" in Wireshark, I can see the chunk sizes sent by the client (3 and 0):

image

And the server complains about "3" – looks like it doesn't recognize it.

@bdd
Copy link
Owner

bdd commented Mar 9, 2021

Thanks for the helpful pointers @cuu508.

No wonder I couldn't reproduce this because local container was behind Caddy and the remote install was behind Nginx.

It's a bummer Django's dev server doesn't fully support HTTP RFC but unless I'm overlooking a fact, runitor doesn't need to stream the request either. The decision to do chunked encoding comes from Go's HTTP library (here exactly https://golang.org/src/net/http/transfer.go#L167). I'll run a few sessiosn under debugger to see what underlying change from 0.6.0 to 0.7.0 changed code flow to take TE: chunked path and see if I can quickly publish a 0.7.1.

@bdd
Copy link
Owner

bdd commented Mar 9, 2021

And the server complains about "3" – looks like it doesn't recognize it.

Yet it returns HTTP 200 as the status while the body mentions and HTTP 400 Bad Request?

@bdd
Copy link
Owner

bdd commented Mar 9, 2021

Request preparation goes through a type switch at https://golang.org/src/net/http/request.go#L888.

With 0.7.0, the ping body is read from a ring buffer to implement tailing with limited resources. Because it doesn't match this limited type switch, library proceeds with assumption of not being able to deduce a header send time content length, resorting to transfer encoding.

I have two options: I either introduce a type switch in api.go:APIClient.Post for *internal.RingBuffer type, or in the main, I convert the ringbuffer to a *strings.Reader type, which will surely cause one more copy, but granted it's just 10K in most cases, it's nothing important.

I'll get back to this later today.

bdd added a commit that referenced this issue Mar 10, 2021
Go's net/http/request.go cannot deduce the body size if what's
behind io.Reader isn't a *bytes.{Buffer,Reader} or *strings.Reader.
*internal.RingBuffer merely implements the io.Reader interface, with
no way of hinting available data length. This causes requests to use
chunked Transfer-Encoding.

Apparently Django's development server doesn't support chunked
encoding but still returns HTTP 200 while the body says HTTP 400...
To cater to Healthchecks private instances served this way, we do a
type assertion for *internal.RingBuffer, and manually set content
length. This should result a request with a Content-Length header
and no Transfer-Encoding.

Fixes #14 and #16
@bdd
Copy link
Owner

bdd commented Mar 10, 2021

@cuu508 @fabMrc @lakemike can you try with 0.7.1-beta.1 binaries?

See the commit above, it's essentially a two line change, explicitly setting the content length to prevent conversion to chunked encoding.

I only tested this with tcpdump. I'd appreciate a 👍 or 👎 with Django devel server setup.

@bdd bdd changed the title Attaching logs is not working No output in the bosy when using Django dev server Mar 10, 2021
@bdd bdd changed the title No output in the bosy when using Django dev server No output in the body when using Django dev server Mar 10, 2021
@cuu508
Copy link
Contributor

cuu508 commented Mar 10, 2021

0.7.1-beta.1 works here with the Django dev server:

image

image

@fabMrc
Copy link
Author

fabMrc commented Mar 10, 2021

Fixed it works nice I receive log in email body and it is recorded in healtcheckio

@cuu508
Copy link
Contributor

cuu508 commented Mar 10, 2021

@fabMrc are using Django development server in your setup? If so, please consider switching to gunicorn or uwsgi. From Django docs:

DO NOT USE THIS SERVER IN A PRODUCTION SETTING. It has not gone through security audits or performance tests. (And that’s how it’s gonna stay. We’re in the business of making Web frameworks, not Web servers, so improving this server to be able to handle a production environment is outside the scope of Django.)

@fabMrc
Copy link
Author

fabMrc commented Mar 10, 2021

No I am using uwsgi

@lakemike
Copy link

This fixed it. Thank you very much! 👍

@cuu508
Copy link
Contributor

cuu508 commented Mar 10, 2021

No I am using uwsgi

TIL uwsgi doesn't yet automagically support chunked. It supports reading chunked request body using uwsgi-specific API. And there's a wsgi-manage-chunked-input configuration option coming, but looks like it is not here yet.

@bdd
Copy link
Owner

bdd commented Mar 10, 2021

Alright, v0.7.1 is out for the two (maybe three) people in the world, running their instances behind toy servers and want to use runitor ;)

@fabMrc
Copy link
Author

fabMrc commented Mar 10, 2021

haha nice to be a VIP user ^^ I am using linuxserver/healthckeckio docker image

@bdd bdd closed this as completed Mar 10, 2021
bdd added a commit that referenced this issue Nov 21, 2021
When we were handling implicit conversion to chunked encoding due to
ring buffer use in #16, we missed one other case client reverts to
chunked encoding: empty request body.

Since Go 1.8*, the ambiguity of default zero value for `ContentLength`
or explicit setting to zero must be handled with explicit passing of
`http.NoBody` or `nil`  to avoid use of chunked encoding.

Pings with empty bodies--i.e. command with no stdout/err continued to
result chunked encoding.

Start pings were not affected because instead of an empty ring buffer,
they always had `nil` passed as their bodies.

* https://golang.org/doc/go1.8#net_http
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

No branches or pull requests

4 participants