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

Restapi 1453 commands log #23

Closed
wants to merge 21 commits into from
Closed

Restapi 1453 commands log #23

wants to merge 21 commits into from

Conversation

iB0nes
Copy link
Collaborator

@iB0nes iB0nes commented Mar 2, 2025

List of changes with description:

Upgraded existing JSON logger to f7t_tracing_log features:

  • separated logs for API call (middleware) and command execution (SSH, filesystem, scheduler);
  • the command log is compatible to v1 to integrate existing dashboards and statistics (tested with Kibana dashboard);
  • all the commands have been labeled to properly trace the execution;
  • information extracted from the request (endpoint, routers, methods) have been normalized in order to log the same data with or without a frontend (e.g Docker Compose vs K8s with API gateway), almost in theory, tested with the available environments at CSCS;
  • a trace ID is automatically created for each request (UUID4), it is used as a field in all the messages of f7t_tracing_log and it is returned in the header section of the response to the user. The ID allow to trace all the log events invoked for a single request, whatever they are;
  • the f7t_tracing_log can be enabled or disabled in the standard F7Tv2 configuration file, I created a section with the related classes. I tried to access the configuration directly from the logger module, but due some "circular imports" errors, I should revert that to explicitly configure the logger with an initialization function called in main.py.

Added new general-purpose/debugging log f7tlog:

  • logging message for general usage labeled F7TLOG independently from the Uvicorn logs;
  • the level of logger can be selected with the standard configuration file, default is 'DEBUG' but id can be set to whatever level is allowed by the Python's logging module

Loggers configuration file:

  • the loggers are configured using a YAML that is passed to Uvicorn as a parameter.
  • my first attempt was to keep the file configurable by the final user deploying in the K8s config map. The problem that it is not possible to parameterize the file passed to Uvicorn's CMD in the Dockerfile and the fact that the naming of the loggers are tightly coupled with the Python code (and we are forced to do it like this) , made me change to a default configuration file /build/environment/firecrest-api-config/log-config.yaml embedded into the Dockerfile. It can however be overwritten at deploy time, or changed at build time

During the work, I found some tests failing because of errors with "Circular module import". After some debugging, I found a way to solve this, but I also noticed that there was some duplication with firecrest.config import Settings vs firecrest.plugin import settings in both main.py and tests/conftest.py. I tried to cleanup the duplication using the same approach used for any routers. Now they should be aligned, but actually I don't know if that matches the architectural choices.

Thank you.

@iB0nes iB0nes requested review from theely and jpdorsch March 2, 2025 16:24
@iB0nes iB0nes self-assigned this Mar 2, 2025
@iB0nes iB0nes closed this Mar 11, 2025
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.

1 participant