[APM] Service Map: `Not Defined` option doesn't work properly #77757

cauemarcondes · 2020-09-17T12:46:35Z

related to #77483

before:
opbeans-node/ruby/rum/dotnet don't have environment not defined, that's why the popover shows a message saying to change the selected env.

after:

elasticmachine · 2020-09-17T12:46:40Z

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

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts

cauemarcondes · 2020-09-17T16:12:51Z

retest

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts

sorenlouv

I'm not seeing any updates to a test for getConnections. Do we not have any? Would be really good with a unit test (api test not necessary) to verify this behaviour.

cauemarcondes · 2020-09-18T13:05:12Z

I'm not seeing any updates to a test for getConnections. Do we not have any? Would be really good with a unit test (api test not necessary) to verify this behaviour.

I've created an API test on my previous PR, it still works as expected even after the change, which is good =D... I can add a unit test if you want to!

cauemarcondes · 2020-09-21T07:58:12Z

@elasticmachine merge upstream

sorenlouv · 2020-09-21T09:35:29Z

I can add a unit test if you want to!

Yes, I think a unit test that verifies the different cases would be good.
The fact that this PR doesn't have any test changes indicates that we don't have adequate coverage.

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts

cauemarcondes · 2020-09-23T11:19:39Z

@elasticmachine merge upstream

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts

sorenlouv

lgtm 👍

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts

dgieselaar

As far as I can tell, we're not testing mixed environments which seems important. Same goes for connections to external destinations. Maybe I missed part of that conversation though.

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts

cauemarcondes · 2020-09-29T09:51:25Z

we're not testing mixed environments which seems important

What do you mean? The paths variable has two environments set production and testing

dgieselaar · 2020-09-29T09:53:53Z

@cauemarcondes yes, but in different paths

cauemarcondes · 2020-09-29T11:55:57Z

x-pack/plugins/apm/server/lib/service_map/get_service_map.ts

+  let { filter } = projection.body.query.bool;
+
+  if (options.serviceName) {
+    filter = filter.concat({
+      term: {
+        [SERVICE_NAME]: options.serviceName,
+      },
+    });
+  }
+
+  if (options.environment) {
+    filter = filter.concat(getEnvironmentUiFilterES(options.environment));
+  }


@elastic/apm-ui this fixes the issue described here

sorenlouv

Test are much easier to read now. Good work 👍

dgieselaar

nice work @cauemarcondes 👍

kibanamachine · 2020-09-30T10:32:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c261094

Metrics [docs]

distributable file count

id	value	diff	baseline
`default`	45636	-1	45637

History

💔 Build #77839 failed 54bb62a
💔 Build #77819 failed db3ab30
💚 Build #77757 succeeded e7d9da2
💚 Build #77357 succeeded 3c93f7f
💚 Build #77335 succeeded dc2e588

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

…c#77757) * Service Map: option doesn't work properly * addressing PR comment * addressing PR comment * adding unit tests * addressing PR comment * fixing tests * addressing PR comment * addressing PR comment * addressing PR comment * addressing PR comment * addressing PR comment * add filter by environment * addressing PR comment * fixing API tests Co-authored-by: Elastic Machine <[email protected]>

…aly-detection-partition-field * 'master' of github.com:elastic/kibana: (37 commits) [UiActions] Don't throw an error if there are no compatible actions to execute (elastic#78917) [Observability] Kibana nav when docked overlaps the content of the pages. (elastic#78593) Invalid `searchSourceJSON` causes saved object migration to fail (elastic#78535) update vega version (elastic#78390) Fix warning text doesn't get displayed on filters with custom filter name (elastic#78617) [ILM] Data tier notices should reflect tier preferences (elastic#78398) [APM] Service Map: `Not Defined` option doesn't work properly (elastic#77757) Improve invalid field error message at search.aggs.param_types.field (elastic#78587) Remove isDeprecated flag on visType (elastic#78820) Remove unused elasticsearch.preserverHost setting (elastic#78608) Fix condition and adjust tests (elastic#78898) [UX] Add percentile selector (elastic#78562) [ML] Replace use of rest_total_hits_as_int with track_total_hits (elastic#78423) expression service docs (elastic#78774) [Functional] Wait for the page to load and then click the new vis button (elastic#78725) [TSVB] No data in visualizations with annotations (elastic#78794) [kbn/ui-shared-deps] track asset sizes (elastic#78718) delete target before building (elastic#78665) Move indexPattern.popularizeField into discover (elastic#77668) [Security Solution][Resolver]Add backdrop to pills (elastic#78625) ...

…c#77757) * Service Map: option doesn't work properly * addressing PR comment * addressing PR comment * adding unit tests * addressing PR comment * fixing tests * addressing PR comment * addressing PR comment * addressing PR comment * addressing PR comment * addressing PR comment * add filter by environment * addressing PR comment * fixing API tests Co-authored-by: Elastic Machine <[email protected]>

…77757) | fixing api test (#78964) (#79063) * [APM] Service Map: `Not Defined` option doesn't work properly (#77757) * Service Map: option doesn't work properly * addressing PR comment * addressing PR comment * adding unit tests * addressing PR comment * fixing tests * addressing PR comment * addressing PR comment * addressing PR comment * addressing PR comment * addressing PR comment * add filter by environment * addressing PR comment * fixing API tests Co-authored-by: Elastic Machine <[email protected]> * fixing api test (#78964) Co-authored-by: Elastic Machine <[email protected]>

Service Map: option doesn't work properly

6b50584

cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 17, 2020

cauemarcondes requested a review from a team as a code owner September 17, 2020 12:46

botelastic bot added the Team:APM All issues that need APM UI Team support label Sep 17, 2020

cauemarcondes commented Sep 17, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts Outdated Show resolved Hide resolved

addressing PR comment

b71b2d6

cauemarcondes requested a review from dgieselaar September 17, 2020 14:28

sorenlouv reviewed Sep 17, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts Outdated Show resolved Hide resolved

sorenlouv approved these changes Sep 17, 2020

View reviewed changes

addressing PR comment

12dc643

sorenlouv requested changes Sep 18, 2020

View reviewed changes

Merge branch 'master' into apm-servicemaps-env-not-defined

00ef666

adding unit tests

26ae62b

cauemarcondes requested a review from sorenlouv September 21, 2020 11:52

sorenlouv reviewed Sep 21, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts Outdated Show resolved Hide resolved

addressing PR comment

a5d026e

cauemarcondes requested a review from sorenlouv September 23, 2020 07:24

elasticmachine and others added 2 commits September 23, 2020 07:19

Merge branch 'master' into apm-servicemaps-env-not-defined

d025d3f

fixing tests

cf0e543

sorenlouv reviewed Sep 23, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts Outdated Show resolved Hide resolved

sorenlouv reviewed Sep 23, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts Outdated Show resolved Hide resolved

sorenlouv reviewed Sep 23, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts Outdated Show resolved Hide resolved

sorenlouv reviewed Sep 23, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts Outdated Show resolved Hide resolved

cauemarcondes requested a review from sorenlouv September 28, 2020 12:19

sorenlouv approved these changes Sep 28, 2020

View reviewed changes

dgieselaar reviewed Sep 28, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts Outdated Show resolved Hide resolved

dgieselaar reviewed Sep 28, 2020

View reviewed changes

x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts Outdated Show resolved Hide resolved

addressing PR comment

e7d9da2

dgieselaar reviewed Sep 29, 2020

View reviewed changes

cauemarcondes added 2 commits September 29, 2020 13:50

addressing PR comment

76e0c76

add filter by environment

db3ab30

cauemarcondes commented Sep 29, 2020

View reviewed changes

sorenlouv approved these changes Sep 29, 2020

View reviewed changes

addressing PR comment

54bb62a

cauemarcondes requested a review from dgieselaar September 29, 2020 13:00

dgieselaar approved these changes Sep 29, 2020

View reviewed changes

fixing API tests

c261094

cauemarcondes merged commit 7b90333 into elastic:master Sep 30, 2020

cauemarcondes deleted the apm-servicemaps-env-not-defined branch September 30, 2020 10:36

cauemarcondes mentioned this pull request Sep 30, 2020

[7.x] [APM] Service Map: Not Defined option doesn't work properly (#77757) #78927

Closed

cauemarcondes mentioned this pull request Oct 1, 2020

[7.x] [APM] Service Map: Not Defined option doesn't work properly (#77757) | fixing api test (#78964) #79063

Merged

sorenlouv added the apm:test-plan-7.10.0 label Oct 7, 2020

ogupte self-assigned this Oct 20, 2020

ogupte added the apm:test-plan-done Pull request that was successfully tested during the test plan label Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Service Map: `Not Defined` option doesn't work properly #77757

[APM] Service Map: `Not Defined` option doesn't work properly #77757

cauemarcondes commented Sep 17, 2020 •

edited

Loading

elasticmachine commented Sep 17, 2020

cauemarcondes commented Sep 17, 2020

sorenlouv left a comment

cauemarcondes commented Sep 18, 2020

cauemarcondes commented Sep 21, 2020

sorenlouv commented Sep 21, 2020 •

edited

Loading

cauemarcondes commented Sep 23, 2020

sorenlouv left a comment

dgieselaar left a comment

cauemarcondes commented Sep 29, 2020

dgieselaar commented Sep 29, 2020

cauemarcondes Sep 29, 2020

sorenlouv left a comment

dgieselaar left a comment

kibanamachine commented Sep 30, 2020

[APM] Service Map: Not Defined option doesn't work properly #77757

[APM] Service Map: Not Defined option doesn't work properly #77757

Conversation

cauemarcondes commented Sep 17, 2020 • edited Loading

elasticmachine commented Sep 17, 2020

cauemarcondes commented Sep 17, 2020

sorenlouv left a comment

Choose a reason for hiding this comment

cauemarcondes commented Sep 18, 2020

cauemarcondes commented Sep 21, 2020

sorenlouv commented Sep 21, 2020 • edited Loading

cauemarcondes commented Sep 23, 2020

sorenlouv left a comment

Choose a reason for hiding this comment

dgieselaar left a comment

Choose a reason for hiding this comment

cauemarcondes commented Sep 29, 2020

dgieselaar commented Sep 29, 2020

cauemarcondes Sep 29, 2020

Choose a reason for hiding this comment

sorenlouv left a comment

Choose a reason for hiding this comment

dgieselaar left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 30, 2020

💚 Build Succeeded

Metrics [docs]

distributable file count

History

[APM] Service Map: `Not Defined` option doesn't work properly #77757

[APM] Service Map: `Not Defined` option doesn't work properly #77757

cauemarcondes commented Sep 17, 2020 •

edited

Loading

sorenlouv commented Sep 21, 2020 •

edited

Loading