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

logfile: exhance internal dlt logging by introducing size limits #369

Merged

Conversation

danielweber2018
Copy link
Contributor

Enhance dlt logging such that multiple files are used as it is done
for the offline traces. Add limit-specific config values for logging.
For this purpose the pattern of index-based file names is used only.
This approach of logging to multiple files and rotating in order
to keep the limits ensures that dlt logs take care of available
space on the underlying file system and do not grow infinitely.

The program was tested solely for our own use cases, which might differ from yours.
Licensed under Mozilla Public License Version 2.0

Daniel Weber, [email protected], Daimler TSS GmbH, imprint

@minminlittleshrimp
Copy link
Collaborator

minminlittleshrimp commented Dec 1, 2022

Hello @danielweber2018
If you still interest in this feature for dlt-daemon logging to file mode,
I am working on and also finding my PR is a duplicated version to yours!
I would like to analyze your changes, then we can harmonize them later.
Mentioned in PR #421 and will be analyzed first before being ready for harmonizing.
Thank you very much!
Regards

@danielweber2018
Copy link
Contributor Author

Hi @minminlittleshrimp
From my side there is definitely still an interest in implementing this feature.

Maybe for a short explanation of my changes:
For my PR I used the offline trace pattern (size limit with multiple files, the files rotates via an file-counter, all configurable via config file). It was important to me to avoid duplicate code, so I refactored out the relevant code blocks from the offline trace. Unfortunately, that bloated the PR a bit.

I haven't maintained the PR (e.g. the new conflicts), but if there is interest in this PR I'll be happy to take care of it. If you have any questions, please contact me.

Thanks and regards Daniel

@minminlittleshrimp
Copy link
Collaborator

Hello @danielweber2018
Thank you for your information.
Then it would be very great if you could maintain your PR.
Thank you very much and looking forward to cooperating with you!
Minh

Enhance dlt logging such that multiple files are used as it is done
for the offline traces. Add limit-specific config values for logging.
For this purpose the pattern of index-based file names is used only.
This approach of logging to multiple files and rotating in order
to keep the limits ensures that dlt logs take care of available
space on the underlying file system and do not grow infinitely.

Signed-off-by: Daniel Weber <[email protected]>
Co-authored-by: Oleg Tropmann <[email protected]>
@danielweber2018
Copy link
Contributor Author

I rebased my PR to the current master and resolved all conflicts. Additionally i simplified the PR via the removing of the split between dlt_common and dlt_log (like requests from michael methner).

@michael-methner michael-methner merged commit 363d433 into COVESA:master Mar 10, 2023
const int file_size = 256;
const int max_file_size = 512;

const char* log1 = "ONE\n";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello @danielweber2018
Build logs show me:

Scanning dependencies of target gtest_dlt_daemon_multiple_files_logging
[ 86%] Building CXX object tests/CMakeFiles/gtest_dlt_daemon_multiple_files_logging.dir/gtest_dlt_daemon_multiple_files_logging.cpp.o
/home/lum3hc/work/github/PR442/dlt-daemon/tests/gtest_dlt_daemon_multiple_files_logging.cpp: In member function ‘virtual void t_dlt_logging_multiple_files_append_reinit_normal_Test::TestBody()’:
/home/lum3hc/work/github/PR442/dlt-daemon/tests/gtest_dlt_daemon_multiple_files_logging.cpp:106:28: warning: format not a string literal and no format arguments [-Wformat-security]
     dlt_vlog(LOG_INFO, log1);
                            ^
/home/lum3hc/work/github/PR442/dlt-daemon/tests/gtest_dlt_daemon_multiple_files_logging.cpp:110:28: warning: format not a string literal and no format arguments [-Wformat-security]
     dlt_vlog(LOG_INFO, log2);
                            ^
[ 87%] Linking CXX executable gtest_dlt_daemon_multiple_files_logging

Could you please consider to use string literal format?
Maybe char array?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Resolved with #481


/* open DLT output file */
errno = 0;
files_buffer->ohandle = open(file_path, O_WRONLY | O_APPEND, S_IRUSR | S_IWUSR |
Copy link
Collaborator

@minminlittleshrimp minminlittleshrimp Mar 29, 2023

Choose a reason for hiding this comment

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

Hello @danielweber2018
I would like to remove the superfluous mode bits with PR: #462
Thank you

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.

3 participants