rs / zerolog

Zero Allocation JSON Logger
MIT License
10.33k stars 564 forks source link

Should journald keys be sanitized to strip/replace invalid characters? #668

Open jqueuniet opened 4 months ago

jqueuniet commented 4 months ago

Some logging middlewares like https://github.com/grpc-ecosystem/go-grpc-middleware/blob/main/interceptors/logging/logging.go like to use dots to namespace field keys. This is not an issue using the console or JSON output formats, but sending those messages to the journald writer triggers errors on the dot character.

Apr 15 07:53:33 hostname process[521208]: 07:53:33 INF started call grpc.component=server grpc.method=Check grpc.method_type=unary grpc.service=grpc.health.v1.Health grpc.start_time=2024-04-15T07:53:33Z grpc.time_ms=0.019 peer.address=1.2.3.4:49140 protocol=grpc request_id=4a4a70b0-1c92-47d9-a50c-b10f9f636bd7
Apr 15 07:53:33 hostname process[521208]: variable name GRPC.SERVICE contains invalid character, ignoring
Apr 15 07:53:33 hostname process[521208]: variable name GRPC.METHOD contains invalid character, ignoring
Apr 15 07:53:33 hostname process[521208]: variable name GRPC.START_TIME contains invalid character, ignoring
Apr 15 07:53:33 hostname process[521208]: variable name GRPC.TIME_MS contains invalid character, ignoring
Apr 15 07:53:33 hostname process[521208]: variable name PEER.ADDRESS contains invalid character, ignoring
Apr 15 07:53:33 hostname process[521208]: variable name GRPC.METHOD_TYPE contains invalid character, ignoring
Apr 15 07:53:33 hostname process[521208]: variable name GRPC.COMPONENT contains invalid character, ignoring

Do you consider the journald writer could make some attempt at sanitising those keys, ie by replacing invalid characters by _ or something else, or do you feel like it is the responsibility of the zerolog user to ensure only valid keys are used?

rs commented 4 months ago

I'm leaning to sanitizing in this case. Any objections?

jqueuniet commented 4 months ago

That would seem the most logical to me, middlewares and other third-party libraries may not leave a lot of room to modify keys depending on the specificities of their logging interface. And as the journald format seems a bit restrictive, expecting users to enforce its limits on all writers may lead to issues like a log ingester pipeline expecting a specific format on the JSON output impossible de reconcile with journald.

It seems this error message is generated by validVarName in the coreos journald Go library.

// validVarName validates a variable name to make sure journald will accept it.
// The variable name must be in uppercase and consist only of characters,
// numbers and underscores, and may not begin with an underscore:
// https://www.freedesktop.org/software/systemd/man/sd_journal_print.html
func validVarName(name string) error {
    if name == "" {
        return errors.New("Empty variable name")
    } else if name[0] == '_' {
        return errors.New("Variable name begins with an underscore")
    }

    for _, c := range name {
        if !(('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') || c == '_') {
            return errors.New("Variable name contains invalid characters")
        }
    }
    return nil
}

zerolog already takes care of converting to upper-case.