-
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] Distributed Tracing #24062
[APM] Distributed Tracing #24062
Conversation
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
💔 Build Failed |
* Adds traces overview with mock data * Updates service overview snapshots * Updates based on review feedback * Adds tests for ManagedTable and ImpactBar * Refactored transaction overview to use new managed table component * Fixes tab size * Cleans up some tests and types * Cleans up types and tests * kbn bootstrap changes to x-pack yarn.lock * Removed jsconfig file in apm * Updates snapshot tests * Reversed bogus yarn lock change in x-pack * review feedback wip * Resolves typescript issues
Refactoring includes: * ImpactBar component to align EuiProgress usage for impact bars * Sharing some logic between transaction and trace queries * Typescript support
Fix tests Make interfaces versioned Fix ts issues Rename eventType to docType WIP Working prototype Fix tests Limit get_transaction by trace.id
* [APM] use react-redux-request * Stricter type checking
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
@@ -167,7 +167,7 @@ | |||
"react-input-range": "^1.3.0", | |||
"react-markdown": "^3.1.4", | |||
"react-redux": "^5.0.7", | |||
"react-router-dom": "4.2.2", | |||
"react-router-dom": "^4.3.1", |
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.
@jasonrhodes We should make sure this doesn't have any breaking changes.
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.
It’s a minor update so it can’t be breaking.
lol - yeah the other package.json had the caret so the newer version was already installed there when we were getting that TS error yesterday, I think. I’ll take a look at the change log.
* [APM] Display transaction info on span flyout * Add links * Fix tests * Eui fixes * Remove color from span type
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
* Duplicates transaction properties table for use in flyout, with dummy values * Brings in real location and url param data for transaction flyout * Converts flyout to TS * Adds query param state for flyouts with ts support * Updates styles and uses EuiTabs for transaction flyout * Updates type references after rebase * Updated index var name per review feedback * WIP transaction flyout * WIP transaction flyout * WIP * Fixes waterfall helpers after rebasing span flyout changes * Finalizes styling for transaction flyover from timeline * Fixes layout for span flyout header * Removed accidentally committed file (should be in other branch) * Small tweaks to the sticky property layout styling * Updates snapshot tests after sticky properties converting to EUI * Final review tweaks for TS
💚 Build Succeeded |
* [APM] Minor docs cleanup * Fix tests * Make `text` required * Simplify documentation module * Break docs into separate files * Remove `translateAgentName` method * Formatting
💚 Build Succeeded |
💔 Build Failed |
💔 Build Failed |
retest |
💚 Build Succeeded |
Transaction sample header completed * Fixes link target for traces overview to include trans/trace ids as query params * Converts Transaction index file to TS * Adds trace link to sample section * Refactors the trace link and applies it to two usages * Implements transaction sample action context menu * Calculates and implements duration percentage * Re-typed how transaction groups work * Fixes transaction flyout links and context menu * Removes unnecessary ms multiplication * Removes unused commented code * Finalizes infra links * Fixes some type shenanigans
💚 Build Succeeded |
if (value === 0) { | ||
return '0'; | ||
} | ||
const formatted = numeral(value).format('0.0'); |
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.
const formatted = numeral(value).format('0.0'); | |
const formatted = asInteger(value); |
|
||
function formatString(value: string) { | ||
return value || 'N/A'; | ||
} |
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.
This is unnecessary. With typescript we know that transaction.transaction.name
and transaction.context.service.name
will always be defined.
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.
Sounds good -- I can clean these little things up in my beta ticket so we don't make this PR restart CI too many more times before getting it into master. :)
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.
Sorry, didn't mean to hold up the merge. Just adding notes for future fix. Non of this is blocking.
💚 Build Succeeded |
💚 Build Succeeded |
pid: number; | ||
}; | ||
service: ContextService; | ||
system: ContextSystem; |
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.
Optional
architecture: string; | ||
hostname: string; | ||
ip: string; | ||
platform: string; |
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.
* Adds traces overview with mock data (#22628) * Updates service overview snapshots * Adds tests for ManagedTable and ImpactBar * Refactored transaction overview to use new managed table component * Removed jsconfig file in apm * [APM] Distributed tracing - Trace details (waterfall) (#22763) * [APM] Add typescript to waterfall (#23635) * [APM] Migrate get_trace and constants to Typescript (#23634) * [APM] Add types for setup_request (#23762) * [APM] Adds trace overview queries and some refactoring (#23605) * ImpactBar component to align EuiProgress usage for impact bars * Sharing some logic between transaction and trace queries * Typescript support * Quick fix ‘banana’ * [APM] Ensure backwards compatibility for v1 and v2 (#23636) * Make interfaces versioned * Rename eventType to docType * Fixes trace links on traces overview (#24089) * [APM] use react-redux-request (#24117) * Updated yarn lockfile for new yarn version * Updated dependency issues for react-router-dom types * [APM] Display transaction info on span flyout (#24189) * [APM] Display transaction info on span flyout * Brings in real location and url param data for transaction flyout * Converts flyout to TS * Adds query param state for flyouts with ts support * Updates styles and uses EuiTabs for transaction flyout * [APM] Transaction flyout * [APM] Minor docs cleanup (#24325) * [APM] Minor docs cleanup * [APM] Fix issues with v1 spans (#24332) * [APM] Add agent marks (#24361) * [APM] Typescript migration for the transaction endpoints (#24397) * [APM] DT transaction sample header (#24294) Transaction sample header completed * Fixes link target for traces overview to include trans/trace ids as query params * Converts Transaction index file to TS * Adds trace link to sample section * Refactors the trace link and applies it to two usages * Implements transaction sample action context menu * Calculates and implements duration percentage * Re-typed how transaction groups work * Fixes transaction flyout links and context menu * Removes unnecessary ms multiplication * Removes unused commented code * Finalizes infra links * Fixes some type shenanigans
WIP