Skip to content

GH-2518: Remove requestOptions from observation context objects #2896

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

Closed
wants to merge 1 commit into from

Conversation

sobychacko
Copy link
Contributor

Fixes: #2518

Issue: #2518

This commit removes the deprecated requestOptions field from ChatModelObservationContext and EmbeddingModelObservationContext classes. Instead of passing options separately, the code now retrieves them directly from the request objects (prompt.getOptions() or embeddingRequest.getOptions()).

Key changes:

  • Removed requestOptions parameter from observation context builders
  • Updated all model implementations to stop passing options separately
  • Fixed EmbeddingRequest handling in several model implementations
  • Added buildEmbeddingRequest method in models to properly merge options

This change simplifies the API and removes duplication, as options are already available in the request objects themselves.

…t objects

Fixes: spring-projects#2518

Issue: spring-projects#2518

This commit removes the deprecated requestOptions field from ChatModelObservationContext
and EmbeddingModelObservationContext classes. Instead of passing options separately,
the code now retrieves them directly from the request objects (prompt.getOptions() or
embeddingRequest.getOptions()).

Key changes:
- Removed requestOptions parameter from observation context builders
- Updated all model implementations to stop passing options separately
- Fixed EmbeddingRequest handling in several model implementations
- Added buildEmbeddingRequest method in models to properly merge options

This change simplifies the API and removes duplication, as options are already
available in the request objects themselves.

Signed-off-by: Soby Chacko <[email protected]>
return "%s %s".formatted(context.getOperationMetadata().operationType(),
context.getRequestOptions().getModel());
ChatOptions options = context.getRequest().getOptions();
if (StringUtils.hasText(options.getModel())) {
Copy link
Contributor

@ThomasVitale ThomasVitale Apr 26, 2025

Choose a reason for hiding this comment

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

Unfortunately, a Prompt object can return null ChatOptions. Before, when handling options separately, we were enforcing the non-nullability. Now, we need to take into account that whenever we call the getOptions() method from a Prompt object, we might get back a null object. This line might throw a NullPointerException. The same applies to the other places here an in the embedding observations where we extract values from ChatOptions.

It would be nice if, in the future, we could enhance the Prompt class to always return a valid ChatOptions object.

Copy link
Member

@markpollack markpollack Apr 28, 2025

Choose a reason for hiding this comment

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

yes, it seems we could remove the nullability and return an empty ChatOptions instance. This should send no options on the wire to the model.

Copy link
Member

Choose a reason for hiding this comment

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

This is addressed via 4eeeb83

@ilayaperumalg ilayaperumalg self-assigned this Apr 28, 2025
@ilayaperumalg ilayaperumalg added this to the 1.0.0-RC1 milestone Apr 28, 2025
@ilayaperumalg
Copy link
Member

Rebased and merged as f18aac0

@ilayaperumalg ilayaperumalg modified the milestones: 1.0.0-RC1, 1.0.0-M8 Apr 28, 2025
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.

Remove deprecation in ObservationContext for models
4 participants