Feature #20864: Allow `Kernel#warn` to accept `**options` and pass these to `Warning.warn`.

• Status: Open
• Assignee: -
• Target version: -

[ioquatix (Samuel Williams)]

Background

Structured logging is a practice that organizes log data in a consistent, easily parseable format. Unlike traditional unstructured logs, which often consist of plain text entries, structured logs format information in key-value pairs or structured data formats such as JSON. This allows for better automation, searchability, and analysis of log data.

In Ruby, Kernel#warn is extremely useful, especially because there is a mechanism for loggers to redirect warnings using the Warning module. However, it is difficult to generate structured logs with Kernel#warn as all the positional arguments are converted to a single string, and arbitrary keyword options are rejected.

As a consequence, code like this is not possible:

begin
  ...
rescue => error
  warn "Something went wrong!", exception: error
end

It is very desirable to have a standard interface in Ruby for emitting structured warnings.

Proposal

I'd like to extend the current implementation to allow all options to be forwarded to Warning.warn. This would allow us to add more details to warnings and emit structured logs using Warning.warn.

A simple example of the proposed interface:

module Kernel
  def warn(*arguments, uplevel: ..., **options)
    # Existing processing of arguments -> message
    ::Warning.warn(message, **options)
  end
end

Current behaviour rejects any unknown options:

warn("Oops", exception: error)
# => <internal:warning>:50:in `warn': unknown keyword: :exception (ArgumentError)

I don't have an opinion about the implementation, but I wanted to get feedback on the interface.

Regarding the default behaviour, I propose no changes.

[zverok (Victor Shepelev)]

I wonder why error: in particular, and not just warn(text, **any_structured_metadata)?..

[Eregon (Benoit Daloze)]

Why not include the error directly in the warning message?
It seems that would just work better with the whole Warning.warn system and existing handling of warnings.

[ioquatix (Samuel Williams)]

I wonder why error: in particular, and not just warn(text, **any_structured_metadata)?..

I'd be okay with that, in fact I was thinking of making that proposal but I wasn't sure it would be accepted if it was too general.

Why not include the error directly in the warning message?

Good question! Unfortunately, I found that the formatting behaviour of Warning.warn is a bit problematic. The first argument to Warning.warn is a string concatenation of all arguments passed to Kernel#warn. Depending on the log output, I want to include the details of the error in a JSON format for structured logging. In order to do that, I need the original objects passed to warn. This includes adding trace metadata which may not be known about until later on in the output handling.

One option would be to do the formatting at the point of the Kernel#warn call. However, this implies that we know what formatting is desired at that point (we don't), and greatly complicates the usage of the interface.

It may be the case that warn was not designed to be used that way, and I'm okay if that's the case. But I think this is a useful and generic extension point for logging. If we can allow extra meta-data (that can be ignored by default) to go via **options, it significantly increases the usefulness of this interface - we can pick up those options in Warning.warn and use those to control the output accordingly.

[Dan0042 (Daniel DeLorme)]

I think I agree with the general idea but I'm not sure if the proposal is what I think it is. It's a bit thin about what happens with this "error" keyword exactly. So is the idea that this meta-information would be forwarded to another method to decide what to do with it? Are we talking about extending Warning.warn to work like: warn(*args, **opts) -> Warning.warn(formatted_str, **opts)

[ioquatix (Samuel Williams)]

Thanks for your feedback. The proposal is deliberately thin - as I said, I don't know exactly how we should adjust the internal interface to handle this (actually I don't have a strong opinion about it). The main point is, to allow for an additional keyword argument. I think Warning.warn should probably receive that keyword argument, and maybe print it using error.full_message or something similar. I don't think it should be part of formatted_str but I think the default implementation could print it separately?

[ioquatix (Samuel Williams)]

• Description updated (diff)

[ioquatix (Samuel Williams)]

• Subject changed from Support `error:` keyword to `Kernel#warn` to Allow `Kernel#warn` to accept `**options` and pass these to `Warning.warn`.
• Description updated (diff)

[ioquatix (Samuel Williams)]

According to the discussion I've updated the proposal.

[ioquatix (Samuel Williams)]

• Description updated (diff)

[jeremyevans0 (Jeremy Evans)]

@ioquatix (Samuel Williams) asked for my feedback on this. I'm against Warning.warn taking **opts for structured logging. We have an existing category keyword option that it would conflict with, and it would prevent the backwards compatible addition of other keyword arguments. If we want to support structured logging in Warning.warn, we should have a keyword argument specifically for the structured logging information. We should also have a description of how the structured logging information should be handled by default.

[ioquatix (Samuel Williams)]

• Description updated (diff)

We should also have a description of how the structured logging information should be handled by default.

Regarding the default behaviour, I propose no changes.

As an alternative, I'd also be open to dumping it, e.g. using pp.

We have an existing category keyword option that it would conflict with, and it would prevent the backwards compatible addition of other keyword arguments.

I don't completely agree with the former part of this statement, but the latter part is fair. It's a toss up for usability and whether we think that we will add more keywords in the future.

e.g.

# Jeremy's preference (some top level keyword argument):
warn "something went wrong", extra: {exception:}

# vs the current proposal:
warn "something went wrong", exception:

I slightly lean towards the usability aspect for the consumer of the API, but I understand Jeremy's concern regarding the long term compatibility of the internal interface Warning.warn.

[ioquatix (Samuel Williams)]

I checked C interface in error.h and we end up having a lot of variants:

• rb_warn
• rb_warning
• rb_category_warn
• rb_compile_warn
• rb_category_compile_warn

etc.

One idea is this:

struct rb_warning_arguments {
  rb_warning_category_t category;
  VALUE extra;
  // others
};

void rb_warning(struct rb_warning_arguments, const char *fmt, ...);

For things like:

void rb_category_compile_warn(rb_warning_category_t cat, const char *file, int line, const char *fmt, ...);

I imagine that cat, file and line are all useful structured data.

[ko1 (Koichi Sasada)]

Compatibility issues for user defined warn methods?

$ ~/gem-codesearch/gem-codesearch 'def warn\b' | wc -l
3837

[ioquatix (Samuel Williams)]

For previous discussion including compatibility, see https://bugs.ruby-lang.org/issues/17122

[shyouhei (Shyouhei Urabe)]

I also want something like this. In order to move things forward we have to clear some points:

• There could be programs that redefine Kernel#warn on the fly in the wild. They might suffer from the API change. About the only thing we can do here is let them die miserably. Is it an acceptable choice? (I guess so though)

• The other issue is Warning.warn side. We already have it, which also doesn't expect unknown arguments for now. We acknowledge that Warning.warn could take exactly one argument (per https://bugs.ruby-lang.org/issues/17122#note-15). We are checking if it takes category: every time now. If we were to pass arbitrary keyword arguments to Warning.warn is the check... kind of possible in practice?

These two might be related, but are not the same thing.

[Eregon (Benoit Daloze)]

From the experience of mspec or ruby/spec which used to override Kernel#warn it's very confusing and messy, and I removed that.
Overriding Kernel#warn typically also breaks uplevel handling and might also break the recursion check (some people user super in Warning.warn).
So I think one should never override Kernel#warn, especially since we have Warning.warn now as the designated extension point.

I wonder if we really need structured warnings.
Warnings at least currently seem primarily meant for developers, and I don't really see the value for structured warnings there.
So I think it would be good to have a concrete example where this would be useful and it's not trivially worked around.

I would think if we want structured something it would make more sense for logging and for exceptions.
Warnings are not logging.