Skip to content

Add "took" attribute to SearchHits/ReactiveSearchHits #2986

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
ELHARROUGUI opened this issue Oct 12, 2024 · 3 comments · Fixed by #2991
Closed

Add "took" attribute to SearchHits/ReactiveSearchHits #2986

ELHARROUGUI opened this issue Oct 12, 2024 · 3 comments · Fixed by #2991
Labels
type: enhancement A general enhancement

Comments

@ELHARROUGUI
Copy link
Contributor

I am working on a Spring API that integrates with Elasticsearch, and my current task involves migrating from the Elasticsearch Java API Client to Spring Data Elasticsearch. While doing so, I noticed that the response body in the Elasticsearch Java API Client includes a valuable attribute called "took", which shows the time (in milliseconds) it took to complete the request. However, this attribute is not accessible in the SearchHits (or ReactiveSearchHits) body in Spring Data Elasticsearch.

This feature is important for performance monitoring and debugging, as it allows developers to measure the responsiveness of Elasticsearch queries.

I propose adding support for retrieving the "took" attribute from SearchHits objects in Spring Data Elasticsearch, ensuring feature parity with the Elasticsearch Java API Client. This enhancement would improve the developer experience by providing essential timing information for queries.

I would be happy to contribute to this feature by submitting a PR if accepted. Thank you!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 12, 2024
@sothawo sothawo added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 14, 2024
@sothawo
Copy link
Collaborator

sothawo commented Oct 14, 2024

Fell free to add it; just a thought: I think we should use a better name than took, perhaps something like executionDuration and instead of using an int or long use a java.time.Duration

@ELHARROUGUI
Copy link
Contributor Author

ELHARROUGUI commented Oct 14, 2024

Thank you, @sothawo ! I can't think of a better name than executionDuration either :)

I just wanted to point out that in the Elasticsearch Java API, the attribute is declared as tookInMillis (as a private long), but they use getTook() as the getter. The getter returns a TimeValue object (org.elasticsearch.core.TimeValue), which encapsulates the duration.

Following a similar approach, we could retrieve the long value from the TimeValue object like this:
long executionDurationInMillis = searchDocumentResponse.getTook().millis();
Then, we could return a java.time.Duration object instead of a long:
Duration executionDuration = Duration.ofMillis(executionDurationInMillis);

P.S.: java.time.Duration is not currently used in the org.elasticsearch.core.TimeValue class.

Does this approach sound good to you?

@sothawo
Copy link
Collaborator

sothawo commented Oct 14, 2024

yes, we need this mapping to a java.time.Duration as we cannot pass Elasticsearch classes through our API. So this change must be added in the org.springframework.data.elasticsearch.client.elc.SearchDocumentResponseBuilder class

ELHARROUGUI added a commit to ELHARROUGUI/spring-data-elasticsearch that referenced this issue Oct 19, 2024
Added support for retreving the query execution time from took() method of ResponseBody class as java.time.Duration in SearchDocumentResponseBuilder class.

Closes spring-projects#2986
ELHARROUGUI added a commit to ELHARROUGUI/spring-data-elasticsearch that referenced this issue Oct 19, 2024
Added support for retreving the query execution time from took() method of ResponseBody class as java.time.Duration in SearchDocumentResponseBuilder class.

Closes spring-projects#2986
ELHARROUGUI added a commit to ELHARROUGUI/spring-data-elasticsearch that referenced this issue Oct 19, 2024
Added support for retreving the query execution time from took() method of ResponseBody class as java.time.Duration in SearchDocumentResponseBuilder class.

Closes spring-projects#2986
ELHARROUGUI added a commit to ELHARROUGUI/spring-data-elasticsearch that referenced this issue Oct 19, 2024
Added support for retreving the query execution time from took() method of ResponseBody class as java.time.Duration in SearchDocumentResponseBuilder class.

Closes spring-projects#2986
ELHARROUGUI added a commit to ELHARROUGUI/spring-data-elasticsearch that referenced this issue Oct 20, 2024
Added support for retreving the query execution time from took() method of ResponseBody class as java.time.Duration in SearchDocumentResponseBuilder class.

Closes spring-projects#2986
sothawo pushed a commit that referenced this issue Oct 22, 2024
@sothawo sothawo added this to the 5.4 GA (2024.1.0) milestone Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants