-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[7.7] Instrument Kibana with APM RUM agent (#44281) #62484
Conversation
* Instrument Kibana with APM RUM agent * make route-change transaction work with properl url * extract page-load transaction url from app link * check if app is hidden and set active:false * make distributed tracing work and merge config * remove config/apm.js and address review * address review comments * add apm.js to build tassks * move apm from dev to src * add @types/hoist-non-react-statics which is required by react rum * apply changes correctly from master
@@ -88,6 +88,9 @@ export interface InjectedMetadataParams { | |||
user?: Record<string, UserProvidedValues>; | |||
}; | |||
}; | |||
apm: { |
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.
apm
was moved inside vars
metadata which is done on this PR - #53832, so this change is not required here.
@@ -26,11 +28,22 @@ export const appEntryTemplate = bundle => ` | |||
* context: ${bundle.getContext()} | |||
*/ | |||
// import global polyfills |
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.
Polyfills were not added as part of the RUM instrumentation PR. This doesnt seem to be correct.
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.
Does ${apmImport()}
still go there?
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.
Yes that should still be there.
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.
Looks good Thanks @smith
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Backports the following commits to 7.7: