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] Use terms enum to fetch environments on Service inventory page #144544

Closed
sorenlouv opened this issue Nov 3, 2022 · 8 comments · Fixed by #150175
Closed

[APM] Use terms enum to fetch environments on Service inventory page #144544

sorenlouv opened this issue Nov 3, 2022 · 8 comments · Fixed by #150175
Assignees
Labels
8.8 candidate apm:low-effort apm:performance APM UI - Performance Work Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture

Comments

@sorenlouv
Copy link
Member

We should use the terms_enum api to retrieve the environments on the Service inventory page. Currently we retrieve them via a term agg request:

const operationName = serviceName
? 'get_environments_for_service'
: 'get_environments';
const params = {
apm: {
events: [
getProcessorEventForTransactions(searchAggregatedTransactions),
ProcessorEvent.metric,
ProcessorEvent.error,
],
},
body: {
track_total_hits: false,
size: 0,
query: {
bool: {
filter: [
...rangeQuery(start, end),
...termQuery(SERVICE_NAME, serviceName),
],
},
},
aggs: {
environments: {
terms: {
field: SERVICE_ENVIRONMENT,
missing: ENVIRONMENT_NOT_DEFINED.value,
size,
},
},
},
},
};

@sorenlouv sorenlouv added Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture labels Nov 3, 2022
@elasticmachine
Copy link
Contributor

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

@dgieselaar
Copy link
Member

@sqren shouldn't this be about removing the unnecessary call to get_environments?

@sorenlouv
Copy link
Member Author

@sqren shouldn't this be about removing the unnecessary call to get_environments?

If that's the problem, yes. I haven't determined if that's the case though.

@achyutjhunjhunwala
Copy link
Contributor

@dgieselaar @sqren I have picked up this ticket to work on. Looking at the comments here, is the expectation different from what's in the description of the ticket ? Can you help me here detailing a bit more?

@sorenlouv
Copy link
Member Author

sorenlouv commented Jan 31, 2023

@achyutjhunjhunwala The intention is to make sure that on the service inventory page we load environments using the terms enum api and only that. Dario suggests that maybe we are loading environments twice. It could be the case, in which case we should remove the duplicate call. If we only make one call and it doesn't use terms enum we should update the API.

@achyutjhunjhunwala
Copy link
Contributor

@sqren Perfect, that's all the information i needed

@achyutjhunjhunwala
Copy link
Contributor

achyutjhunjhunwala commented Feb 1, 2023

@sqren I had a check at the network tab to see if we are calling the '/internal/apm/environments' twice, but i don't see that.

Regarding switching to term_enum api, i am not getting the exact result as we would get from the existing API. The term_enum api does not let to filter the result as we expect.

Let me know if you have something in mind around a custom filter which we can implement on the code side.

With current API

Screenshot 2023-02-01 at 17 30 06

With Terms_Enum API

image

Even with index_filters we are getting different results

image

@sorenlouv
Copy link
Member Author

Regarding switching to term_enum api, i am not getting the exact result as we would get from the existing API. The term_enum api does not let to filter the result as we expect.

Thanks for the thorough investigation @achyutjhunjhunwala ! We are not aiming for a 1:1 replacement so I think the terms enum query with index_filter is good enough.

I think in your case the result is a little exaggerated because of the nature of our data. I don't expect real users to see as big variances in service.environment terms over short periods of time.

achyutjhunjhunwala added a commit that referenced this issue Feb 8, 2023
Fixes #144544

## Summary

To get the list of Environments, aggregation was used on search api.
With this change, list of environments will now be retrieved using the
[Terms Enum
API](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-terms-enum.html#search-terms-enum)

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.8 candidate apm:low-effort apm:performance APM UI - Performance Work Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants