Support crt logs in 'logging' module#728
Conversation
| /* Fill header */ | ||
| struct s_py_log_header *header = (struct s_py_log_header *)output->bytes; | ||
| header->level = level; | ||
| header->thread_name_len = thread_name.len; | ||
| header->subject_len = subject_name.len; |
There was a problem hiding this comment.
wow, this is hacky, I spent 5 mins to understand this...
| _awscrt.set_log_level(int(crt_level)) | ||
|
|
||
|
|
||
| def logf(level: int, subject: LogSubject, message: str): |
There was a problem hiding this comment.
why naming it as logf? in Java, we name it just log. I believe f stardards for format and google told me:
Log: Takes a variable number of arguments and prints them (similar to fmt.Println).
Logf: Takes a format string and arguments, allowing for formatted output using placeholders like %d or %s (similar to fmt.Printf).
And here we just take a message and subject, I think it's better to name it to log instead.
There was a problem hiding this comment.
I only called it logf because we are directly calling AWS_LOGF() i some sense.. but yes we can use log as well... but it would also conflict with logger.log() is that a problem we need to worry about?
There was a problem hiding this comment.
I don't think so, it's like we have two init_logging method...
| if (!impl) { | ||
| return PyErr_AwsLastError(); | ||
| } |
There was a problem hiding this comment.
stops checking for allocation failures.
| #include <memoryobject.h> | ||
|
|
||
| static struct aws_logger s_logger; | ||
| static bool s_logger_init = false; |
There was a problem hiding this comment.
this should also be atomic. Given freethreading python
| if (!buf) { | ||
| va_end(args); | ||
| return AWS_OP_ERR; | ||
| } |
There was a problem hiding this comment.
no need to check for allocation failures,
|
|
||
| struct aws_string *thread_name_str = NULL; | ||
| aws_thread_current_name(logger->allocator, &thread_name_str); | ||
| const char *thread_name = thread_name_str ? aws_string_c_str(thread_name_str) : "main"; |
There was a problem hiding this comment.
are we using main as the default?
But, why in your example in the description there is python as the thread name? Are we getting the python from our aws_thread_current_name???
2026-04-14 17:44:38,450 - python - awscrt.http.connection-manager - INFO - id=0x556489b95560: Successfully created
I think we should follow what ever thread name python uses. Maybe that if we are not getting a thread_name from our function, we just don't change the thread name from the callback? So, passing null or something
| return NULL; | ||
| } | ||
|
|
||
| if (s_logger_init) { |
There was a problem hiding this comment.
with atomic, you will need fetch and flip to be thread safe.(compare and set https://stackoverflow.com/questions/14370575/why-are-atomic-operations-considered-thread-safe)
2 participants
Issue #, if available:
Description of changes:
CRT log messages can now be routed through Python's standard logging module via the awscrt.logging module.
Usage:
Note: Existing awscrt.io.init_logging() is retained for backward compatibility, however CRT cannot publish to multiple subscribers, so only one of the loggers can be initialized.
Log Example:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.