-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat: add more detail log in chat api #453
Conversation
58e2838
to
d9aae55
Compare
For this conversation management https://github.com/kubeagi/arcadia/blob/main/apiserver/pkg/chat/chat.go#L42 , should we make this a |
yep, will cache it into postgress later, maybe in next version. |
@@ -102,8 +103,8 @@ func AppRun(ctx context.Context, req ChatReqBody, respStream chan string) (*Chat | |||
if err != nil { | |||
return nil, err | |||
} | |||
klog.Infoln("begin to run application", obj.GetName()) | |||
out, err := appRun.Run(ctx, c, respStream, application.Input{Question: req.Query, NeedStream: req.ResponseMode == Streaming, History: conversation.History}) | |||
klog.FromContext(ctx).Info("begin to run application", "appName", req.APPName, "appNamespace", req.AppNamespace) |
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.
klog.V(5) ?
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.
Info or V(5).Info are both ok for me.
The update inherited from the original of @nkwangleiGIT .
IMO, this log conveniently allows us to calculate how much time was actually spent running the app, leaving aside the chat api initialization time.
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.
That's good.
Seems to me
V(5) -> `DEBUG`
V(3) -> Info
V(1) -> ERROR
Since we are going to release v0.1.0 , it's the time that we should make a agreement on logging levels.
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.
klog.InfoS
- Structured logs to the INFO log.InfoS
should be used for routine logging. It can also be used to log warnings for expected errors (errors that can happen during routine operations).
Depending on log severity it's important to pick a proper verbosity level to ensure that consumer is neither under nor overwhelmed by log volume:klog.V(0).InfoS
=klog.InfoS
- Generally useful for this to always be visible to a cluster operator- Programmer errors
- Logging extra info about a panic
- CLI argument handling
klog.V(1).InfoS
- A reasonable default log level if you don't want verbosity.- Information about config (listening on X, watching Y)
- Errors that repeat frequently that relate to conditions that can be corrected (pod detected as unhealthy)
klog.V(2).InfoS
- Useful steady state information about the service and important log messages that may correlate to significant changes in the system. This is the recommended default log level for most systems.- Logging HTTP requests and their exit code
- System state changing (killing pod)
- Controller state change events (starting pods)
- Scheduler log messages
klog.V(3).InfoS
- Extended information about changes- More info about system state changes
klog.V(4).InfoS
- Debug level verbosity- Logging in particularly thorny parts of code where you may want to come back later and check it
klog.V(5).InfoS
- Trace level verbosity- Context to understand the steps leading up to errors and warnings
- More information for troubleshooting reported issues
Also we can use |
hahaha, in next version, we will use pg, so mutex will be pg table lock or something. |
👀 |
Besides how about we use https://github.com/go-playground/validator for common struct validations? Then we do not need to code too many |
Signed-off-by: Abirdcfly <[email protected]>
What type of PR is this?
/kind cleanup
What this PR does / why we need it
X-Request-ID
from the request) to make it easier to filter the logs of individual requests.fmt.Errorf("xxx:%w", err)
), easy to locate the problem.Which issue(s) this PR fixes
For #413
Special notes for your reviewer
-v=5 stream common request:
Details
-v=5 blocking common request
Details
stream common request:
Details
blocking common request:
Details