-
Notifications
You must be signed in to change notification settings - Fork 53
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
[JUJU-3777] Multiplex services to log targets using only a log-target.services
field
#252
Conversation
- fix service name validation to handle `-`
- catch iteration mistake in LogsTo - modify plan override test, to test changing location/type
log-target.services
fieldlog-target.services
field
Co-authored-by: Ben Hoyt <benhoyt@gmail.com>
- consolidate svc / -svc validation in CombineLayers
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.
Awesome, glad we were able to come to a simple solution that doesn't really lose much of the functionality we were seeking.
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.
Excellent, thanks for the updates -- looks good to me.
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 much nicer, thanks Jordan. Only trivial comments below, and LGTM.
- simplify log-target.services validation - improve deprecation warning - expand examples in README
This PR contains the second part of #165, including the actual mechanics of log forwarding in Pebble. It builds upon #209 and #252. This includes an abstract `logClient` interface but doesn't include the specific Loki/syslog implementations - this will come in later PRs. *This is a modification of #218, designed to fix fundamental issues identified with that PR.* ### Current design For each log target, there is a single "gatherer", which receives logs from a bunch of services and writes them to a "client". The gatherer runs a control loop in a separate goroutine, which waits to receive logs on a channel, and writes these to the client. For each service, the gatherer spawns a "log puller". Each puller runs in a separate goroutine, pulling logs from an iterator on the service's ringbuffer. Pulled logs are sent to the gatherer's control loop on the shared channel. The `logClient` interface represents a client to a specific type of backend. In future PRs, we will add `lokiClient` and `syslogClient` types which implement `logClient`. `logClient` includes two methods ```go type logClient interface { Write(context.Context, servicelog.Entry) error Flush(context.Context) error } ``` Client implementations have some freedom about the semantics of these methods. - For a buffering client (e.g. HTTP), `Write` could add the log to the client's internal buffer, while `Flush` would prepare and send an HTTP request with the buffered logs. - For a non-buffering client (TCP?), `Write` could serialise the log directly to the open connection, while `Flush` would be a no-op. ### Teardown Gracefully stopping a log gatherer is a complex process with multiple steps. - At the instant we attempt to stop the gatherer, it may be in the middle of flushing its client. So, wait a small amount of time for the client to finish flushing, but cancel the flush if this takes too long. - The service may have emitted some final logs on shutdown. Give each puller some time to pull the final logs from its iterator - but again, force kill it if this is taking too long. - Once the pullers are all finished, we must have received all the logs we're gonna get, so we can safely shut down the main loop. - Do a final flush of the client to send off any remaining buffered logs. All this logic is encapsulated in the `gatherer.stop()` method. ## QA I've included some sample implementations of `logClient` [here] (https://github.com/barrettj12/pebble/blob/logfwd-fake/internals/overlord/logstate/fake.go). They just print the logs to stdout. These can be used to verify that the log forwarding mechanics work properly. Create a simple logging service, e.g. ```bash #!/bin/bash while true; do echo "Hello" sleep 1 done ``` and a simple plan using this service ```yaml services: svc1: &logsvc command: /home/jb/git/canonical/pebble/logfwd-impl2/pebble/logsvc startup: enabled override: merge svc2: *logsvc log-targets: tgt1: override: merge services: [all] type: loki location: unnecessary ``` Add the [`fake.go`] (https://github.com/barrettj12/pebble/blob/logfwd-fake/internals/overlord/logstate/fake.go) file to the repo. Comment out the following line https://github.com/canonical/pebble/blob/3e904f9d22f297b68cba2dc33c9cf8e1bbbadd90/internals/overlord/logstate/gatherer.go#L356 and replace it with e.g. ```go return &nonBufferingClient{}, nil // unbuffered return &bufferingClient{}, nil // unlimited-size buffer, will flush on timeout only return &bufferingClient{threshold: 3}, nil // buffer with max size: 3 logs ``` You might also want to change the gatherer's tick period: https://github.com/canonical/pebble/blob/3e904f9d22f297b68cba2dc33c9cf8e1bbbadd90/internals/overlord/logstate/gatherer.go#L32 Run Pebble with ``` go run ./cmd/pebble run ``` and verify the logs are printing to stdout. --- JUJU-3776
#223 was proving too complex, so we've come up with an alternative way to specify which services each log target should collect logs from.
The log target spec now has a
services
field, which accepts a list of services to collect log targets from.This replaces the old
log-target.selection
field, which has now been removed. Theservices
field is also much more expressive thanselection
, so it also removes the need for theservice.log-targets
field. Effectively,log-target.services
is now the only place to specify which services' logs go to which log targets.Some bonus features:
Use the
all
keyword to match all services (including future services):Use
-svc
to remove a service from the list. This is especially useful when merging or after usingall
.matches all services except
svc1
andsvc3
.Use
-all
to remove all service from the list. Again, useful when merging or after usingall
.matches no services.
The
services
field is simply appended when merging. In combination with the above special syntaxes, this allows "replace" behaviour in a merge:To remove service(s) specified in a lower layer, you can just merge on top
To remove all services for a log target (i.e. opting out of log forwarding entirely), just merge on top
Context / links
LogTargets = nil
andLogTargets = []string{}
#217!replace
-ing servicelog-targets
in merge #221^log-targets
#223