-
Notifications
You must be signed in to change notification settings - Fork 41
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
[TT-590] logwatch with multiple targets and retries #789
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #789 +/- ##
==========================================
+ Coverage 31.04% 32.38% +1.34%
==========================================
Files 41 42 +1
Lines 5370 5993 +623
==========================================
+ Hits 1667 1941 +274
- Misses 3524 3832 +308
- Partials 179 220 +41 ☔ View full report in Codecov by Sentry. |
logstream/logstream.go
Outdated
} | ||
} | ||
|
||
// Stop stops the consumer and closes temp file |
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.
Maybe we change the comment, I don't see any file closing.
return errors.Wrap(err, "failed to open log file. File logging stopped") | ||
} | ||
|
||
defer logFile.Close() |
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.
We are closing and opening a file for each line, can we open the file once?
This PR introduces the ability to easy add new logging targets while adding three of them: Loki, file and in-memory. It also adds retries for log production failures.
It is possible to use any number of logging targets concurrently. By default there are no logging targets set. They should be specified using env variable LOGWATCH_LOG_TARGETS (case insensitive). For testing purposes I also added functional option to set them.
The constructor accepts also a more couple of variadic options:
It also executes some log target validations and operations:
Also, Loki client is now lazy initalised the first time it is requested instead of being always initialised in the constructor (even if streaming logs to Loki was not requested)
run_id
that's used to group together test logs in Loki (in CI it's GH'srun_id
on local it's a generated value stored in.run.id
file)Apart from that a lot of other things were done, like:
Last, but not least: I've added buffering logs in temp file. So in general the flow is like this:
Important: when we flush logs, then consumer state is set to
done
. From now on it won't accept any logs and sending them will log an error. Also, because we work with a temp file that's read/written using a shared reference we cannot read/write at the same time as it would mess up cursor position, that's why there's a mutex to synchronise these actions.Based on
run_id
andcontainer_ids
it's possible to save a test summary with log location for tests that were executed. It can be used to display link to Grafana Dashboard with logs of failed tests.It is recommended to connect/disconnect LogWatch to containers using PostStarts/PostStops hooks instead of doing it manually.
Known issues existing in previous versions:
New env variables:
GRAFANA_URL
-- URL to Grafana instance that is connected to Loki instance to which logs where streamedGRAFANA_DATASOURCE
-- Loki datasource id that will be used when building the URLLOKI_TENANT_ID
LOKI_URL
LOKI_BASIC_AUTH
LOGWATCH_LOG_TARGETS
-- "file", "loki" or "in-memory"Chainlink repo example: https://github.com/smartcontractkit/chainlink/actions/runs/7105718638