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

draft: add extended-span-chat-model-listener #1260

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

flyinfish
Copy link

enabling adding more/custom attributes to span, namely before span is closed

for onRequest this would already be possible just using another ChatModelListener. not so unfortunately for onResponse as span is ended in SpanChatModelListener.onResponse() and so additional attributes land on parent-span overwriting each other with every new request.

this change is just missing some Processor Magic producing a default-ExtendedSpanChatModelListener and some docs may-be.

@flyinfish flyinfish requested a review from a team as a code owner February 3, 2025 07:20
@flyinfish flyinfish marked this pull request as draft February 3, 2025 07:20
@geoand
Copy link
Collaborator

geoand commented Feb 3, 2025

Interesting!

I will have a closer look in a few days as I am traveling

/**
* extended ChatModelListener adding possibility to add custom attributes to span
*/
public interface ExtendedSpanChatModelListener {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would probably call this ChatModelSpanEnhancer

Copy link
Author

Choose a reason for hiding this comment

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

if you like it i can rename and make it merge-ready

Copy link
Collaborator

Choose a reason for hiding this comment

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

I want to take a closer look in a few days.

But the PR will also need to make sure that the beans are picked up (if they exist) and work regardless of whether such beans are present

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought about this a little more and I like the approach a lot.
So let's make sure it works under all circumstances (ideally with some tests) and also document it.

enabling adding more/custom attributes to span, namely  before span is closed

 quarkiverse#1256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants