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

Automatically configure the Gunicorn web server #4

Open
edmorley opened this issue Feb 28, 2023 · 2 comments
Open

Automatically configure the Gunicorn web server #4

edmorley opened this issue Feb 28, 2023 · 2 comments
Assignees
Labels
classic buildpack parity Features required for parity with the classic Heroku Python buildpack

Comments

@edmorley
Copy link
Member

edmorley commented Feb 28, 2023

The classic Heroku Python buildpack automatically configures gunicorn at runtime (it sets FORWARDED_ALLOW_IPS and GUNICORN_CMD_ARGS):
https://github.com/heroku/heroku-buildpack-python/blob/main/vendor/python.gunicorn.sh

Something similar to that should be added to the Python CNB for parity with the classic buildpack.

Internal tracking epic

@edmorley edmorley added enhancement New feature or request classic buildpack parity Features required for parity with the classic Heroku Python buildpack and removed enhancement New feature or request labels Feb 28, 2023
@edmorley
Copy link
Member Author

edmorley commented Nov 26, 2024

Notes:

  • FORWARDED_ALLOW_IPS is used by both gunicorn and uvicorn. Setting it to * is environment-specific (since it requires knowing there is a LB/proxy that's correctly setting headers), and forgetting to set it (eg in the app's own config) can result in confusing behaviour (eg HTTP -> HTTPS redirect loops). Therefore we should continue to do so in the CNB.
  • Setting --access-logfile - in GUNICORN_CMD_ARGS has caused confusion in customer tickets for the classic buildpack (example; since it's not obvious how the access logs are being enabled, or how to override that. Plus the default access log format includes query strings which can in some cases contain tokens/secrets). Given the Heroku router already emits access logs, it feels like adding additional gunicorn-level access logs should be an app concern, and not something the buildpack magically configures. So I think we should drop this for the CNB, and update the Python getting started guide's gunicorn.conf.py to demonstrate enabling access logs (and for bonus points, how to change the format).

@edmorley edmorley self-assigned this Nov 26, 2024
edmorley added a commit to heroku/python-getting-started that referenced this issue Dec 8, 2024
* Explicitly enable the access log (since currently it's only enabled
  when using the classic Python buildpack, and not locally or when
  using the Python CNB).
* Customise the gunicorn access log format to make it use the Heroku
  logfmt style, and to improve which fields are included.

Before:

```
2024-12-08T16:23:33.457957+00:00 app[web.1]: ::ffff:10.1.50.245 - - [08/Dec/2024:16:23:33 +0000] "GET / HTTP/1.1" 200 9585 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:134.0) Gecko/20100101 Firefox/134.0"
```

After:

```
2024-12-08T16:32:47.618296+00:00 app[web.1]: gunicorn method=GET path="/" status=200 duration=14ms request_id=e45f9c86-d6d8-4b79-937e-18c0b6bf859e fwd="123.456.789.0" user_agent="Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:134.0) Gecko/20100101 Firefox/134.0"
```

Ref heroku/buildpacks-python#4.
GUS-W-17316745.
@edmorley
Copy link
Member Author

edmorley commented Dec 8, 2024

  • So I think we should drop this for the CNB, and update the Python getting started guide's gunicorn.conf.py to demonstrate enabling access logs (and for bonus points, how to change the format).

The getting started guide now configures access logs explicitly (and with an improved log format):
heroku/python-getting-started#246

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
classic buildpack parity Features required for parity with the classic Heroku Python buildpack
Projects
None yet
Development

No branches or pull requests

1 participant