-
Notifications
You must be signed in to change notification settings - Fork 36
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
Simplify NSM logging system #1308
base: main
Are you sure you want to change the base?
Simplify NSM logging system #1308
Conversation
Signed-off-by: anastasia.malysheva <anastasia.malysheva@xored.com>
b07d7e9
to
ebf273b
Compare
Signed-off-by: anastasia.malysheva <anastasia.malysheva@xored.com>
Signed-off-by: anastasia.malysheva <anastasia.malysheva@xored.com>
pkg/registry/core/trace/common.go
Outdated
} else { | ||
msg = fmt.Sprint(v) | ||
registryInfo, ok := trace(ctx) | ||
if ok && registryInfo.Request != nil { |
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.
This condition is always false
We don't need to check Request and Response, because these fields are used only for networkservice
(not for registry
)
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.
@glazychev-art , fixes for both comments were made
pkg/registry/core/trace/context.go
Outdated
) | ||
|
||
// traceInfo - struct, containing string representations of request and response, used for tracing. |
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.
// traceInfo - struct, containing string representations of request and response, used for tracing. | |
type traceInfo struct {} |
Signed-off-by: anastasia.malysheva <anastasia.malysheva@xored.com>
Signed-off-by: anastasia.malysheva <anastasia.malysheva@xored.com>
@glazychev-art Is this ready to merge? |
Description
EnableTracing was removed. Instead LogLevel is used to enable tracing if TraceLevel is set.
Issue link
#1272
How Has This Been Tested?
Types of changes