Skip to content

Remove environment from uiFilters#89647

Merged
smith merged 34 commits intoelastic:masterfrom
smith:nls/env-filters
Feb 17, 2021
Merged

Remove environment from uiFilters#89647
smith merged 34 commits intoelastic:masterfrom
smith:nls/env-filters

Conversation

@smith
Copy link
Copy Markdown
Contributor

@smith smith commented Jan 28, 2021 •
edited
Loading

  • Explicitly use environment from query string instead of uiFilters
  • Remove environment from setup.esFilter
  • Replace getEnvironmentUiFilterES with environmentQuery
  • Replace rangeFilter with rangeQuery

@smith
Copy link
Copy Markdown
Contributor Author

smith commented Feb 2, 2021

@elasticmachine merge upstream

@smith
Copy link
Copy Markdown
Contributor Author

smith commented Feb 2, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 2, 2021

merge conflict between base and head

@spalger
Copy link
Copy Markdown
Contributor

spalger commented Feb 4, 2021

jenkins test this

@smith smith mentioned this pull request Feb 5, 2021
Closed
8 tasks
smith
smith commented Feb 5, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/queries.test.ts.snap
"bool": Object {
"filter": Array [
Object {
"range": Object {
Copy link
Copy Markdown
Contributor Author

@smith smith Feb 5, 2021

Choose a reason for hiding this comment

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

The changes here are only a reflection in the changes of ordering the filters for readability.

smith
smith commented Feb 5, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/server/routes/create_apm_api.ts
.add(transactionGroupsOverviewRoute)
.add(transactionLatencyChatsRoute)
.add(transactionThroughputChatsRoute)
.add(transactionLatencyChartsRoute)
Copy link
Copy Markdown
Contributor Author

@smith smith Feb 5, 2021

Choose a reason for hiding this comment

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

Fixed misspelling here.

shahzad31
shahzad31 reviewed Feb 11, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/server/lib/rum_client/has_rum_data.ts
filter: {
range: rangeFilter(start, end),
},
filter: rangeQuery(start, end)[0],
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 Feb 11, 2021

Choose a reason for hiding this comment

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

rangeQuery returns an array or object?

Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 Feb 11, 2021

Choose a reason for hiding this comment

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

i think it returns array, i guess it can be assigned directly here.

Copy link
Copy Markdown
Contributor Author

@smith smith Feb 17, 2021

Choose a reason for hiding this comment

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

These query functions return arrays because they are normally spread. This is because filter: [{}] is an error in Elasticsearch queries but filter: [...[]] is not.

shahzad31
shahzad31 approved these changes Feb 11, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

UX App changes LGTM!!

sorenlouv
sorenlouv reviewed Feb 17, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/common/utils/queries.test.ts
});

it('creates a query for missing service environments', () => {
expect(environmentQuery(ENVIRONMENT_NOT_DEFINED.value)[0]).toHaveProperty(
Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv Feb 17, 2021

Choose a reason for hiding this comment

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

Definitely a better name 👍

sorenlouv
sorenlouv reviewed Feb 17, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/public/components/app/service_inventory/index.tsx
sorenlouv
sorenlouv reviewed Feb 17, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/common/utils/queries.ts
sorenlouv
sorenlouv reviewed Feb 17, 2021
View reviewed changes
Comment thread x-pack/plugins/apm/server/lib/helpers/aggregated_transactions/index.ts Outdated
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 17, 2021

💚 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 5.2MB 5.2MB +1.0KB

History

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

@smith smith merged commit f650c38 into elastic:master Feb 17, 2021
@smith smith deleted the nls/env-filters branch February 17, 2021 20:28
@kibanamachine kibanamachine mentioned this pull request Feb 17, 2021
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 17, 2021
@smith @kibanamachine
Remove environment from uiFilters (elastic#89647)
40d01a6
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 17, 2021

💚 Backport successful

✅ 7.x / #91745

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Feb 17, 2021
@kibanamachine @smith
Remove environment from uiFilters (#89647) (#91745)
828aa1b 
Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
This was referenced Feb 19, 2021
Merged
Closed
Merged
smith added a commit to smith/kibana that referenced this pull request Mar 3, 2021
@smith
Fix service map for All environment single service
fef1f52 
Before we removed environment from the UI filters (elastic#89647), the environment query parameter would be undefined if "All" was selected. Now we send ENVIRONMENT_ALL in as the query parameter.

Changes in https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts made it so no connections would be returned if ENVIRONMENT_ALL was selected, rather than all connections. Since no connections were being returned, no elements except the selected service would be returned in the API response.

This changes it so if ENVIRONMENT_ALL is selected, the connection will always be returned, just like what used to be the case when environment was undefined.

Add an API test for this case.

Fixes elastic#93385.
@smith smith mentioned this pull request Mar 3, 2021
Merged
smith added a commit that referenced this pull request Mar 3, 2021
@smith
Fix service map for All environment single service (#93517)
4e21fae 
Before we removed environment from the UI filters (#89647), the environment query parameter would be undefined if "All" was selected. Now we send ENVIRONMENT_ALL in as the query parameter.

Changes in https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts made it so no connections would be returned if ENVIRONMENT_ALL was selected, rather than all connections. Since no connections were being returned, no elements except the selected service would be returned in the API response.

This changes it so if ENVIRONMENT_ALL is selected, the connection will always be returned, just like what used to be the case when environment was undefined.

Add an API test for this case.

Fixes #93385.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 3, 2021
@smith @kibanamachine
Fix service map for All environment single service (elastic#93517)
c43c0a2 
Before we removed environment from the UI filters (elastic#89647), the environment query parameter would be undefined if "All" was selected. Now we send ENVIRONMENT_ALL in as the query parameter.

Changes in https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts made it so no connections would be returned if ENVIRONMENT_ALL was selected, rather than all connections. Since no connections were being returned, no elements except the selected service would be returned in the API response.

This changes it so if ENVIRONMENT_ALL is selected, the connection will always be returned, just like what used to be the case when environment was undefined.

Add an API test for this case.

Fixes elastic#93385.
smith added a commit to smith/kibana that referenced this pull request Mar 3, 2021
@smith
Fix service map for All environment single service (elastic#93517)
775fa55 
Before we removed environment from the UI filters (elastic#89647), the environment query parameter would be undefined if "All" was selected. Now we send ENVIRONMENT_ALL in as the query parameter.

Changes in https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts made it so no connections would be returned if ENVIRONMENT_ALL was selected, rather than all connections. Since no connections were being returned, no elements except the selected service would be returned in the API response.

This changes it so if ENVIRONMENT_ALL is selected, the connection will always be returned, just like what used to be the case when environment was undefined.

Add an API test for this case.

Fixes elastic#93385.
kibanamachine added a commit that referenced this pull request Mar 4, 2021
@kibanamachine @smith
Fix service map for All environment single service (#93517) (#93534)
da2a7fc 
Before we removed environment from the UI filters (#89647), the environment query parameter would be undefined if "All" was selected. Now we send ENVIRONMENT_ALL in as the query parameter.

Changes in https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts made it so no connections would be returned if ENVIRONMENT_ALL was selected, rather than all connections. Since no connections were being returned, no elements except the selected service would be returned in the API response.

This changes it so if ENVIRONMENT_ALL is selected, the connection will always be returned, just like what used to be the case when environment was undefined.

Add an API test for this case.

Fixes #93385.

Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
smith added a commit that referenced this pull request Mar 4, 2021
@smith
Fix service map for All environment single service (#93517) (#93535)
73948a3 
Before we removed environment from the UI filters (#89647), the environment query parameter would be undefined if "All" was selected. Now we send ENVIRONMENT_ALL in as the query parameter.

Changes in https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts made it so no connections would be returned if ENVIRONMENT_ALL was selected, rather than all connections. Since no connections were being returned, no elements except the selected service would be returned in the API response.

This changes it so if ENVIRONMENT_ALL is selected, the connection will always be returned, just like what used to be the case when environment was undefined.

Add an API test for this case.

Fixes #93385.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahzad31 shahzad31 shahzad31 approved these changes

+1 more reviewer

@sorenlouv sorenlouv sorenlouv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.12.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@smith @kibanamachine @spalger @elasticmachine @sorenlouv @shahzad31