bdd / runitor

A command runner with healthchecks.io integration
BSD Zero Clause License
274 stars 15 forks source link

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

Closed fabMrc closed 3 years ago

fabMrc commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Yes I read your issue yesterday 0.6.0 is working nice

bdd commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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.

cuu508 commented 3 years ago

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

image

image

fabMrc commented 3 years ago

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

cuu508 commented 3 years ago

@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 commented 3 years ago

No I am using uwsgi

lakemike commented 3 years ago

This fixed it. Thank you very much! 👍

cuu508 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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