search

getsentry / self-hosted

Sentry, feature-complete and packaged up for low-volume deployments and proofs-of-concept
https://develop.sentry.dev/self-hosted/
Other
7.83k stars 1.76k forks source link

Unnecessary logging request in nginx #1208

Open aminvakil opened 2 years ago

aminvakil commented 2 years ago

Problem Statement

Official guidelines suggests and everyone in production uses a reverse proxy in front of sentry nginx (I guess), and logging should be done on the proxy instead of on docker IMO, so I'm not sure if logging in nginx container again is useful (unless on debugging).

Solution Brainstorm

Add a variable to disable nginx logging in container (which is enabled by default).

aminvakil commented 2 years ago

OK, so as nginx.conf is mounted read-only on nginx, there isn't anything that could be done now after container is up (look-up a variable and change configuration according to it).

Now we should either remove read_only: true line from docker-compose or ...?

chadwhitacre commented 2 years ago

there isn't anything that could be done now after container is up

Why does there need to be? Won't people stop and restart containers while upgrading?

aminvakil commented 2 years ago

Sorry I should have been clearer. I didn't find an environment variable which can be set in nginx.conf so that it sets access.log on or off, so I was more thinking of a sed in docker-compose command so it can change the line configuring access.log if it was set to be off.

So anyone can change a variable in .env or .env.custom and it gets done, not to force users to change nginx.conf manually.

glensc commented 2 years ago

see "Using environment variables in nginx configuration (new in 1.19)"

nginx entrypoint supports envsubst: