-
Notifications
You must be signed in to change notification settings - Fork 164
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
Add new types and functions to pillar needed by newlog and edge-view #4449
Add new types and functions to pillar needed by newlog and edge-view #4449
Conversation
pkg/pillar/types/global.go
Outdated
@@ -295,6 +295,10 @@ const ( | |||
SyslogLogLevel GlobalSettingKey = "debug.syslog.loglevel" | |||
// KernelLogLevel global setting key | |||
KernelLogLevel GlobalSettingKey = "debug.kernel.loglevel" | |||
// SyslogRemoteLogLevel global setting key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why we have this tradition of adding a useless comment to all the properties; it would make sense to stop and add something more meaningful (I'm trying to do it myself).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be beneficial to add documentation for the new props in the same commit or in one that's close.
https://github.com/lf-edge/eve/blob/master/docs/CONFIG-PROPERTIES.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why we have this tradition of adding a useless comment to all the properties; it would make sense to stop and add something more meaningful (I'm trying to do it myself).
It is a lint requirement to document all exported names. But we can put something more useful after the // SyslogRemoteLogLevel required prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be beneficial to add documentation for the new props in the same commit or in one that's close.
I'm doing it in my main PR #4413
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the comments to more meaningful descriptions
// DevPrefixUpload - file prefix string for device log files to upload | ||
DevPrefixUpload = "dev.log.upload." | ||
// DevPrefixKeep - file prefix string for device log files to keep on device | ||
DevPrefixKeep = "dev.log.keep." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, are we going to change the directories naming scheme?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no the directories themselves, but the files inside of them
@@ -81,3 +84,22 @@ type NewlogMetrics struct { | |||
DevMetrics logfileMetrics // Device metrics | |||
AppMetrics logfileMetrics // App metrics | |||
} | |||
|
|||
// GetTimestampFromGzipName - get timestamp from gzip file name | |||
func GetTimestampFromGzipName(fName string) (time.Time, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this should also replace this func, I guess?
eve/pkg/pillar/cmd/loguploader/loguploader.go
Line 739 in 63c8aae
func getTimeNumber(isApp bool, fName string) (bool, int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly, I'm doing this in the main PR #4413
Adds remote log levels for kernel and syslog logs: - debug.syslog.remote.loglevel - debug.kernel.remote.loglevel Adds log levels "all" and "none": as the names suggest, "all" will log everything and "none" will suppress all logs. Signed-off-by: Paul Gaiduk <paulg@zededa.com>
Add new metrics for newlog: - totalSizeLogs: total size of logs on device - oldestSavedDeviceLog: timestamp of the latest device log saved on device Add new log file prefixes for device logs: - "dev.log.upload.": device logs that are uploaded - "dev.log.keep.": device logs that stay on the device Signed-off-by: Paul Gaiduk <paulg@zededa.com>
Since the functionality is used across pillar, newlog and edgeview code, it is better to have it in a separate file imported by all of them. Signed-off-by: Paul Gaiduk <paulg@zededa.com>
46c6ba4
to
64218a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is a dependency required by #4413