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.

Sign up for GitHub

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

[Linux] Add mechanism to redirect CHIP log other than stdout #33338

Closed
wants to merge 1 commit into from
Closed

[Linux] Add mechanism to redirect CHIP log other than stdout #33338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions src/platform/Linux/Logging.cpp
View file
Open in desktop
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why is this Linux-specific?

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,27 @@ void __attribute__((weak)) OnLogOutput() {}
namespace Logging {
namespace Platform {

// File pointer for the log file
FILE * logFile = nullptr;

void OpenLogFile(const char * filePath)
{
logFile = fopen(filePath, "a");
if (logFile == nullptr)
{
perror("Failed to open log file");
Copy link
Contributor

Choose a reason for hiding this comment

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

This must VerifyOrDie.

}
}

void CloseLogFile()
{
if (logFile != nullptr)
{
fclose(logFile);
logFile = nullptr;
}
}

/**
* CHIP log output functions.
*/
Expand All @@ -44,17 +65,19 @@ void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char
gettimeofday(&tv, nullptr);

#if !CHIP_USE_PW_LOGGING
// Lock standard output, so a single log line will not be corrupted in case
FILE * outputStream = (logFile == nullptr) ? stdout : logFile;
// Lock outputStream, so a single log line will not be corrupted in case
// where multiple threads are using logging subsystem at the same time.
flockfile(stdout);
flockfile(outputStream);

printf("[%" PRIu64 ".%06" PRIu64 "][%lld:%lld] CHIP:%s: ", static_cast<uint64_t>(tv.tv_sec), static_cast<uint64_t>(tv.tv_usec),
static_cast<long long>(syscall(SYS_getpid)), static_cast<long long>(syscall(SYS_gettid)), module);
vprintf(msg, v);
printf("\n");
fflush(stdout);
fprintf(outputStream, "[%" PRIu64 ".%06" PRIu64 "][%lld:%lld] CHIP:%s: ", static_cast<uint64_t>(tv.tv_sec),
static_cast<uint64_t>(tv.tv_usec), static_cast<long long>(syscall(SYS_getpid)),
static_cast<long long>(syscall(SYS_gettid)), module);
vfprintf(outputStream, msg, v);
fprintf(outputStream, "\n");

funlockfile(stdout);
fflush(outputStream);
funlockfile(outputStream);
#else // !CHIP_USE_PW_LOGGING
char formattedMsg[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
snprintf(formattedMsg, sizeof(formattedMsg),
Expand Down
19 changes: 19 additions & 0 deletions src/platform/logging/LogV.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,25 @@ namespace Platform {
*/
void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v);

/**
* Open a log file for appending log messages. This function opens
* the specified file and ensures that subsequent logs are written to
* this file. It should be called at the start of the application
* before any logging is done.
*
* @param[in] filePath The path to the log file. If the file does not
* exist, it will be created. If it exists, log
* messages will be appended to it.
*/
void OpenLogFile(const char * filePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have a list of log sinks (one of which is stdout by default) rather than assume a single place where logs go. Otherwise it is impossible to have stdout + output to file logs.

Copy link
Contributor

Choose a reason for hiding this comment

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

We do currently allow setting one log sink (the logging redirector setup), which can then be used to multiplex as desired without changing any platform implementations....

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/project-chip/connectedhomeip/blob/master/src/lib/support/logging/TextOnlyLogging.h#L359 contains a SetLogRedirectCallback where you can move your logging to go anywhere. This is what python uses to redirect to python logging instead of just console.

Could we use this for the CLI bits instead? One could even imagine some term-linke thing where you can both do cli-shell-interact and view the log in a separate pane...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this method works.


/**
* Close the log file if it is open. This function should be called
* at the end of the application to ensure that all log messages are
* flushed to disk and the file is properly closed.
*/
void CloseLogFile();
Comment on lines +43 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these called?


} // namespace Platform
} // namespace Logging
} // namespace chip
Loading