Fix/Feature: Nginx respect logger configuration (sometimes)

[skrashevich]

resolve #6828

[netlify]

✅ Deploy Preview for frigate-docs canceled.

Name	Link
🔨 Latest commit	5049c91
🔍 Latest deploy log	https:https://app.netlify.com/sites/frigate-docs/deploys/6490c73bc80f360008a17df9

[NickM-27]

This is not going to work, it will fragment the error log and access log meaning a user can only have one or the other but not both enabled at the same time.

Also, the S6 logger is still enabled which would likely cause conflicts

[skrashevich]

This is not going to work, it will fragment the error log and access log meaning a user can only have one or the other but not both enabled at the same time.

I quickly understood it and rolled out the fix)

Also, the S6 logger is still enabled which would likely cause conflicts

S6 logger watch to stdout/stderr. When there's nothing in them, it doesn't write anything

[skrashevich]

NB: Nginx, when it receives an HUP signal, not only reload its configs, but also re-opens file descriptors, including logs

[NickM-27]

I think it would be preferable to have this logic before NGINX starts in the S6 script.

[trunet]

I think it would be preferable to have this logic before NGINX starts in the S6 script.

Because of this:
https://github.com/blakeblackshear/frigate/blob/dev/docker/rootfs/etc/s6-overlay/s6-rc.d/nginx/dependencies.d/frigate

nginx only starts after frigate is up I think.

[NickM-27]

I think it would be preferable to have this logic before NGINX starts in the S6 script.

Because of this: https://github.com/blakeblackshear/frigate/blob/dev/docker/rootfs/etc/s6-overlay/s6-rc.d/nginx/dependencies.d/frigate

nginx only starts after frigate is up I think.

that's fine, I still think it should be done in the nginx s6 script and not in the frigate code. Same as how go2rtc reads from the frigate config and defines its own config to use

[trunet]

Can a new config be added like nginx_access_log: false (default being true) under logger? if yes, a simple grep and sed on the supervisor run script would do the trick.

[NickM-27]

Or just nginx with the same options as the other logs and anything higher than info disables the access log.

[trunet]

We can use something like python -c 'import yaml, sys; yaml.safe_load(sys.stdin); [parsing code]' < config.yml to parse the config yaml, get the default logger option and sys.exit(0) or sys.exit(1) and if it to sed nginx.conf.

[NickM-27]

I don't think python should be used for that, there is already a good example in frigate itself

frigate/docker/rootfs/etc/s6-overlay/s6-rc.d/frigate/run

Lines 12 to 43 in 9e531b0

	function migrate_db_path() {
	# Find config file in yaml or yml, but prefer yaml
	local config_file="${CONFIG_FILE:-"/config/config.yml"}"
	local config_file_yaml="${config_file//.yml/.yaml}"
	if [[ -f "${config_file_yaml}" ]]; then
	config_file="${config_file_yaml}"
	elif [[ ! -f "${config_file}" ]]; then
	echo "[ERROR] Frigate config file not found"
	return 1
	fi
	unset config_file_yaml
	# Use yq to check if database.path is set
	local user_db_path
	user_db_path=$(yq eval '.database.path' "${config_file}")
	if [[ "${user_db_path}" == "null" ]]; then
	local previous_db_path="/media/frigate/frigate.db"
	local new_db_dir="/config"
	if [[ -f "${previous_db_path}" ]]; then
	if mountpoint --quiet "${new_db_dir}"; then
	# /config is a mount point, move the db
	echo "[INFO] Moving db from '${previous_db_path}' to the '${new_db_dir}' dir..."
	# Move all files that starts with frigate.db to the new directory
	mv -vf "${previous_db_path}"* "${new_db_dir}"
	else
	echo "[ERROR] Trying to migrate the db path from '${previous_db_path}' to the '${new_db_dir}' dir, but '${new_db_dir}' is not a mountpoint, please mount the '${new_db_dir}' dir"
	return 1
	fi
	fi
	fi
	}

[trunet]

I don't think python should be used for that, there is already a good example in frigate itself

Nice to have yq available.

[skrashevich]

that's fine, I still think it should be done in the nginx s6 script and not in the frigate code. Same as how go2rtc reads from the frigate config and defines its own config to use

There is a nuance here: the wrong configuration when starting nginx will not allow it to start, but if it is changed after launch, it simply will not be applied, but the services will continue to work

And I want to note that changing the configuration without restarting was one of the main features of nginx from the beginning of the nginx project

[NickM-27]

I still think this is a very simple change to the file which can be done before starting nginx as frigate process itself does not need to worry about managing the other processes (which is the job of S6).

In any case, the current implementation here still lacks the ability for the user to view a unified log of both access and error logs at the same time if it was enabled that way. Currently you can only have access logs or error logs.

I think a much simpler solution would be to leave both access_log and error_log printing to stdout and simply comment out the access log file in the case that access logs are disabled by the user

[skrashevich]

I still think this is a very simple change to the file which can be done before starting nginx as frigate process itself does not need to worry about managing the other processes (which is the job of S6).

In this case, we need to decide who should be the general manager of all logs in the project. It's necessary to put the logs of all processes in one place, and take them from this place with corresponding filters/settings

[NickM-27]

I still think this is a very simple change to the file which can be done before starting nginx as frigate process itself does not need to worry about managing the other processes (which is the job of S6).

In this case, we need to decide who should be the general manager of all logs in the project. It's necessary to put the logs of all processes in one place, and take them from this place with corresponding filters/settings

If I understand you correctly then I disagree with that last part. Each service (nginx, go2rtc, frigate) has its own configurations for logs and controls the filtering / behavior of what logs are output on its own. Once these logs are output to stdout S6 manages the logs and keeps them in SHM.

This doesn't need to be so complicated, S6 script that starts nginx can read the frigate config and adjust its own behavior accordingly just like scripts that start frigate and go2rtc do already. That way it fits in with the current architecture.

[vertex-github]

Did this get implemented? Id also like to disable the access logs from my docker container if possible.

[NickM-27]

/stats and /version were removed from the access logs. There is no official way to disable all access logs