Add support for publishing logs to NATS.#36527
Conversation
…sult in published messages.
ebusto
requested review from
a team,
lukeheath and
rachaelshaw
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #36527 +/- ##
==========================================
- Coverage 65.86% 65.85% -0.02%
==========================================
Files 2361 2363 +2
Lines 187305 187608 +303
Branches 8010 7976 -34
==========================================
+ Hits 123364 123540 +176
- Misses 52664 52766 +102
- Partials 11277 11302 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@ebusto thanks for your contribution! Temporarily converting this to a draft PR while we bring this change through our Drafting process. You'll be notified on the connected issue once it makes it into a release. |
There was a problem hiding this comment.
LGTM ✅ . I have a few comments in server/logging/nats.go, but nothing major.
I've tested this locally as follows (will document it in a follow-up PR):
- Installed
nats:go install github.com/nats-io/natscli/nats@latest - Started the NATS server locally:
nats-server:
- Start fleet with the following flags:
FLEET_ACTIVITY_ENABLE_AUDIT_LOG=true FLEET_ACTIVITY_AUDIT_LOG_PLUGIN=nats FLEET_OSQUERY_RESULT_LOG_PLUGIN=nats FLEET_OSQUERY_STATUS_LOG_PLUGIN=nats FLEET_NATS_SERVER=nats://localhost:4222 FLEET_NATS_STATUS_SUBJECT=osquery_status FLEET_NATS_RESULT_SUBJECT=osquery_result FLEET_NATS_AUDIT_SUBJECT=fleet_audit ./build/fleet serve --dev- Configured a scheduled query:
- On a separate terminal, ran
./nats --server=nats://localhost:4222 subscribe ">". Per GPT, thislistens on all subjects and prints any messages that get published. Eventually, I saw the query results being logged:
EDIT: I also tested with one of the authentication methods, NKey:
- Install
nkey:go install github.com/nats-io/nkeys/nk@latest. - Run
nk -gen user -pubout. This will output something like:
SUxxx
Uyyy
- Copy the output above to a txt file, e.g.
nkey-cred-file.txt. - Create a new NATS server config file, such as
nats-server-config.confwith this content:
authorization {
users = [
{
nkey: "Uxxx"
}
]
}
- Start the NATS server providing the config file above:
./nats-server -config nats-server-config.conf. You should see a log sayingUsing configuration file: nats-server-config.conf.
- Start fleet specifying the NKey cred file to use:
FLEET_ACTIVITY_ENABLE_AUDIT_LOG=true FLEET_ACTIVITY_AUDIT_LOG_PLUGIN=nats FLEET_OSQUERY_RESULT_LOG_PLUGIN=nats FLEET_OSQUERY_STATUS_LOG_PLUGIN=nats FLEET_NATS_SERVER="nats://localhost:4222" FLEET_NATS_STATUS_SUBJECT=osquery_status FLEET_NATS_RESULT_SUBJECT=osquery_result FLEET_NATS_AUDIT_SUBJECT=fleet_audit FLEET_NATS_NKEY_FILE="nkey-cred-file.txt" ./build/fleet serve --dev
| // Define the supported compression algorithms. | ||
| var compressionOk = map[string]bool{ | ||
| "gzip": true, | ||
| "snappy": true, | ||
| "zstd": true, | ||
| } |
There was a problem hiding this comment.
I'd remove the comment on L66 and rename the variable to a more intent-revealing name, e.g. supportedCompressionAlgorithms.
Also, this could probably be a slice instead of a map[string]bool. Since all entries are effectively true, the boolean doesn’t add much value, and a list of supported algorithms feels more readable and intuitive here. With so few elements, performance isn’t really a concern IMO.
There was a problem hiding this comment.
Idiomatic Go for sets is map[string]struct{}.
| "zstd": true, | ||
| } | ||
|
|
||
| // NewNatsLogWriter creates a new NATS log writer. |
There was a problem hiding this comment.
The method's name already makes me infer that it does this, so I'd remove this comment.
| // Whether to use JetStream. | ||
| jetstream bool |
There was a problem hiding this comment.
since this is a boolean I'd prefix it with is or has, or maybe in this case useJetstream
| @@ -0,0 +1,484 @@ | |||
| package logging | |||
There was a problem hiding this comment.
IMO, some of the comments in this file shouldn't be needed since the reader can infer what the code does by reading the variable names or following through the statements. For example, the comment above NewNatsLogWriter says // NewNatsLogWriter creates a new NATS log writer., which I don't think adds any value and is also redundant.
iansltx
left a comment
iansltx
left a comment
There was a problem hiding this comment.
Approving FE for LogDestinationIndicator.tsx and frontend/interfaces/config.ts.
|
@ebusto this change shipped in Fleet 4.80.0. Thanks for adding this feature! 🎉 |
5 participants
Related issue: Resolves 34890
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
New Fleet configuration settings
Looking at other log destinations, I couldn't find anything relevant in GitOps. Please let me know if I missed something, however.
fleetd/orbit/Fleet Desktop
I've tested this on both Linux and MacOS.