-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 execution context - app, page, entitiy id #124996
Conversation
move more things to top level context
…side-execution-context
…side-execution-context
@mshustov I wasn't sure what needs to be done on this line (in the context of swapping parent and child on the context) |
… client-side-execution-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 |
const newVal = { | ||
url: window.location.pathname, | ||
name: this.appId, | ||
...this.context$.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Server-side doesn't merge the contexts but gets the final version. It allows customers to implement any custom logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes LGTM
💔 All backports failed
How to fixRe-run the backport manually:
Questions ?Please refer to the Backport tool documentation |
* 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 <[email protected]> * fix maps context * jest tests for maps * cr * docs * Update execution_context.test.ts * docs * lint Co-authored-by: Marco Liberati <[email protected]> (cherry picked from commit d5416ed) # Conflicts: # src/plugins/discover/public/application/context/context_app.tsx # x-pack/plugins/lens/public/app_plugin/app.tsx
* 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 <[email protected]> * fix maps context * jest tests for maps * cr * docs * Update execution_context.test.ts * docs * lint Co-authored-by: Marco Liberati <[email protected]> (cherry picked from commit d5416ed) # Conflicts: # src/plugins/discover/public/application/context/context_app.tsx # x-pack/plugins/lens/public/app_plugin/app.tsx
…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 <[email protected]>
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 theapplication
serviceAnd 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 beapp
which is a single dashboard andlist
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?
export ELASTIC_APM_ACTIVE=true
[Liza] localhost transactions
.kibana_uuid
in the filter to your localkibana_uuid
. (Some people have their uuid statically set by ftr tests to5b2de169-2785-441b-ae8c-186a1936b17d
- if that's your id, delete your.uuid
file.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:
app-change
transactions get the wrong nameChecklist
Delete any items that are not applicable to this PR.
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:
For maintainers