OpentelemetryFinch failing on streams.

[ksmithut]

Using the Finch.stream/5 function fails with a %KeyError{} and detaches the :request_stop telemetry handler.

Expected behavior A clear and concise description of what you expected to happen.

I've tracked it down to this line

The issue is that when you're using the stream function, meta.result is going to have whatever the developer returned in their stream function, which is not guaranteed to have anything, let alone be a map. Maybe this is something that needs to be augmented in the Finch telemetry calls.

[tsloughter]

Is the stop event not only triggered when the stream ends?

It does sound like something that can be better resolved in Finch, but we should have a fix too to just call end without any metadata I guess.

[bryannaegele]

Yeah, this is a Finch issue. Telemetry metadata must always be a map and should always have the same keys for a given event.

[ksmithut]

This is the issue as I see it:

This is the result of a Finch.request()

request = Finch.build(:get, "https://hex.pm")
{:ok, %{status: 200}} = Finch.request(request, MyFinch)

Finch.request() returns a %Finch.Response{} struct in it's result tuple. The Finch.stream result is different:

acc = %{no_status: true, custom_data: true}
request = Finch.build(:get, "https://hex.pm")
{:ok, ^acc} = Finch.stream(request, MyFinch, acc, fn _event, acc -> acc end)

The result of stream doesn't return a %Finch.Response{}, it returns whatever the accumulator is, and that's the object that gets sent in the telemetry.

So is the consensus that Finch needs to keep track of the %Finch.Response{} data separate from that accumulator and send that in the telemetry data instead? If so, I'll go open up an issue over there.

[bryannaegele]

Yeah, I'm not sure what the answer is. If I had to wager a guess, I'd think finch streams either shouldn't emit the same telemetry as a normal request, or something would have to change for these user functions and when things are emitted.