Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAI: instrument embeddings and chat completions #2398

Merged
merged 77 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
77 commits
Select commit Hold shift + click to select a range
0c348d0
OpenAI instrumentation scaffold
kaylareopelle Jan 18, 2024
0d9e3fd
Rubocop
kaylareopelle Jan 18, 2024
9fb700c
Instrument json_post method
kaylareopelle Jan 23, 2024
11bf4d4
Draft json_post instrumentation to get response header attrs
kaylareopelle Jan 23, 2024
3a9b12a
Cleanup
kaylareopelle Jan 24, 2024
c372d90
OpenAI instrumentation scaffold
kaylareopelle Jan 18, 2024
543e92c
Rubocop
kaylareopelle Jan 18, 2024
90e5d97
Instrument json_post method
kaylareopelle Jan 23, 2024
cc5b2cc
Draft json_post instrumentation to get response header attrs
kaylareopelle Jan 23, 2024
8ec46d9
Cleanup
kaylareopelle Jan 24, 2024
9be9588
Sketch out some attribute assignments
kaylareopelle Jan 30, 2024
53864e5
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Jan 30, 2024
6854a8b
Add chat completion event params
hannahramadan Jan 30, 2024
adf18cc
Document embeddings attributes
hannahramadan Jan 30, 2024
2c352b8
Add embedding event details
hannahramadan Jan 31, 2024
76cb29d
Add attributes for messages
kaylareopelle Jan 31, 2024
42adee3
Test hash to assign correct string values to custom event attributes
kaylareopelle Jan 31, 2024
477053f
Update attribute names to period-delimited strings
kaylareopelle Jan 31, 2024
cadb30e
Default LlmEvent ID value to guid, allow passed-in arg to override
kaylareopelle Jan 31, 2024
118dbda
Separate instrumentation methods
hannahramadan Jan 31, 2024
da7692d
Add conversation_id from transaction custom attributes
kaylareopelle Jan 31, 2024
3146677
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Jan 31, 2024
c179385
Move post-response operations out of begin block
hannahramadan Jan 31, 2024
e1a70c9
Minor instrumentation refactors and comments
kaylareopelle Feb 1, 2024
4cbcc61
Add multiverse tests for openai requests
kaylareopelle Feb 1, 2024
bf0eecf
Add more tests for chat completions
kaylareopelle Feb 2, 2024
00e0453
Raise in Net::HTTP for the unwritten tests related to OpenAI
kaylareopelle Feb 2, 2024
b6bd502
Rubocop
kaylareopelle Feb 2, 2024
b84945c
Create AI monitoring suite
kaylareopelle Feb 2, 2024
b57e021
Update spelling for OpenAI constant
kaylareopelle Feb 2, 2024
d0386d6
Merge branch 'dev' into openai
kaylareopelle Feb 2, 2024
be65cc5
Create set_llm_agent_attribute_on_error_transaction
kaylareopelle Feb 2, 2024
6377fcf
Add llm agent attribute to transactions with OpenAI segments
kaylareopelle Feb 2, 2024
8ffcd47
Misc cleanup
kaylareopelle Feb 5, 2024
004dcd3
Move response organization and request id to ResponseHeaders
kaylareopelle Feb 5, 2024
36d9e83
Be prepared for String or Symbol
hannahramadan Feb 5, 2024
9a75463
Rubocop cleanup
hannahramadan Feb 5, 2024
2be5e44
Pluralize embedding-relate methods
hannahramadan Feb 5, 2024
d083888
Pass name keyword arg to embedding start segment
kaylareopelle Feb 5, 2024
208b971
Add embedding tests
hannahramadan Feb 6, 2024
6136eab
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Feb 6, 2024
b2b166d
Rubocop
hannahramadan Feb 6, 2024
d40461a
WIP commit refactor for multiple version support
kaylareopelle Feb 7, 2024
d627f15
WIP update tests for 3.4.0 error handling
kaylareopelle Feb 7, 2024
e23c734
WIP Fix tests for errors in 3.4.0
kaylareopelle Feb 7, 2024
5ffbf9e
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Feb 7, 2024
4c60bae
Support 3.4.0+
kaylareopelle Feb 7, 2024
98c2b29
Test fix for Ruby 2.4
hannahramadan Feb 7, 2024
2451bdf
Update comments in Envfile
kaylareopelle Feb 7, 2024
8a10359
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Feb 7, 2024
cadfc35
llm_event attribute on txn test
hannahramadan Feb 7, 2024
5fed88f
use symbols v strings for testing
hannahramadan Feb 7, 2024
8cb0072
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Feb 8, 2024
fd1401c
Add "Supportability" to the OpenAI metric
kaylareopelle Feb 8, 2024
a8687b7
Addtl supportability metric updates
kaylareopelle Feb 8, 2024
3dac7e0
Add setup for tests, llm_event test refactor
hannahramadan Feb 8, 2024
585327d
Update OpenAI segment names
kaylareopelle Feb 10, 2024
f15925d
Refactor supportability metric
hannahramadan Feb 12, 2024
8ac6b0f
Add missing params test & code change
hannahramadan Feb 12, 2024
c2c9959
appease rubocop
hannahramadan Feb 12, 2024
b486d23
Remove unused method
hannahramadan Feb 12, 2024
9985865
Merge branch 'openai' of github.com:newrelic/newrelic-ruby-agent into…
kaylareopelle Feb 12, 2024
7b1484d
Update Net::HTTP AI methods to be OpenAI-specific
kaylareopelle Feb 12, 2024
f79a2f6
Add chat completions segment name constant back
kaylareopelle Feb 12, 2024
55dd343
Add openai to jruby CI
kaylareopelle Feb 12, 2024
7273d1f
Code review
kaylareopelle Feb 13, 2024
c83a892
Only send llm attributes to DST_TRANSACTION_EVENTS
hannahramadan Feb 13, 2024
36d6cab
Conditonally merge depending on Ruby version
hannahramadan Feb 13, 2024
4e26f74
Remove tests that look for a txn in error collector
hannahramadan Feb 13, 2024
85f7d51
Don't run flaky test
hannahramadan Feb 13, 2024
22968ea
Remove unit test assert for txn found in error collector
hannahramadan Feb 13, 2024
90b7ce9
Update capitalization for OpenAI in segment names
kaylareopelle Feb 13, 2024
0359ae1
Remove placeholder Net::HTTP tests for response headers
kaylareopelle Feb 13, 2024
d69d404
Rubocop
kaylareopelle Feb 13, 2024
cdd42f6
is_response: true - code feedback
hannahramadan Feb 13, 2024
a8092e0
Update lib/new_relic/agent/instrumentation/ruby_openai/instrumentatio…
kaylareopelle Feb 14, 2024
d43b1a6
Freeze openai regex
hannahramadan Feb 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ jobs:
strategy:
fail-fast: false
matrix:
multiverse: [agent, background, background_2, database, frameworks, httpclients, httpclients_2, rails, rest]
multiverse: [agent, ai, background, background_2, database, frameworks, httpclients, httpclients_2, rails, rest]
ruby-version: [2.4.10, 3.3.0]

steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci_cron.yml
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ jobs:
strategy:
fail-fast: false
matrix:
multiverse: [agent, background, background_2, database, frameworks, httpclients, httpclients_2, rails, rest]
multiverse: [agent, ai, background, background_2, database, frameworks, httpclients, httpclients_2, rails, rest]
ruby-version: [2.4.10, 2.5.9, 2.6.10, 2.7.8, 3.0.6, 3.1.4, 3.2.2, 3.3.0]
steps:
- name: Configure git
Expand Down
9 changes: 9 additions & 0 deletions lib/new_relic/agent/configuration/default_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1570,6 +1570,15 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
:allowed_from_server => false,
:description => 'Controls auto-instrumentation of `Net::HTTP` at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.'
},
:'instrumentation.ruby_openai' => {
:default => 'auto',
:documentation_default => 'auto',
:public => true,
:type => String,
:dynamic_name => true,
:allowed_from_server => false,
:description => 'Controls auto-instrumentation of the ruby-openai gem at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.'
},
:'instrumentation.puma_rack' => {
:default => value_of(:'instrumentation.rack'),
:documentation_default => 'auto',
Expand Down
19 changes: 19 additions & 0 deletions lib/new_relic/agent/instrumentation/net_http/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,31 @@ def request_with_tracing(request)
end

wrapped_response = NewRelic::Agent::HTTPClients::NetHTTPResponse.new(response)
add_llm_response_headers(wrapped_response, segment.parent) if llm_parent?(segment)
segment.process_response_headers(wrapped_response)
response
ensure
segment&.finish
end
end

def llm_parent?(segment)
segment&.parent&.name&.match?(/Llm\/.*\/OpenAI\/create/)
end

def add_llm_response_headers(response, parent)
return unless parent.instance_variable_defined?(:@chat_completion_summary) || parent.instance_variable_defined?(:@embedding) # and maybe log a warning??

if parent.instance_variable_defined?(:@chat_completion_summary)
event = parent.chat_completion_summary
event.request_id = response[NewRelic::Agent::Llm::LlmEvent::X_REQUEST_ID] # every event needs this, maybe we should move it someplace else?
event.populate_openai_response_headers(response.to_hash)
elsif parent.instance_variable_defined?(:@embedding)
event = parent.embedding
event.request_id = response[NewRelic::Agent::Llm::LlmEvent::X_REQUEST_ID] # every event needs this, maybe we should move it someplace else?
event.populate_openai_response_headers(response.to_hash)
end
end
end
end
end
Expand Down
33 changes: 33 additions & 0 deletions lib/new_relic/agent/instrumentation/ruby_openai.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

require_relative 'ruby_openai/instrumentation'
require_relative 'ruby_openai/chain'
require_relative 'ruby_openai/prepend'

DependencyDetection.defer do
named :'ruby_openai'

depends_on do
defined?(OpenAI) && defined?(OpenAI::Client)
# maybe add DT check here eventually?
# possibly also a config check for ai.enabled
end

executes do
NewRelic::Agent.logger.info('Installing ruby-openai instrumentation')

if use_prepend?
# instead of metaprogramming on OpenAI::Client, we could also use OpenAI::HTTP,
# it's a module that's required by OpenAI::Client and contains the
# json_post method we're instrumenting
prepend_instrument OpenAI::Client,
NewRelic::Agent::Instrumentation::OpenAI::Prepend,
NewRelic::Agent::Instrumentation::OpenAI::VENDOR
else
chain_instrument NewRelic::Agent::Instrumentation::OpenAI::Chain,
NewRelic::Agent::Instrumentation::OpenAI::VENDOR
end
end
end
21 changes: 21 additions & 0 deletions lib/new_relic/agent/instrumentation/ruby_openai/chain.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

module NewRelic::Agent::Instrumentation
module OpenAI::Chain
def self.instrument!
::OpenAI::Client.class_eval do
include NewRelic::Agent::Instrumentation::OpenAI

alias_method(:json_post_without_new_relic, :json_post)

def json_post(**kwargs)
json_post_with_new_relic(**kwargs) do
json_post_without_new_relic(**kwargs)
end
end
end
end
end
end
180 changes: 180 additions & 0 deletions lib/new_relic/agent/instrumentation/ruby_openai/instrumentation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

module NewRelic::Agent::Instrumentation
module OpenAI
VENDOR = 'OpenAI' # or SUPPORTBILITY_NAME? or both?
# TODO: should everything below be called embeddings if we renamed to chat completions?
EMBEDDINGS_PATH = '/embeddings'
CHAT_COMPLETIONS_PATH = '/chat/completions'
SEGMENT_NAME_FORMAT = 'Llm/%s/OpenAI/create'

# This method is defined in the OpenAI::HTTP module that is included
# only in the OpenAI::Client class
def json_post_with_new_relic(path:, parameters:)
if path == EMBEDDINGS_PATH
NewRelic::Agent.record_instrumentation_invocation(VENDOR)
embedding_instrumentation(parameters) { yield }
elsif path == CHAT_COMPLETIONS_PATH
NewRelic::Agent.record_instrumentation_invocation(VENDOR)
chat_completions_instrumentation(parameters) { yield }
else
yield
end
end

private

def embedding_instrumentation(parameters)
segment = NewRelic::Agent::Tracer.start_segment(SEGMENT_NAME_FORMAT % 'embedding')
record_openai_metric
event = create_embedding_event(parameters)
segment.embedding = event
begin
response = NewRelic::Agent::Tracer.capture_segment_error(segment) { yield }

response
ensure
add_embedding_response_params(response, event) if response
segment&.finish
event&.error = true if segment_noticed_error?(segment) # need to test throwing an error
event&.duration = segment&.duration
event&.record # always record the event
end
end

def chat_completions_instrumentation(parameters)
# TODO: Do we have to start the segment outside the ensure block?
segment = NewRelic::Agent::Tracer.start_segment(name: SEGMENT_NAME_FORMAT % 'completion')
record_openai_metric
event = create_chat_completion_summary(parameters)
segment.chat_completion_summary = event
messages = create_chat_completion_messages(parameters, summary_event_id)
response = NewRelic::Agent::Tracer.capture_segment_error(segment) { yield }
add_response_params(parameters, response, event) if response
messages = update_chat_completion_messages(messages, response, event) if response

response # return the response to the original caller
ensure
segment&.finish
event&.error = true if segment_noticed_error?(segment)
event&.duration = segment&.duration
event&.record # always record the event
messages&.each { |m| m&.record }
end

def create_chat_completion_summary(parameters)
event = NewRelic::Agent::Llm::ChatCompletionSummary.new(
# metadata => TBD, create API
vendor: VENDOR,
conversation_id: conversation_id,
api_key_last_four_digits: parse_api_key,
# TODO: Determine how to access parameters with keys as strings
request_max_tokens: parameters[:max_tokens],
request_model: parameters[:model],
temperature: parameters[:temperature]
)
end

def create_embedding_event(parameters)
# TODO: Determine how to access parameters with keys as strings
event = NewRelic::Agent::Llm::Embedding.new(
# metadata => TBD, create API
vendor: VENDOR,
input: parameters[:input],
api_key_last_four_digits: parse_api_key,
request_model: parameters[:model]
)
end

def add_response_params(parameters, response, event)
event.response_number_of_messages = parameters[:messages].size + response['choices'].size
event.response_model = response['model']
event.response_usage_total_tokens = response['usage']['total_tokens']
event.response_usage_prompt_tokens = response['usage']['prompt_tokens']
event.response_usage_completion_tokens = response['usage']['completion_tokens']
event.response_choices_finish_reason = response['choices'][0]['finish_reason']
end

def add_embedding_response_params(response, event)
event.response_model = response['model']
event.response_usage_total_tokens = response['usage']['total_tokens']
event.response_usage_prompt_tokens = response['usage']['prompt_tokens']
end

def parse_api_key
'sk-' + headers['Authorization'][-4..-1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... it looks like the ability to use [-4..] instead of [-4..-1] wasn't introduced until Ruby v2.6.

end

# The customer must call add_custom_attributes with conversation_id before
# the transaction starts. Otherwise, the conversation_id will be nil
def conversation_id
return @nr_conversation_id if @nr_conversation_id

@nr_conversation_id ||= NewRelic::Agent::Tracer.current_transaction.attributes.custom_attributes['conversation_id']
end

def create_chat_completion_messages(parameters, summary_id)
# TODO: Determine how to access parameters with keys as strings
parameters[:messages].map.with_index do |message, i|
NewRelic::Agent::Llm::ChatCompletionMessage.new(
content: message[:content] || message['content'],
role: message[:role] || message['role'],
sequence: i,
completion_id: summary_id,
vendor: VENDOR,
is_response: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the spec, this shouldn't be set to false, it should be not included if it would be false

set to True if a message is the result of a chat completion and not an input message - omitted in False cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout! Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been updated! cdd42f6

)
end
end

def create_chat_completion_response_messages(response, sequence_origin, summary_id)
response['choices'].map.with_index(sequence_origin) do |choice, i|
NewRelic::Agent::Llm::ChatCompletionMessage.new(
content: choice['message']['content'],
role: choice['message']['role'],
sequence: i,
completion_id: summary_id,
vendor: VENDOR,
is_response: true
)
end
end

def update_chat_completion_messages(messages, response, summary)
messages += create_chat_completion_response_messages(response, messages.size, summary.id)
response_id = response['id'] || NewRelic::Agent::GuidGenerator.generate_guid

messages.each do |message|
# metadata => TBD, create API
message.id = "#{response_id}-#{message.sequence}"
message.conversation_id = conversation_id
message.request_id = summary.request_id
message.response_model = response['model']
end
end

# Name is defined in Ruby 3.0+
# copied from rails code
# Parameter keys might be symbols and might be strings
# response body keys have always been strings
def hash_with_indifferent_access_whatever
if Symbol.method_defined?(:name)
key.kind_of?(Symbol) ? key.name : key
else
key.kind_of?(Symbol) ? key.to_s : key
end
end

# the preceding :: are necessary to access the OpenAI module defined in the gem rather than the current module
# TODO: discover whether this metric name should be prepended with 'Supportability'
def record_openai_metric
NewRelic::Agent.record_metric("Ruby/ML/OpenAI/#{::OpenAI::VERSION}", 0.0)
end

def segment_noticed_error?(segment)
segment&.instance_variable_get(:@noticed_error)
end
end
end
13 changes: 13 additions & 0 deletions lib/new_relic/agent/instrumentation/ruby_openai/prepend.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

module NewRelic::Agent::Instrumentation
module OpenAI::Prepend
include NewRelic::Agent::Instrumentation::OpenAI

def json_post(**kwargs)
json_post_with_new_relic(**kwargs) { super }
end
end
end
14 changes: 14 additions & 0 deletions lib/new_relic/agent/llm/chat_completion_summary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ class ChatCompletionSummary < LlmEvent
response_usage_total_tokens response_usage_prompt_tokens
response_usage_completion_tokens response_choices_finish_reason
request_temperature duration error]
ATTRIBUTE_NAME_EXCEPTIONS = {
response_number_of_messages: 'response.number_of_messages',
request_model: 'request.model',
response_usage_total_tokens: 'response.usage.total_tokens',
response_usage_prompt_tokens: 'response.usage.prompt_tokens',
response_usage_completion_tokens: 'response.usage.completion_tokens',
response_choices_finish_reason: 'response.choices.finish_reason',
temperature: 'request.temperature'
}

EVENT_NAME = 'LlmChatCompletionSummary'

attr_accessor(*ATTRIBUTES)
Expand All @@ -24,6 +34,10 @@ def attributes
LlmEvent::ATTRIBUTES + ChatCompletion::ATTRIBUTES + ResponseHeaders::ATTRIBUTES + ATTRIBUTES
end

def attribute_name_exceptions
LlmEvent::ATTRIBUTE_NAME_EXCEPTIONS.merge(ResponseHeaders::ATTRIBUTE_NAME_EXCEPTIONS, ATTRIBUTE_NAME_EXCEPTIONS)
end

def event_name
EVENT_NAME
end
Expand Down
13 changes: 11 additions & 2 deletions lib/new_relic/agent/llm/embedding.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ class Embedding < LlmEvent
include ResponseHeaders

ATTRIBUTES = %i[input api_key_last_four_digits request_model
response_organization response_usage_total_tokens
response_usage_prompt_tokens duration error]
response_usage_total_tokens response_usage_prompt_tokens duration
error]
ATTRIBUTE_NAME_EXCEPTIONS = {
request_model: 'request.model',
response_usage_total_tokens: 'response.usage.total_tokens',
response_usage_prompt_tokens: 'response.usage.prompt_tokens'
}
EVENT_NAME = 'LlmEmbedding'

attr_accessor(*ATTRIBUTES)
Expand All @@ -19,6 +24,10 @@ def attributes
LlmEvent::ATTRIBUTES + ResponseHeaders::ATTRIBUTES + ATTRIBUTES
end

def attribute_name_exceptions
LlmEvent::ATTRIBUTE_NAME_EXCEPTIONS.merge(ResponseHeaders::ATTRIBUTE_NAME_EXCEPTIONS, ATTRIBUTE_NAME_EXCEPTIONS)
end

def event_name
EVENT_NAME
end
Expand Down
Loading
Loading