Fetch plugin logs from server #193
Conversation
| if allNew != tc.expectedAllNew { | ||
| t.Logf("expected allNew: %v, got %v", tc.expectedAllNew, allNew) | ||
| t.Fail() | ||
| } |
There was a problem hiding this comment.
I really wanted to around importing testify as that way all plugins would have it in their go.mod.
| @@ -1,6 +1,6 @@ | |||
| module github.com/mattermost/mattermost-plugin-starter-template | |||
|
|
|||
| go 1.19 | |||
| go 1.21 | |||
| now := time.Now() | ||
| var oldest string | ||
|
|
||
| ticker := time.NewTicker(1 * time.Second) |
There was a problem hiding this comment.
Fetching the logs every second feels good from a UX perspective. I think the load is acceptable, but I'm open to increasing the interval.
It could even go down to 500 milliseconds, at least for local dev environments.
|
I've been thinking about adding a |
There was a problem hiding this comment.
This is cool, @hanzei! Love the simplicity that this affords plugin developers. A few thoughts below.
build/pluginctl/logs.go
Outdated
| // fetchLogs fetches log entries from Mattermost | ||
| // and filters them based on pluginID and timestamp. | ||
| func fetchLogs(ctx context.Context, client *model.Client4, page int, pluginID string, since time.Time) ([]string, error) { | ||
| logs, _, err := client.GetLogs(ctx, page, logsPerPage) |
There was a problem hiding this comment.
Not required for this PR, but I wonder if we can do better than just polling a logs endpoint that may not give us all the logs if a high volume of logs gets emitted? (e.g. 101 new log lines between polling intervals means some logs get "lost").
When I develop and want plugin logs, I basically just tail -f | jq ... the log file. Could we do something as reliable as that?
There was a problem hiding this comment.
Regarding the high volume of logs, please see #193 (comment). The code does now fetch more than one page.
There was a problem hiding this comment.
When I develop and want plugin logs, I basically just
tail -f | jq ...the log file. Could we do something as reliable as that?
I would love to use this simple, reliable solution, but I wanted to build a tool that also works for remote servers like the ones in MM Cloud. Is there another similar tool that we can use here instead of reading the local file?
There was a problem hiding this comment.
For MM Cloud, at least, I just use Grafana's livestreaming feature. Unless we build something more robust into the server, I guess we'll be limited in what we can do here, which might be fine for the level of logging needed.
build/pluginctl/logs.go
Outdated
| i := slices.Index(logs, oldest) | ||
| switch i { | ||
| case -1: | ||
| // Every log entry is new | ||
| return logs, logs[(len(logs) - 1)], true | ||
| case len(logs) - 1: | ||
| // No new log entries | ||
| return nil, oldest, false | ||
| default: | ||
| // Filter out oldest log entry | ||
| return logs[i+1:], logs[(len(logs) - 1)], false | ||
| } |
There was a problem hiding this comment.
See discussion above about whether or not we can eliminate allNew, but if so, I think this whole block could be reduced to:
return logs[i+1:], logs[(len(logs) - 1]The idea being that if i == -1, that's the same as returning logs[0:], and if i == len(logs) - 1, that's the same as returning logs[len(logs):] or the nil slice anyway.
There was a problem hiding this comment.
That is a clever suggestion, even though I'm not sure if it is easier to read. I've simplified the return value via 91f7b12, but I'm leaning toward keeping the switch as it is. Let me know what you think!
Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #193 +/- ##
======================================
Coverage 5.26% 5.26%
======================================
Files 3 3
Lines 38 38
======================================
Hits 2 2
Misses 36 36 ☔ View full report in Codecov by Sentry. |
@mickmister I think you meant to ping @lieut-data, right? |
hanzei
added
4: Reviews Complete
Summary
To make it easier for plugin developers to get the log of the plugin that they are developing, this PR add two new Make target:
make logsfetches the latest 500 log messages from the MM server. This is useful for figuring out existing issues with the plugin. I'm 0/5 of the concrete numbers. Maybe the command should show the latest 100 messages from the plugin instead of filtering 500 general ones.make logs-watchis the development-focused target. It continually fetches logs from Mattermost and prints only new ones to stdout. It's meant to be run in a separate terminal while developing the plugin.Screencast.from.18.12.2023.12.37.35.webm
Ticket Link
NONE