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

[APM] Add range query to terms enum call #162614

Conversation

achyutjhunjhunwala
Copy link
Contributor

Closes #159202

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@achyutjhunjhunwala achyutjhunjhunwala marked this pull request as ready for review July 27, 2023 10:05
@achyutjhunjhunwala achyutjhunjhunwala requested a review from a team as a code owner July 27, 2023 10:05
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Jul 27, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

Copy link
Contributor

@kpatticha kpatticha left a comment

Choose a reason for hiding this comment

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

👍 LGTM, there is already an api test. Could you please update it to cover the time ranges

https://github.com/elastic/kibana/blob/main/x-pack/test/apm_api_integration/tests/storage_explorer/get_services.spec.ts#L24C18-L24C29

@achyutjhunjhunwala
Copy link
Contributor Author

achyutjhunjhunwala commented Jul 27, 2023

👍 LGTM, there is already an api test. Could you please update it to cover the time ranges

https://github.com/elastic/kibana/blob/main/x-pack/test/apm_api_integration/tests/storage_explorer/get_services.spec.ts#L24C18-L24C29

I already did update it to have time range ? Did you meant something else ?

Do you mean to add more expectation based on time range ? If yes, i did not add it intentionally as the index filter addition should not change the expectation, but improve performance. Let me know if you have any other idea

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.7MB 3.7MB +18.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @achyutjhunjhunwala

@kpatticha
Copy link
Contributor

kpatticha commented Jul 27, 2023

👍 LGTM, there is already an api test. Could you please update it to cover the time ranges

https://github.com/elastic/kibana/blob/main/x-pack/test/apm_api_integration/tests/storage_explorer/get_services.spec.ts#L24C18-L24C29

I already did update it to have time range ? Did you meant something else ?

Do you mean to add more expectation based on time range ? If yes, i did not add it intentionally as the index filter addition should not change the expectation, but improve performance. Let me know if you have any other idea

Oh my bad, I totally missed your last commit. All good, thanks for updating the tests as well

@achyutjhunjhunwala achyutjhunjhunwala merged commit 28800ef into elastic:main Jul 27, 2023
@achyutjhunjhunwala achyutjhunjhunwala deleted the add-range-query-to-terms-enum-call branch July 27, 2023 11:49
@kibanamachine kibanamachine added v8.10.0 backport:skip This commit does not require backporting labels Jul 27, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:fix Team:APM All issues that need APM UI Team support v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Add range query to terms enum calls
6 participants