[APM] Dependencies table for backend inventory/detail views #106995

dgieselaar · 2021-07-28T12:13:59Z

Adds dependencies tables for backend inventory and detail views.

Still needs to happen, preferably in follow-up PRs:

take kuery into account (not sure if this is reasonable)
some ui tweaks
API tests

elasticmachine · 2021-07-28T12:14:01Z

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

dgieselaar · 2021-07-28T12:38:31Z

x-pack/plugins/apm/public/components/routing/apm_route_config.tsx

@@ -63,6 +54,15 @@ const apmRoutes = route([
      }),
    ]),
  },
+  {


this puts the main UI routes after the redirect routes, to ensure those are matched first (they're more specific)

cauemarcondes · 2021-07-28T16:39:06Z

x-pack/plugins/apm/common/connections.ts

+
+export type Node = ServiceNode | BackendNode;
+
+export interface ConnectionMetricItem {


Should we really call it Metric? I remember there were a lot of discussions when I implemented the comparison feature that "metric" is already over-utilized.

agree, will change to "stats"

cauemarcondes · 2021-07-28T16:40:47Z

x-pack/plugins/apm/common/connections.ts

+  metrics: {
+    latency: {
+      value: number | null;
+      timeseries: Array<{ x: number; y: number | null }>;


Can you use Coordinate?

cauemarcondes · 2021-07-28T16:59:23Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/index.ts

+      let to: Node = metricItem.to;
+
+      to = destinationMap.get(to.backendName) ?? to;


Suggested change

let to: Node = metricItem.to;

to = destinationMap.get(to.backendName) ?? to;

const to: Node = destinationMap.get(to.backendName) ?? metricItem.to

cauemarcondes · 2021-07-28T17:02:19Z

x-pack/plugins/apm/server/lib/backends/get_top_backends.ts

+    start,
+    end,
+    numBuckets,
+    filter: [...environmentQuery(environment)],


Suggested change

filter: [...environmentQuery(environment)],

filter: environmentQuery(environment),

Actually, I don't mind this - I think it's more obviously an array this way, which I think it's a good thing.

cauemarcondes · 2021-07-28T17:07:43Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/get_destination_map.ts

+  return withApmSpan('get_destination_map', async () => {
+    const { apmEventClient } = setup;
+
+    const offsetInMs = getOffsetInMs(start, offset);


Have you considered returning startWithOffset and endWithOffset in getOffsetInMs? I feel like everywhere we'll use it we'll have to subtract the offset from start and end.

Suggested change

const offsetInMs = getOffsetInMs(start, offset);

const {offsetInMs, startWithOffset, endWithOffset} = getOffsetInMs(start, offset);

👍 good idea

cauemarcondes · 2021-07-28T17:08:35Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/get_destination_map.ts

+              ...filter,
+              {
+                bool: {
+                  must_not: [{ term: { [AGENT_NAME]: 'rum-js' } }],


Maybe you could add a comment explaining why rum is being filtered out?

good one, I'll probably extract it to some other file as it's used in multiple places.

cauemarcondes · 2021-07-28T17:16:48Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/get_destination_map.ts

+      let node: Node;
+      if ('serviceName' in mergedDestination) {
+        node = {
+          serviceName: mergedDestination.serviceName,
+          agentName: mergedDestination.agentName,
+          environment: mergedDestination.environment,
+          id: objectHash({ serviceName: mergedDestination.serviceName }),
+          type: NodeType.service,
+        };
+      } else {
+        node = {
+          backendName: mergedDestination.backendName,
+          spanType: mergedDestination.spanType,
+          spanSubtype: mergedDestination.spanSubtype,
+          id: objectHash({ backendName: mergedDestination.backendName }),
+          type: NodeType.backend,
+        };
+      }


Can't you make node a const and use a ternary to set its value?

Suggested change

let node: Node;

if ('serviceName' in mergedDestination) {

node = {

serviceName: mergedDestination.serviceName,

agentName: mergedDestination.agentName,

environment: mergedDestination.environment,

id: objectHash({ serviceName: mergedDestination.serviceName }),

type: NodeType.service,

};

} else {

node = {

backendName: mergedDestination.backendName,

spanType: mergedDestination.spanType,

spanSubtype: mergedDestination.spanSubtype,

id: objectHash({ backendName: mergedDestination.backendName }),

type: NodeType.backend,

};

}

const node: Node = 'serviceName' in mergedDestination ? {

serviceName: mergedDestination.serviceName,

agentName: mergedDestination.agentName,

environment: mergedDestination.environment,

id: objectHash({ serviceName: mergedDestination.serviceName }),

type: NodeType.service,

} :

{

backendName: mergedDestination.backendName,

spanType: mergedDestination.spanType,

spanSubtype: mergedDestination.spanSubtype,

id: objectHash({ backendName: mergedDestination.backendName }),

type: NodeType.backend,

}

I think the if/else is a little more readable here, due to the size of the object being created.

cauemarcondes · 2021-07-28T17:17:48Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/get_metrics.ts

+  const offsetInMs = getOffsetInMs(start, offset);
+  const startWithOffset = start - offsetInMs;
+  const endWithOffset = end - offsetInMs;


This is what I meant.

cauemarcondes · 2021-07-28T17:18:20Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/get_metrics.ts

+              ...rangeQuery(startWithOffset, endWithOffset),
+              {
+                bool: {
+                  must_not: [{ term: { [AGENT_NAME]: 'rum-js' } }],


a comment here too?

Probably want to use RUM_AGENT_NAMES to include legacy and otel names.

cauemarcondes · 2021-07-28T17:19:05Z

x-pack/plugins/apm/server/lib/connections/get_connection_metrics/get_destination_map.ts

+          connections: {
+            composite: {
+              size: 10000,
+              sources: asMutableArray([


Why do you need to use asMutableArray here?

We have to use as const to create a tuple type, otherwise typescript will create a partial type of all element types, which is not recognised by the ES client's types, and the type of the response will be unknown. For the same reason we wrap it in asMutableArray, as as const will convert an Array type to a ReadonlyArray type.

cauemarcondes

just some nits

smith · 2021-07-28T22:30:45Z

...gins/apm/public/components/app/backend_detail_overview/backend_detail_dependencies_table.tsx

+export function BackendDetailDependenciesTable() {
+  const {
+    urlParams: { start, end, environment, comparisonEnabled, comparisonType },
+  } = useUrlParams();


I know we still do magic with useUrlParams and start/end, but can the rest fo them come out of the useApmParams below?

I think the others have slightly different types/defaults as well. I'm trying to err on the safe side here, and then hopefully we can refactor these separately.

Do we have an issue to refactor this?

We have #105914

smith · 2021-07-28T22:36:59Z

x-pack/plugins/apm/public/components/shared/dependencies_table/index.tsx

+            text={name}
+            content={
+              <EuiFlexGroup gutterSize="s" responsive={false}>
+                <EuiFlexItem grow={false}>{icon}</EuiFlexItem>


We might have to resolve conflicts between this and my changes in https://github.com/elastic/kibana/pull/107089/files#diff-732775f9fc7d8281a24dd8fef294675022aec9fab6f68546ebf75088304c83ffR123-R142

kibanamachine · 2021-07-30T13:09:10Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1567	1570	+3

Async chunks

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

id	before	after	diff
`apm`	4.2MB	4.3MB	+3.4KB

History

💔 Build #141882 failed a5fdb23
💔 Build #141864 failed 9fe5b0d
💚 Build #141309 succeeded 82b0f94
💔 Build #141296 failed 9790e3c
💔 Build #141306 failed bc71638

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

cauemarcondes · 2021-07-30T13:21:08Z

x-pack/plugins/apm/server/lib/connections/exclude_rum_exit_spans_query.ts

+import { AGENT_NAME } from '../../../common/elasticsearch_fieldnames';
+import { RUM_AGENT_NAMES } from '../../../common/agent_name';
+
+// exclude RUM exit spans, as they're high cardinality and don't usually


cauemarcondes

LGTM. Nice work 👏🏻

kibanamachine · 2021-07-30T13:26:19Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 106995

…106995)

kibanamachine · 2021-08-03T13:28:42Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…106995)

dgieselaar added 3 commits July 28, 2021 13:36

Refactor dependencies API so backends UI can use it

28e5bf9

Update snapshot tests

56ae35c

Add dependencies tables to backend inventory/detail views

9790e3c

dgieselaar added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. release_note:skip Skip the PR/issue when compiling release notes v7.15.0 labels Jul 28, 2021

dgieselaar requested a review from a team as a code owner July 28, 2021 12:13

dgieselaar commented Jul 28, 2021

View reviewed changes

dgieselaar added 2 commits July 28, 2021 14:38

Refactor link building

bc71638

Remove unused translations

82b0f94

cauemarcondes reviewed Jul 28, 2021

View reviewed changes

cauemarcondes requested changes Jul 28, 2021

View reviewed changes

smith reviewed Jul 28, 2021

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into backend-ui-apis

b4726ac

smith mentioned this pull request Jul 29, 2021

Add backend info and icons to flyouts #107089

Merged

dgieselaar added 5 commits July 30, 2021 09:45

Merge branch 'master' of github.com:elastic/kibana into backend-ui-apis

9fe5b0d

Use ServiceLink/BackendLink when appropriate

a5fdb23

Review feedback

7da928f

Remove icon

4ed6c48

Merge branch 'master' of github.com:elastic/kibana into backend-ui-apis

ce67fdd

dgieselaar requested a review from cauemarcondes July 30, 2021 10:54

cauemarcondes reviewed Jul 30, 2021

View reviewed changes

dgieselaar added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 30, 2021

dgieselaar enabled auto-merge (squash) July 30, 2021 13:21

cauemarcondes approved these changes Jul 30, 2021

View reviewed changes

dgieselaar merged commit efd2540 into elastic:master Jul 30, 2021

dgieselaar added a commit to dgieselaar/kibana that referenced this pull request Aug 3, 2021

[APM] Dependencies table for backend inventory/detail views (elastic#…

9a7e9f7

…106995)

dgieselaar mentioned this pull request Aug 3, 2021

[APM] Backport #106837, #106995, #107367 #107518

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 3, 2021

dgieselaar added a commit that referenced this pull request Aug 3, 2021

[APM] Backport #106837, #106995, #107367 (#107518)

343a3e1

dgieselaar added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Aug 3, 2021

dgieselaar deleted the backend-ui-apis branch August 3, 2021 14:50

streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021

[APM] Dependencies table for backend inventory/detail views (elastic#…

3340072

…106995)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Dependencies table for backend inventory/detail views #106995

[APM] Dependencies table for backend inventory/detail views #106995

dgieselaar commented Jul 28, 2021

elasticmachine commented Jul 28, 2021

dgieselaar Jul 28, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes Jul 28, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes Jul 28, 2021

cauemarcondes Jul 28, 2021

smith Jul 30, 2021

cauemarcondes Jul 28, 2021

dgieselaar Jul 30, 2021

cauemarcondes left a comment

smith Jul 28, 2021

dgieselaar Jul 30, 2021

sorenlouv Aug 2, 2021

dgieselaar Aug 3, 2021

smith Jul 28, 2021

kibanamachine commented Jul 30, 2021

cauemarcondes Jul 30, 2021

cauemarcondes left a comment

kibanamachine commented Jul 30, 2021

kibanamachine commented Aug 3, 2021

@@ @@ -63,6 +54,15 @@ const apmRoutes = route([ @@
                     }),
                   ]),
                 },
+                {


		export type Node = ServiceNode \| BackendNode;

		export interface ConnectionMetricItem {

		let to: Node = metricItem.to;

		to = destinationMap.get(to.backendName) ?? to;

	filter: [...environmentQuery(environment)],
	filter: environmentQuery(environment),

	const offsetInMs = getOffsetInMs(start, offset);
	const {offsetInMs, startWithOffset, endWithOffset} = getOffsetInMs(start, offset);

[APM] Dependencies table for backend inventory/detail views #106995

[APM] Dependencies table for backend inventory/detail views #106995

Conversation

dgieselaar commented Jul 28, 2021

elasticmachine commented Jul 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Jul 30, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 30, 2021

💔 Backport failed

kibanamachine commented Aug 3, 2021