feat: add agentsdk function for sending logs #7629
Conversation
c8befde to
f196725
Compare
This enables external API consumers to easily send logs in order and with failure tolerance.
|
This Pull Request is becoming stale. In order to minimize WIP, prevent merge conflicts and keep the tracker readable, I'm going close to this PR in 3 days if there isn't more activity. |
| } | ||
| c.SDK.Logger.Warn(ctx, "upload startup logs failed", slog.Error(err), slog.F("count", len(logs))) | ||
| } | ||
| if ctx.Err() != nil { |
There was a problem hiding this comment.
I think this should be captured and shared, i.e. if lastSendErr == nil { lastSendErr = ctx.Err(); } or some such.
There was a problem hiding this comment.
I recently made a PR (#8084) to fix a log sending deadlock in the agent. Since I noticed this PR I decided to give it a quick review and it didn't have the same problem (but perhaps some others). Not sure whether or not it's still relevant but submitting the review in case it is.
| queuedLogs := make([]StartupLog, 0, 100) | ||
| for { | ||
| select { | ||
| case <-time.After(debounce): |
There was a problem hiding this comment.
Suggestion: Consider a timer vs time.After for conserving resources/cleaner shutdown. Let's say we log 1000 lines in 100ms, we will have ~1000 of these timers.
I also wonder if we should avoid starting a new timer because 100 log entries at a rate of 249ms/line will take ~25s to be sent to the server.
| sendLogsWithRetry(queuedLogs) | ||
| queuedLogs = nil | ||
| case <-closeChan: | ||
| close(logChan) |
There was a problem hiding this comment.
The func(log StartupLog) may panic after this since it can try to send on the closed channel. It should be left open.
https://go.dev/play/p/-qzJ3Wo1TYp (Try running a few times.)
|
See 3b9b06f |
This enables external API consumers to easily send logs in order and with failure tolerance.