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

Sarc 368 loki connect #139

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Sarc 368 loki connect #139

wants to merge 34 commits into from

Conversation

nurbal
Copy link
Collaborator

@nurbal nurbal commented Nov 5, 2024

This PR adds an opentelemetry logging handler that sends logs to a loki endpoint.
Related settings added in the logging section of the settings JSON file:

    "logging": {
        "log_level": "INFO",
        "OTLP_endpoint": "http://path.to.loki.endpoint/otlp/v1/logs",
        "service_name": "sarc"
    },

@nurbal nurbal marked this pull request as ready for review January 27, 2025 01:53
@nurbal
Copy link
Collaborator Author

nurbal commented Jan 27, 2025

J'ai renoncé à tester de bout en bout le logger vers loki, de part la difficulté à utiliser logging au sein d'un test (pytest intercepte le logging, et j'ai tout essayé pour désactiver cela sans succès). Au lieu de cela, un test plus restreint du logging handler de opentelemetry vers loki est mis en place, avec un httpserver pour mocker le endpoint.

@nurbal nurbal requested a review from bouthilx February 11, 2025 16:10
sarc/logging.py Outdated
config_log_level = logging_levels.get(
config().logging.log_level, logging.WARNING
)
verbose_log_level = verbose_levels.get(verbose_level, config_log_level)
Copy link
Member

Choose a reason for hiding this comment

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

The problem now is that verbose_level will always override the config log level even if set with the command line. A solution would be that verbose_level is set to None when not specified in cmdline.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Damn, you're right.
Fixed in ab430fe
Command-line has priority if specified (-v or -vv); config-file log level is used otherwise.

@nurbal nurbal requested a review from bouthilx February 14, 2025 15:37
@nurbal nurbal mentioned this pull request Feb 17, 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.

2 participants