Skip to content
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

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

Open
jqueuniet opened this issue Apr 15, 2024 · 2 comments
Open

Comments

@jqueuniet
Copy link

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
Copy link
Owner

rs commented Apr 15, 2024

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

@jqueuniet
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants