APM execution context - app, page, entitiy id by lizozom · Pull Request #124996 · elastic/kibana

lizozom · 2022-02-08T17:34:25Z

Summary

This PR introduces a client side ExecutionContextService and uses it to provide application level context and passes it to APM transactions both on the client and the server side.

The information will be available for all APM RUM transactions as well as all server side APM request transactions.

All APM transactions are now automatically provided with:

name - as provided by the application service

And Dashboard, Discover, Lens, Visualizations Editor and Dev Tools also provide:

page - a sub-division that can be used to identify an application's logical unit. For example for dashboards that would be app which is a single dashboard and list for the dashboards list.
id - a saved object id that can be used to identify a key entity. For example the dashboard ID for a dashboard or a saved search id for Discover.

How to test this PR?

Enable APM by running export ELASTIC_APM_ACTIVE=true
Run Kibana locally
Turn off any ad blockers (they block FE events)
Open this view and load a saved search named [Liza] localhost transactions.
Reenable and modify the kibana_uuid in the filter to your local kibana_uuid. (Some people have their uuid statically set by ftr tests to 5b2de169-2785-441b-ae8c-186a1936b17d - if that's your id, delete your .uuid file.
Navigate through your local kibana and verify that any frontend (kibana-frontend) and backend (kibana) events have all the required fields.

Home

Discover

Saved

Unsaved

Visualizations list

Lens visualization

Unsaved

Saved

Other visualizations

TSVB

Vega

Data Table

Timelion

Dashboard list

Single dashboard

Saved

Unsaved

Maps list

Single map

Unsaved

Saved

Here's a list of issues I'm already aware of:

Bundles are loaded without any context
app-change transactions get the wrong name

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

move more things to top level context

…side-execution-context

lizozom · 2022-02-08T18:14:53Z

@mshustov I wasn't sure what needs to be done on this line (in the context of swapping parent and child on the context)
https://github.com/elastic/kibana/blob/main/src/core/server/execution_context/execution_context_service.ts#L139

… client-side-execution-context

mshustov · 2022-02-09T16:06:13Z

@mshustov I wasn't sure what needs to be done on this line (in the context of swapping parent and child on the context)

Hm. You need to copy the whole context "linked list", traverse to the last node and attach a new child node.

Could you remind me why we need to change the parent-child relationships?

mshustov · 2022-02-09T15:37:25Z

src/core/public/execution_context/execution_context_service.ts

+        const newVal = {
+          url: window.location.pathname,
+          name: this.appId,
+          ...this.context$.value,


The Server-side doesn't merge the contexts but gets the final version. It allows customers to implement any custom logic.

src/core/public/execution_context/execution_context_service.ts

lukasolson

Code changes LGTM

kibanamachine · 2022-03-03T09:01:14Z

💔 All backports failed

Status	Branch	Result
❌	8.1	Backport failed because of merge conflicts
❌	8.0	Backport failed because of merge conflicts

How to fix

Re-run the backport manually:

node scripts/backport --pr 124996

Questions ?

Please refer to the Backport tool documentation

@mikhail

* Client side execution app level context propagation * context$ + apm rum integration * invert the context parent \ child relationship (cc @mikhail) move more things to top level context * Pass down context to apm on server * types * eslint * parent <> child * docs + eslint + jest * execution context mock * eslint * jest * jest * server jest * check * jest * storybook * jest * report the current space * fix server side context container * Remove spaces for now * docssss * jest * lint * test * docs * revert file * doc * all context params are optional * clear on page change * lint * ts * skipped test again * testing fixes * oops * code review #1 * code review #2 * getAsLabels * maps inherit dashboard context * docs * ts * Give common context to all vis editors * fix test * ts \ es \ tests * labels * missing types * docsy docs * cr #3 * improve jest * Use editor name * Update src/plugins/visualizations/public/visualize_app/components/visualize_editor.tsx Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> * fix maps context * jest tests for maps * cr * docs * Update execution_context.test.ts * docs * lint Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> (cherry picked from commit d5416ed) # Conflicts: # src/plugins/discover/public/application/context/context_app.tsx # x-pack/plugins/lens/public/app_plugin/app.tsx

@mikhail

* Client side execution app level context propagation * context$ + apm rum integration * invert the context parent \ child relationship (cc @mikhail) move more things to top level context * Pass down context to apm on server * types * eslint * parent <> child * docs + eslint + jest * execution context mock * eslint * jest * jest * server jest * check * jest * storybook * jest * report the current space * fix server side context container * Remove spaces for now * docssss * jest * lint * test * docs * revert file * doc * all context params are optional * clear on page change * lint * ts * skipped test again * testing fixes * oops * code review #1 * code review #2 * getAsLabels * maps inherit dashboard context * docs * ts * Give common context to all vis editors * fix test * ts \ es \ tests * labels * missing types * docsy docs * cr #3 * improve jest * Use editor name * Update src/plugins/visualizations/public/visualize_app/components/visualize_editor.tsx Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> * fix maps context * jest tests for maps * cr * docs * Update execution_context.test.ts * docs * lint Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> (cherry picked from commit d5416ed) # Conflicts: # src/plugins/discover/public/application/context/context_app.tsx # x-pack/plugins/lens/public/app_plugin/app.tsx

lizozom · 2022-03-08T13:21:29Z

We can now use the new fields in discover, apm or any other app to filter data

…ecution context (#153616) #153218 Existing execution context implementation spread results from `executionContextServiceStart().get()` to add map `id`. This implementation did not work when map was embedded in another application. When embedded, `executionContextServiceStart().get()` returned the `id` of the parent application and not the `id` for the map. This PR resolves the issue by building executionContext directly in MapEmbeddable and MapApp. MapApp uses `savedObjectId` for `executionContext.id`. MapEmbeddable uses `embeddableId` for `executionContext.id`. The PR also updates the MVT routes to include executionContextId for the execution context with `_mvt` calls. #### Testing To view execution context in kibana logs, add below to kibana.dev.yml. For more information https://www.elastic.co/guide/en/kibana/8.7/kibana-troubleshooting-trace-query.html ``` logging: loggers: - name: execution_context level: debug appenders: [console] ``` #### Execution context for lens and maps panels in dashboard There is not a lot of consistency for what `type` and `name` should be across Kibana. I found [this comment](https://github.com/elastic/kibana/pull/105206/files#r671174649) for when lens added execution context for lens embeddable that stated `type:feature` and `name:sub_feature`. Therefore, I followed the lens example and made `type:maps` and `name:maps` since maps does not have sub_features. ``` [2023-03-23T18:29:12.750-06:00][DEBUG][execution_context] {"type":"application","name":"dashboards","url":"/mth/app/dashboards","page":"app","id":"d98e6530-c9a8-11ed-9340-d743c2c88db8","child":{"type":"maps","name":"maps","id":"91f17723-367d-460e-ad90-dbac1fc072cb","url":"/map/de71f4f0-1902-11e9-919b-ffe5949a18d2","description":"es_geo_grid_source:cluster"}} [2023-03-23T18:29:12.750-06:00][DEBUG][execution_context] {"type":"application","name":"dashboards","url":"/mth/app/dashboards","page":"app","id":"d98e6530-c9a8-11ed-9340-d743c2c88db8","child":{"type":"maps","name":"maps","id":"91f17723-367d-460e-ad90-dbac1fc072cb","url":"/map/de71f4f0-1902-11e9-919b-ffe5949a18d2","description":"es_term_source:terms"}} [2023-03-23T18:29:13.241-06:00][DEBUG][execution_context] {"type":"dashboard","name":"dashboards","url":"/mth/app/dashboards","page":"app","id":"d98e6530-c9a8-11ed-9340-d743c2c88db8","description":"single map","child":{"type":"lens","name":"lnsXY","id":"b9e44118-9e67-4c1c-9159-597d8a9f80d1","description":"lens","url":"/mth/app/lens#/edit/32e87880-c9ab-11ed-9340-d743c2c88db8"}} ``` #### Execution context for maps There is not a lot of consistency across kibana for `type` and `name` in applications. [ExecutionContextService.getDefaultContext](https://github.com/elastic/kibana/blob/main/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts#L99) returns the below so I stuck with `application` for type in client and `server` for type in server. Here is [one more link](#124996) that provides some context value of `name` property. ``` return { type: 'application', name: this.appId, url: window.location.pathname, }; ``` ``` [2023-03-23T18:30:39.886-06:00][DEBUG][execution_context] {"type":"application","name":"maps","url":"/mth/app/maps/map/de71f4f0-1902-11e9-919b-ffe5949a18d2","id":"de71f4f0-1902-11e9-919b-ffe5949a18d2","page":"editor","description":"es_geo_grid_source:cluster"} [2023-03-23T18:30:39.886-06:00][DEBUG][execution_context] {"type":"application","name":"maps","url":"/mth/app/maps/map/de71f4f0-1902-11e9-919b-ffe5949a18d2","id":"de71f4f0-1902-11e9-919b-ffe5949a18d2","page":"editor","description":"es_term_source:terms"} ``` --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

lizozom added 6 commits February 8, 2022 16:11

Client side execution app level context propagation

7deba6b

context$ + apm rum integration

c44c4f2

invert the context parent \ child relationship (cc @mikhail)

6b18395

move more things to top level context

Pass down context to apm on server

a0044cd

Merge branch 'main' of https://github.com/elastic/kibana into client-…

1115e93

…side-execution-context

types

ac41e1f

lizozom self-assigned this Feb 8, 2022

lizozom added v8.0.1 v8.1.0 v8.2.0 labels Feb 8, 2022

lizozom added 3 commits February 8, 2022 20:01

eslint

c2ed495

Merge branch 'main' of https://github.com/elastic/kibana into client-…

600607c

…side-execution-context

parent <> child

85408b1

lizozom linked an issue Feb 8, 2022 that may be closed by this pull request

Collect Saved Object ID in APM data #112608

Closed

lizozom mentioned this pull request Feb 8, 2022

Open a new APM transaction for every meaningful page instead of every currentAppId$ #114843

Closed

lizozom added 5 commits February 9, 2022 10:57

docs + eslint + jest

efb7952

execution context mock

5da627a

eslint

1c1fce1

git pushMerge branch 'main' of https://github.com/elastic/kibana into…

ed018ba

… client-side-execution-context

jest

5200052

lizozom requested a review from mshustov February 9, 2022 13:34

lizozom added 4 commits February 9, 2022 15:48

jest

4c151dd

server jest

b64e5e2

check

f1fc110

jest

f413c38

mshustov reviewed Feb 9, 2022

View reviewed changes

lizozom added 2 commits February 9, 2022 18:11

storybook

97350de

jest

cf33228

lukasolson approved these changes Mar 2, 2022

View reviewed changes

lizozom merged commit d5416ed into elastic:main Mar 3, 2022

lizozom added v8.0.2 and removed v8.0.1 labels Mar 3, 2022

lizozom mentioned this pull request Mar 3, 2022

[8.1] APM execution context - app, page, entitiy id (#124996) #126751

Merged

lizozom removed the v8.0.2 label Mar 3, 2022

alisonelizabeth mentioned this pull request Mar 23, 2022

Add execution context support for management plugins #128415

Merged

9 tasks

lukasolson mentioned this pull request Apr 19, 2022

[data.search] Move integration with execution context to data plugin search service #118230

Closed

xcrzx mentioned this pull request May 11, 2022

[Security Solution] Propagate execution context #131805

Merged

nreese mentioned this pull request Mar 24, 2023

[maps] fix Maps embeddables in a dashboard not capturing map id in execution context #153616

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

APM execution context - app, page, entitiy id#124996

APM execution context - app, page, entitiy id#124996
lizozom merged 79 commits intoelastic:mainfrom
lizozom:client-side-execution-context

lizozom commented Feb 8, 2022 •

edited

Loading

Uh oh!

lizozom commented Feb 8, 2022 •

edited

Loading

Uh oh!

mshustov commented Feb 9, 2022

Uh oh!

mshustov Feb 9, 2022

Uh oh!

Uh oh!

Uh oh!

lukasolson left a comment

Uh oh!

kibanamachine commented Mar 3, 2022

Uh oh!

lizozom commented Mar 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

Conversation

lizozom commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test this PR?

Home

Discover

Visualizations list

Lens visualization

Other visualizations

Dashboard list

Single dashboard

Maps list

Single map

Checklist

Risk Matrix

For maintainers

Uh oh!

lizozom commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mshustov commented Feb 9, 2022

Uh oh!

mshustov Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lukasolson left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 3, 2022

💔 All backports failed

How to fix

Questions ?

Uh oh!

lizozom commented Mar 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

lizozom commented Feb 8, 2022 •

edited

Loading

lizozom commented Feb 8, 2022 •

edited

Loading