Skip to content

Feat: Added support for switching to ECS conformant JSON logging. - #23

Open
buddemat wants to merge 3 commits into
mainfrom
slb/mbu
Open

Feat: Added support for switching to ECS conformant JSON logging.#23
buddemat wants to merge 3 commits into
mainfrom
slb/mbu

Conversation

@buddemat

@buddemat buddemat commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Goal: Make cadenzaanalytics logging configurable to emit Elastic Common Schema (ECS) JSON, suitable for container log aggregation, covering the library itself, dependent packages, and analytics extension code — with minimal burden on extension developers.

Changes:

  • moved loggin to new file src/cadenzaanalytics/logging_config.py
  • added CadenzaEcsFormatter as subclass of ecs_logging.StdlibFormatter and stamping every record with service.name/service.version
  • explicitly re-pointting gunicorn.error/gunicorn.access at the same handler
  • documentation and tests

logging.getLogger('some.dependency').warning('dependency warning')

record = json.loads(capsys.readouterr().err.strip())
assert record['message'] == 'dependency warning'

@DanDits DanDits Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that we don't yet have any unittest dependency. These raw asserts are not really built for tests. See for example https://docs.python.org/3/library/unittest.html
Do you see a chance to migrate our few tests?
Edit: I checked, pytest seems to be completely fine with the basic "assert". Let's keep it simple then and leave it as is.

f'Invalid CADENZAANALYTICS_LOG_FORMAT "{log_format}". Supported values are "plain" and "ecs".'
)

log_level = os.environ.get('CADENZAANALYTICS_LOG_LVL', 'INFO')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how flexible the logging config is, but maybe an ".upper()" call might still be safer

@DanDits

DanDits commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Looks fine, I like having the two options and an easy switch so local development is not affected. Looks like you tried to work around some gunicorn quirks at best effort. Do the mentioned non-json lines that might be logged bother a potential json-expecting log consumer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants