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

AttributeError when using LOGGING_CONFIGURATION_FILE environment variable #4692

Open
matthewelwell opened this issue Oct 3, 2024 · 2 comments · May be fixed by #4693
Open

AttributeError when using LOGGING_CONFIGURATION_FILE environment variable #4692

matthewelwell opened this issue Oct 3, 2024 · 2 comments · May be fixed by #4693
Assignees
Labels
bug Something isn't working

Comments

@matthewelwell
Copy link
Contributor

Using the minimal docker-compose.yml file and logging.json configuration file provided below, it is possible to generate the following error.

Traceback (most recent call last):
2024-10-03T14:41:45.387328675Z   File "/usr/local/bin/gunicorn", line 8, in <module>
2024-10-03T14:41:45.387331633Z     sys.exit(run())
2024-10-03T14:41:45.387333550Z              ^^^^^
2024-10-03T14:41:45.387335133Z   File "/build/.venv/lib/python3.11/site-packages/gunicorn/app/wsgiapp.py", line 67, in run
2024-10-03T14:41:45.387366883Z     WSGIApplication("%(prog)s [OPTIONS] [APP_MODULE]", prog=prog).run()
2024-10-03T14:41:45.387376050Z   File "/build/.venv/lib/python3.11/site-packages/gunicorn/app/base.py", line 236, in run
2024-10-03T14:41:45.387466966Z     super().run()
2024-10-03T14:41:45.387477966Z   File "/build/.venv/lib/python3.11/site-packages/gunicorn/app/base.py", line 72, in run
2024-10-03T14:41:45.387480091Z     Arbiter(self).run()
2024-10-03T14:41:45.387481591Z     ^^^^^^^^^^^^^
2024-10-03T14:41:45.387483008Z   File "/build/.venv/lib/python3.11/site-packages/gunicorn/arbiter.py", line 58, in __init__
2024-10-03T14:41:45.387484550Z     self.setup(app)
2024-10-03T14:41:45.387485966Z   File "/build/.venv/lib/python3.11/site-packages/gunicorn/arbiter.py", line 93, in setup
2024-10-03T14:41:45.387546508Z     self.log = self.cfg.logger_class(app.cfg)
2024-10-03T14:41:45.387551925Z                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-10-03T14:41:45.387553508Z   File "/build/.venv/lib/python3.11/site-packages/gunicorn/glogging.py", line 194, in __init__
2024-10-03T14:41:45.387792425Z     self.setup(cfg)
2024-10-03T14:41:45.387798925Z   File "/app/util/logging.py", line 60, in setup
2024-10-03T14:41:45.388940091Z     if settings.LOG_FORMAT == "json":
2024-10-03T14:41:45.388955758Z        ^^^^^^^^^^^^^^^^^^^
2024-10-03T14:41:45.388957800Z   File "/build/.venv/lib/python3.11/site-packages/django/conf/__init__.py", line 83, in __getattr__
2024-10-03T14:41:45.388984050Z     val = getattr(self._wrapped, name)
2024-10-03T14:41:45.388990758Z           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-10-03T14:41:45.389075466Z AttributeError: 'Settings' object has no attribute 'LOG_FORMAT'. Did you mean: 'DATE_FORMAT'?

Example docker-compose file

volumes:
  pgdata:

services:
  postgres:
    image: postgres:15.5-alpine
    environment:
      POSTGRES_PASSWORD: password
      POSTGRES_DB: flagsmith
    container_name: flagsmith_postgres
    volumes:
      - pgdata:/var/lib/postgresql/data
    healthcheck:
      test: ['CMD-SHELL', 'pg_isready -d flagsmith -U postgres']
      interval: 2s
      timeout: 2s
      retries: 20
      start_period: 20s

  flagsmith:
    image: flagsmith.docker.scarf.sh/flagsmith/flagsmith:latest
    environment:
      DATABASE_URL: postgresql://postgres:password@postgres:5432/flagsmith
      LOGGING_CONFIGURATION_FILE: /app/logging.json
    ports:
      - 8000:8000
    healthcheck:
      test: ['CMD-SHELL', 'python /app/scripts/healthcheck.py']
      interval: 2s
      timeout: 2s
      retries: 20
      start_period: 20s
    depends_on:
      postgres:
        condition: service_healthy
    volumes:
      - "/tmp/flagsmith-logging/logging.json:/app/logging.json"
      - "/tmp/flagsmith-logging/logs/api/:/var/log"

Example logging.json file

{
    "version": 1,
    "disable_existing_loggers": false,
    "formatters": {
        "generic": {"format": "%(name)-12s %(levelname)-8s %(message)s"},
        "json": {
            "()": "util.logging.JsonFormatter",
            "datefmt": "%Y-%m-%d %H:%M:%S"
        }
    },
    "handlers": {
        "file": {
            "level": "INFO",
            "class": "logging.FileHandler",
            "formatter": "json",
            "filename": "/var/log/flagsmith.log"
        }
    },
    "loggers": {
        "": {"level": "INFO", "handlers": ["file"]}
    }
}
@matthewelwell matthewelwell added the bug Something isn't working label Oct 3, 2024
@matthewelwell
Copy link
Contributor Author

The issue here is that the run-docker.sh (as well as the makefile) default to using the GunicornJsonCapableLogger as per this line of code.

Perhaps a simple solution would be to pull the LOG_FORMAT outside of the conditional logic here, but I'm not certain if that's the desired behaviour. It seems like we have a bit of a tangle here? I think that the --logger-class argument passed to gunicorn controls both the application logs and the access logs. However, since the application logs should be controlled by the configuration file passed in, maybe we need to split out the LOG_FORMAT setting into multiple variables?

@khvn26
Copy link
Member

khvn26 commented Oct 3, 2024

GunicornJsonCapableLogger only exists to cater for the LOG_FORMAT setting, therefore I don't see a need to modify logic anywhere.

I think it's important to mention that the logger class is overridable via the GUNICORN_LOGGER_CLASS environment variable.

Ideally, it should fallback to default gunicorn logger when LOGGING_CONFIGURATION_FILE is set. This fix is essentially it.

The root problem that we might want to solve or not is that logging should be configured outside of Django settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants