The block forms of Appsignal.add_params, Appsignal.add_session_data, and Appsignal.add_headers are evaluated lazily when the transaction is sampled, at completion. If such a block raises, Transaction#params, #session_data, and #request_headers catch it, log Exception while fetching params: <class>: <message>, and return nil.
Two things could be better here. First, the whole value for that key is dropped, so one raising block also discards any plain values and other working blocks added for the same key. SampleData#value merges the blocks and values in order, so only the raising block needs to be skipped. Second, the log does not say where the block was defined, so a customer cannot tell which of their blocks failed.
#1544 added this kind of handling for error blocks and lifecycle hooks. It logs the error class, the message, and the block's definition site, then carries on. Sample data blocks were left out of that PR because they do not cause the leak it fixes. The getters above already catch them, so completion still finishes and the OpenTelemetry context is still detached. This would be a logging and granularity improvement, not a bug fix.
A fix would guard the block call inside SampleData#value, log the failure with the block's source_location, and skip only that block so the other data for the key survives. There is no test for a raising sample data block today, so a fix should add one.
The block forms of
Appsignal.add_params,Appsignal.add_session_data, andAppsignal.add_headersare evaluated lazily when the transaction is sampled, at completion. If such a block raises,Transaction#params,#session_data, and#request_headerscatch it, logException while fetching params: <class>: <message>, and returnnil.Two things could be better here. First, the whole value for that key is dropped, so one raising block also discards any plain values and other working blocks added for the same key.
SampleData#valuemerges the blocks and values in order, so only the raising block needs to be skipped. Second, the log does not say where the block was defined, so a customer cannot tell which of their blocks failed.#1544 added this kind of handling for error blocks and lifecycle hooks. It logs the error class, the message, and the block's definition site, then carries on. Sample data blocks were left out of that PR because they do not cause the leak it fixes. The getters above already catch them, so completion still finishes and the OpenTelemetry context is still detached. This would be a logging and granularity improvement, not a bug fix.
A fix would guard the block call inside
SampleData#value, log the failure with the block'ssource_location, and skip only that block so the other data for the key survives. There is no test for a raising sample data block today, so a fix should add one.