Skip to content

Conversation

Cirilla-zmh
Copy link
Member

No description provided.

@otelbot-java-instrumentation
Copy link
Contributor

🔧 The result from spotlessApply was committed to the PR branch.

*/
@AutoValue
@JsonClassDescription("Generic part")
public abstract class GenericPart implements MessagePart {
Copy link
Member Author

Choose a reason for hiding this comment

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

jackson annotations or io.opentelemetry.api.common.Value?

* @param chunkMessage the chunk message to append
* @return a new OutputMessages instance with the merged content
*/
public OutputMessages merge(int index, OutputMessage chunkMessage) {
Copy link
Member Author

Choose a reason for hiding this comment

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

If possible, abstracting all StreamListener types into a single interface might be a better choice. It should have an onChunk(OutputMessages) method, an onEnd() method, and an onError(Throwable) method. We can complete the common collection logic here uniformly — such as the aggregation of OutputMessages, the captures of usage tokens and time per output chunk , etc.

How do you think about that? cc @anuraaga

import javax.annotation.Nullable;

// as a field of Attributes Extractor
// replace the 'ChatCompletionEventsHelper'
Copy link
Member Author

Choose a reason for hiding this comment

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

By combining this instance into the GenAiAttributesExtractor, we can uniformly manage various collection switches in the AttributesExtractor — whether to collect chat history, whether to collect as span attributes or as logs, etc.

However, this also means that I might need to emit events in the AttributesExtractor, and I'm not sure if this is the right approach cc @trask. But according to the current semantic specification, we need to record some necessary attributes in events — and these attributes can almost only be obtained in the AttributesExtractor and SpanProcessor.

If we don't do this in the AttributesExtractor, the best approach might be to define an EventsExtractor in the Instrumenter and allow it to read the current attributes like:

public interface EventsExtractor {
  public Context onStart(Attributes startAttributes, Context context);
  public void onEnd(Attributes endAttributes, Context context);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

OperationListener maybe another nice choice.

OutputMessage.create(
Role.ASSISTANT,
messageParts,
choice.finishReason().asString()));
Copy link
Member Author

Choose a reason for hiding this comment

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

Each choice has a finish_reason, so which one should be recorded in the span's gen_ai.response.finish_reasons at this point?

} else if (msg.isUser()) {
inputMessages.append(InputMessage.create(Role.USER, contentToMessageParts(msg.asUser().content())));
} else if (msg.isAssistant()) {
ChatCompletionAssistantMessageParam assistantMsg = msg.asAssistant();
Copy link
Member Author

Choose a reason for hiding this comment

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

Should these parts be recorded in the same ChatMessage?

In multi-turn conversation scenarios, there may be multiple 'assistant messages' in the 'input messages' - should all of them be recorded? (I think we should truthfully record the input of each call, regardless of whether it's a multi-turn conversation or not.)

@Cirilla-zmh
Copy link
Member Author

Cirilla-zmh commented Sep 8, 2025

So far, spring-ai has provided proxies for many model service SDKs, such as openai, gemini, etc. Some of these may already have dedicated instrumentation, and if spring-ai instrumentation is enabled, it will generate two duplicate inference spans. Therefore, we have two choices:

  • Do not implement spring-ai instrumentation at the model layer, relying entirely on the SDK's native instrumentation. However, some additional information may not be accessible (such as metadata defined in spring ai).

  • Similar to spring-kafka, also implement instrumentation and add a ProcessTracingState. Suppress the SDK's native instrumentation through context:

// have separate copies of helper classes.
public final class KafkaClientsConsumerProcessTracing {
private static final ThreadLocal<Boolean> wrappingEnabled = ThreadLocal.withInitial(() -> true);
private KafkaClientsConsumerProcessTracing() {}
public static boolean setEnabled(boolean enabled) {
boolean previous = wrappingEnabled.get();
wrappingEnabled.set(enabled);
return previous;
}
public static boolean wrappingEnabled() {
return wrappingEnabled.get();
}
public static BooleanSupplier wrappingEnabledSupplier() {
return KafkaClientsConsumerProcessTracing::wrappingEnabled;
}

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.

1 participant