Add support for llm. custom attributes

[hannahramadan]

Adds custom attributes to LLM events when prefixed with llm..

Closes #2437

[fallwith]

Would you please explain how the metadata attribute is designed to work? I'm specifically curious about why it is on its own here instead of being included in the ATTRIBUTES array.

Currently all other attributes can be sent in at initialize time.

Setting aside metadata, your new test code that looks like this:

event = NewRelic::Agent::Llm::LlmEvent.new(id: 123)
event.vendor = 'OpenAI'
event.response_model = 'gpt-4'

could instead be written as this:

event = NewRelic::Agent::Llm::LlmEvent.new(id: 123, vendor: 'OpenAI', response_model: 'gpt-4')

The model class is flexible enough to allow you to specify a vendor and response model later, but if you know them at initialize time, it's easier to just pass them to new().

But now with the new :metadata attribute, we're breaking from that pattern and requiring that the attribute writer be used post initialize time. So we'd have to write this:

event = NewRelic::Agent::Llm::LlmEvent.new(id: 123, vendor: 'OpenAI', response_model: 'gpt-4')
event.metadata = {'Marathon' => '26.2', 'Ultra Marathon' => 'Ouch'}

instead of this:

event = NewRelic::Agent::Llm::LlmEvent.new(
  id: 123,
  vendor: 'OpenAI',
  response_model: 'gpt-4',
  metadata: {'Marathon' => '26.2', 'Ultra Marathon' => 'Ouch'})

If there's a good reason for having :metadata work differently, such as it not being known at initialize time, then we can add a code comment to explain that. Otherwise, we can just add :metadata to the existing ATTRIBUTES array to match existing attribute behavior. You shouldn't even need to refactor the new unit tests if you do.

[kaylareopelle]

I believe the metadata attribute needs to be assigned in a way that’s unique from all the other attributes.

Currently, the other attributes are assigned to the final event using metaprogramming within the NewRelic::Agent::Llm::LlmEvent#event_attributes method: 


newrelic-ruby-agent/lib/new_relic/agent/llm/llm_event.rb

Lines 37 to 41 in ad0dfa4

	def event_attributes
	attributes.each_with_object({}) do |attr, hash|
	hash[attr] = instance_variable_get(:"@#{attr}")
	end
	end

This means, the attributes that get sent to the custom event API are pretty much a direct copy of the attributes we iterate over. The trace_id instance var becomes trace_id: value.

We can’t assign metadata this way because the custom event API flattens nested hashes and separates the nesting with a period when it assigns attribute names. 


If we assign metadata like the other attributes, a structure like:
{ metadata: { child: ‘value’ }

would have the final custom event attribute:

metadata.child: value

The spec requires the customer-provided key is the key, without any reference to metadata, llm, or other prefixes. We need a way to assign the attribute as child: value.

By separating metadata from the rest of the attributes, we can avoid this assignment issue. 

However, this isn’t the only way we could achieve this. We could also try to flatten the metadata hash before recording the event attributes. We could update the inititalize method to address the concerns about not being able to assign the value on initialization.

I acknolwedge that this is a bit weird, so happy to work on something else that seems like a better design.


[kaylareopelle]

Also, with regards to the test code, it's currently written to test assignment in both ways so that we can make sure passing args to the initalizer and assigning using the attr_writer are both functional. I recognize this isn't well documented and is cluttering the reason behind the test, so we can rewrite those to be more specific.

[fallwith]

This one looks like a fun one to hit 100% branch coverage on.

[kaylareopelle]

Great updates, Hannah! One small assertion suggestion, but take it or leave it. 🌻

[github-actions]

SimpleCov Report

	Coverage	Threshold
Line	93.7%	93%
Branch	71.26%	71%