[telemetry-subscribers] add RUST_LOG_TAILS for per-target log file tails - #27026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
amnn
left a comment
There was a problem hiding this comment.
This is currently feeling quite ad-hoc to me, especially the way it requires json log output to be disabled in order for json log target to be applicable, and the fact that enabling this causes logs to disappear from the normal format (coupled with the fact that targets are prefix matched, which means you may end up inadvertently capturing logs you didn't mean to).
Instead, can we offer some kind of orthogonal control surface for this? I.e. I'd like to be able to emit logs matching certain targets (separate from the normal filter) to a file (separate from log_file) in addition to whatever normal logging configuration there is.
I changed the env var to be less focused on JSON by allowing it to set either the format, the file, or both per target. Should the logs in this new file be accessed through Grafana like the existing logs or copied out of the k8s pod? I was testing doing it either way in https://github.com/MystenLabs/sui-operations/pull/8197/, but logging to a new file would require either ingesting that file into Grafana (would be a separate change) or getting it from the k8s pod. |
75143e0 to
d402743
Compare
d402743 to
74520d8
Compare
amnn
left a comment
There was a problem hiding this comment.
The telemetry subscriber is already pretty complex, with lots of flags interacting with each other. If we can avoid introducing new complexity (interactions between flags or multiple ways to do things) then we should.
In that vein, I'd suggest introducing an environment variable for tailing logs, filtered by target, to a file, optionally modifying the format. Key differences from what we have here:
- Just duplicate the logs (don't eliminate logs from the normal stream because they are being tailed elsewhere). Given filters are prefix matches, it can be difficult to predict what they pick up and it's incorrect to assume that the audience for the human-readable logs and the audience for the tailed logs are the same (in a way where they would care that the logs are duplicated).
- The file/path becomes mandatory -- we are always writing to a file, never to the default stream -- but the format is optional. This reduces overlap with other features (like
json_log_output, so don't ignore this config if that flag is set), and also the potential to duplicate logs in an obvious way. - I would also suggest parsing the path as a
PathBufrather than aString, and keying the format on the extension of that path (e.g. if it's.jsonlthen you assume json formatting), to avoid having to invent new syntax. - Name it something other than
RUST_LOG_TARGETS-- a "target" is the label for the log and lots of configs we care about accept a target in this file. PerhapsRUST_LOG_TAILSor something?
9516251 to
aa086d5
Compare
aa086d5 to
c38d621
Compare
c38d621 to
18fc90a
Compare
fa6369c to
f6c5ac4
Compare
f6c5ac4 to
fd61030
Compare
fd61030 to
0d2c33b
Compare
0d2c33b to
7e40b00
Compare
7e40b00 to
5796bd8
Compare
Updated the PR
Updated the PR with these changes which ended up simplifying the code a bit. |
5796bd8 to
c99df3d
Compare
c99df3d to
d93f5de
Compare
amnn
left a comment
There was a problem hiding this comment.
Looks good, thanks @evan-wall-mysten !
RUST_LOG_TAILS is a comma-separated list of `target@file` entries. Each entry additionally writes the matching tracing target's logs to a daily-rolling file (like RUST_LOG_FILE), on top of the normal output stream — events are duplicated, not moved, including when RUST_LOG_JSON is set. The `@file` is mandatory and its extension selects the format (`.jsonl`/`.ndjson` select JSON, everything else is text). Each tail honors RUST_LOG's per-target level. A malformed entry or an uncreatable file directory hard-fails init.
Description
Adds
RUST_LOG_TAILStotelemetry-subscribers(with awith_log_tailbuilder /log_tailsconfig) that additionally writes individual tracing targets to dedicated files, on top of the normal log output. A "tail" is an extra file sink: matching events are duplicated to the file, not moved off the console/JSON stream.Syntax — comma-separated
target=fileentries (mirroringRUST_LOG'starget=level, but assigning a file instead of a level):=fileis a daily-rolling file (likeRUST_LOG_FILE) and is mandatory —RUST_LOG_TAILSonly adds file sinks, it never routes to the stream..jsonl/.ndjson(case-insensitive) → newline-delimited JSON; everything else (including.jsonand.log) → text.graphql_request=/var/log/sui/gql.loggraphql_request→ text in that filegraphql_request=/var/log/sui/gql.jsonlgraphql_request→ NDJSON in that file (replay corpus)a=/x.jsonl,b=/y.logBehavior:
RUST_LOG_JSON(global JSON output) is set.RUST_LOG's per-target level (its layer filter is the target restriction ANDed with anEnvFilter): e.g. withRUST_LOG=warnabartail captures onlywarn+, whileRUST_LOG=warn,bar=debugalso capturesbardebug. Targets are matched by prefix (likeRUST_LOG).=file) or a file whose directory cannot be created hard-failsinit()rather than silently degrading.Example — capture
graphql_requestas a JSON replay corpus in its own file while normal logs stay human-readable:Test plan
Added unit tests (entry parsing, extension→format inference, malformed-entry panics, builder).
cargo test -p telemetry-subscribers,cargo xclippy, andcargo fmt -- --checkall pass.Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.
RUST_LOG_TAILSenv var additionally writes individual tracing targets to dedicated files (format inferred from the file extension —.jsonl/.ndjsonfor JSON, otherwise text), leaving normal log output unchanged. Backward-compatible — unset by default, so log output is unchanged and no action is required.RUST_LOG_TAILSenv var is available to indexer binaries (e.g.sui-indexer-alt-graphql). Set it alongsideRUST_LOG(e.g.RUST_LOG="info,graphql_request=trace"withRUST_LOG_TAILS="graphql_request=/var/log/sui/graphql_request.jsonl") to capture that target as NDJSON for replay while normal logs stay readable. Opt-in; no action required otherwise.