Data argument supplied to Telemetry#record fails when it is a Hash

[ntl]

include 'telemetry'

class SomeSink
  include Telemetry::Sink
  record :some_signal
end

sink = SomeSink.new

telemetry = Telemetry.build
telemetry.register sink

# does not raise an error
data = "not a hash"
telemetry.record(:some_signal, data)

# raises error!
data = { :some_key => 'some value' }
telemetry.record(:some_signal, data)

The problem is ultimately that, if data is a hash, then ruby considers it a container of keyword arguments. This is because the sink's record method has a keyword argument, force.

[sbellware]

Yep. That's a well-known problem, unfortunately, and has caused design problems on a number of occasions. Not sure what to do about it other than remove the force argument, or make it a positional argument. That just seems like admitting defeat to one of Ruby's worst language design decisions, though.

But... I haven't been in a position to record a hash as the telemetry data. I usually create a semantically-significant class that represents the data. So, I could say that storing a hash in telemetry data sounds like an avoidance of the necessary work to create domain-specific telemetry data. But at the same time, I know that is as much a cop out as Ruby's design of binding keyword arguments to hashes just to make them compatible with Rails.

Dunno. I don't know if there's a good answer for this. The best answer is changing Ruby so that it does the right thing. I don't think the best answer is going to be available though.

[ntl]

Yeah, I've already moved to defining Struct classes for the telemetry records. Actually, the appropriate resolution for this problem might be to simply ensure our documentation contains a caveat.

[sbellware]

That would mean we'd have to add that caveat anywhere that this proper functioning or Ruby is unknown to our users, which is any method whose last argument is a named argument that is preceded by a positional argument.

I see that it could be helpful for users, but at the same time, I could see that this is something that should be considered common-ish knowledge to Ruby devs.

I'm inclined to see this as a problem for @matz than for library developers. I'm not convinced of that stance though. It's a bit willfully negligent. But at the same time... I have a hard time seeing us wanting to document this problem every time we use this Ruby feature.

[ntl]

I can buy that line of reasoning, but there's something else that's different here -- passing a hash as a value for telemetry data is not actually unreasonable. I.E., I don't see any reason why telemetry data couldn't be be primitive (except this issue making it impossible). That's why I'd suggest documenting this case; users are likely to expect to be able to pass in a Hash, and the current experience when you do so is a painful debugging session. I'd wager that in most of the other cases where we have keyword arguments, a hash wouldn't even make sense as the final positional argument.

[sbellware]

Actually, there have been a handful of other cases where this thing has happened. So, it's made sense a number of times, and we've been denied the desirable design because of Ruby's decision to bind hashes to named args.

It's totally reasonable to expect to be able to use a hash as telemetry data. It sucks that this is the outcome. Either way, it is painful, and should be noted in documentation.

[matz]

That will be fixed in the future version. Please submit a bug report to bugs.ruby-lang.org for the record.

[sbellware]

That's really good news, @matz. Thanks for the notification.

We'll write up a bug report and submit it.