minsev: `Severity` and `SeverityVar` should implement string-related interfaces

[sahidvelji]

Problem Statement

I would like to easily convert a string like "info", "DEBUG", "error3", or "FATAL4" to either minsev.Severity or minsev.SeverityVar.

The use case here is reading log severity from an environment variable. It is common for log severity to differ by environment, meaning that it is reasonable to expect the value of log severity to be read from an environment variable. However, there is currently no easy way to convert a string to minsev.Severity or minsev.SeverityVar. Such conversion would have to be done manually.

Proposed Solution

I would like minsev.Severity to implement:

• encoding.TextUnmarshaler.
• encoding.TextMarshaler
• encoding/json.Marshaler
• encoding/json.Unmarshaler
• encoding.TextAppender
• fmt.Stringer

and minsev.SeverityVar to implement:

• encoding.TextUnmarshaler.
• encoding.TextMarshaler
• encoding.TextAppender
• fmt.Stringer

We should have Severity and SeverityVar implement the fmt.Stringer interface so that printing a Severity or SeverityVar results in an output of, for example, FATAL2 instead of 13.

See the Prior Art section. I am used to using slog, but this problem statement does not apply to slog because slog.Level and slog.LevelVar both implement encoding.TextUnmarshaler.

Alternatives

One alternative would be to implement such conversion from a string to minsev.Severity or minsev.SeverityVar myself. However, I consider this to be a fairly common use case.

Prior Art

The standard library slog package has types that are analogous to the minsev package. Namely:

• slog.Leveler is analogous to minsev.Severitier
• slog.Level is analogous to minsev.Severity
• slog.LevelVar is analogous to minsev.SeverityVar

slog.Level implements

• encoding.TextUnmarshaler
• encoding.TextMarshaler
• encoding/json.Marshaler
• encoding/json.Unmarshaler
• encoding.TextAppender
• fmt.Stringer

slog.LevelVar implements

• encoding.TextUnmarshaler
• encoding.TextMarshaler
• encoding.TextAppender
• fmt.Stringer

I imagine the implementation in the minsev package would be very similar to those in the slog package.

Additional Context

There are libraries that can parse environment variables into a struct, like ardanlabs/conf. This works with slog, but not with minsev because of the above.

[pellared]

@MrAlias, PTAL as a code owner.

[MrAlias]

LGTM 👍

[pellared]

@sahidvelji, do you want to contribute and submit a PR?

[sahidvelji]

@pellared Yes, feel free to assign me.

[sahidvelji]

I missed something. I should have included in the original post that I also want Severity and SeverityVar to implement the fmt.Stringer interface. Currently,

fmt.Println(minsev.SeverityFatal2)

prints 13. Instead I think it should print FATAL2.

I will update my original post.

[sahidvelji]

I intend to see this through, but it might take me a week or two before I get back to this.

[MrAlias]

@sahidvelji are you still able to work on this? Is this something another person can pick up and get done for this milestone?

[sahidvelji]

@MrAlias Sorry for the delay. I don't think I will have the time to get to this before GopherCon.

Is this something another person can pick up and get done for this milestone?

Yes absolutely. I can also help review the PR.

[MrAlias]

I'm contemplating what string conversions we want.

Ideally, the Severity from minsev would produce a string representation that is the same as log.Severity. That way there is an interchange between any text object (e.g. environment variable, configuration file) used to configure both.

The log.Severity is bounded. Meaning values outside of the defined range are not supported. We could follow this and treat any value outside of -8 to 15 as similar to log.SeverityUndefined. This means that Severity.String() would return "UNDEFINED" for those values and the parse operation would just fail.

Alternatively, we could try to accept more values than log.Severity. We could have generic rules that support parsing things like TRACE-10, FATAL9, and INFO+5 in addition to all the log.Severity values. We would default to returning known values of log.Severity for values in the range of [-8, 15], but use +/- syntax for values outside of this.

Supporting a wider range is likely not worth it though. We already clamp our conversions to log.Severity. Allowing users to set it to values outside of this range is not going to provide much value as we use this conversion when actually determining the processor behavior.