Adding a configuration for setting audit logs level #3512
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
idanovo
added
the
include-changelog
nopcoder
requested changes
Jun 16, 2022
| @@ -13,7 +13,7 @@ has_children: false | |||
| {% include toc.html %} | |||
|
|
|||
| Configuring lakeFS is done using a yaml configuration file and/or environment variable. | |||
| The configuration file location can be set with the '--config' flag. If not specified, the the first file found in the following order will be used: | |||
| The configuration file location can be set with the '--config' flag. If not specified, the first file found in the following order will be used: | |||
pkg/httputil/logging.go
Outdated
| "took": time.Since(startTime), | ||
| "status_code": writer.StatusCode, | ||
| "sent_bytes": writer.ResponseSize, | ||
| }).Debug("HTTP call ended") | ||
| } | ||
| endLogMsg := "HTTP call ended" |
pkg/httputil/logging.go
Outdated
| } | ||
| endLogMsg := "HTTP call ended" | ||
|
|
||
| switch strings.ToLower(middlewareLogLevel) { |
There was a problem hiding this comment.
pkg/httputil/logging.go
Outdated
| }) | ||
| } | ||
| } | ||
|
|
||
| func LoggingMiddleware(requestIDHeaderName string, fields logging.Fields, traceRequestHeaders bool) func(next http.Handler) http.Handler { | ||
| if logging.Level() == "trace" { | ||
| loggingMiddlewareLevel := viper.GetString(config.LoggingAuditLogLevel) |
There was a problem hiding this comment.
Usually, we extract the configuration value from the config structure. The config structure also expose a method in this case it can also parse the string and return the logging level
pkg/httputil/logging.go
Outdated
| @@ -63,6 +66,7 @@ func DebugLoggingMiddleware(requestIDHeaderName string, fields logging.Fields) f | |||
| logging.MethodFieldKey: r.Method, | |||
| logging.HostFieldKey: r.Host, | |||
| logging.RequestIDFieldKey: reqID, | |||
| logging.LoggerName: "MiddlewareLog", | |||
There was a problem hiding this comment.
- Usually we have property called "service".
- How is this property is used? Because we didn't really create another logger and having a name can be confusing.
There was a problem hiding this comment.
It's just my way to add a kind of "tag" for logs coming from the audit logger. Maybe I'll change it to something like log_source?
nopcoder
approved these changes
Jun 18, 2022
| @@ -13,6 +14,7 @@ type contextKey string | |||
|
|
|||
| const ( | |||
| RequestIDContextKey contextKey = "request_id" | |||
| AuditLogEndMessage string = "HTTP call ended" | |||
There was a problem hiding this comment.
- No need the type - constant will get the type while using it.
- You can update the message to include
API call informationor something like that if it make more sense.
Suggested change
| AuditLogEndMessage string = "HTTP call ended" | |
| AuditLogEndMessage = "HTTP call ended" |
pkg/logging/dummy.go
Outdated
| @@ -52,6 +52,10 @@ func (d DummyLogger) Panic(args ...interface{}) { | |||
|
|
|||
| } | |||
|
|
|||
| func (d DummyLogger) Log(level string, args ...interface{}) { | |||
There was a problem hiding this comment.
Think it is time to have also level as part of the logging abstraction so we will not work with strings.
pkg/httputil/logging.go
Outdated
| @@ -63,6 +66,7 @@ func DebugLoggingMiddleware(requestIDHeaderName string, fields logging.Fields) f | |||
| logging.MethodFieldKey: r.Method, | |||
| logging.HostFieldKey: r.Host, | |||
| logging.RequestIDFieldKey: reqID, | |||
| logging.LoggerName: "MiddlewareLog", | |||
pkg/httputil/logging.go
Outdated
| @@ -63,6 +65,7 @@ func DebugLoggingMiddleware(requestIDHeaderName string, fields logging.Fields) f | |||
| logging.MethodFieldKey: r.Method, | |||
| logging.HostFieldKey: r.Host, | |||
| logging.RequestIDFieldKey: reqID, | |||
| logging.LogSource: "MiddlewareLogger", | |||
There was a problem hiding this comment.
Consider "audit" with value "api"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.