Skip to content
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] Display transaction info on span flyout #24189

Merged

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Oct 18, 2018

This PR add transaction info to the SpanFlyout, in particular the service name and the transaction name.

screen shot 2018-10-18 at 11 20 42

Related #23568

@sorenlouv sorenlouv requested a review from jasonrhodes October 18, 2018 09:18
@sorenlouv sorenlouv force-pushed the add-parent-transaction-span-items branch from 0906748 to 5da4dac Compare October 18, 2018 09:22
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sorenlouv sorenlouv force-pushed the add-parent-transaction-span-items branch from d3baa5e to 10dd510 Compare October 18, 2018 13:14
@sorenlouv
Copy link
Member Author

Rebased and ready for review

@sorenlouv sorenlouv added the Team:APM All issues that need APM UI Team support label Oct 18, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui

@formgeist
Copy link
Contributor

@sqren I see that the span type still has a colour legend attached. We should probably remove this and add it to the service instead since it's what colours the rows in the trace?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sorenlouv
Copy link
Member Author

@sqren I see that the span type still has a colour legend attached. We should probably remove this and add it to the service instead since it's what colours the rows in the trace?

Good point. There are also a few other outstanding taks (like the Discover button) that I was thinking about doing in a new PR but might just do it here.

@formgeist
Copy link
Contributor

@sqren I can also add it to a design polish issue, which I'll open soon to catch all these things (hopefully) 😉

<div>
<HeaderXSmall>DB Statement</HeaderXSmall>
<Fragment>
<EuiText>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also EuiTitle, not 100% sure which is best for which

Copy link
Member Author

@sorenlouv sorenlouv Oct 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't know. Maybe that's better. Will take a look.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the EuiTitle size=xs for these titles

@jasonrhodes
Copy link
Member

@sqren can you add "hasBorder" to the EuiFlyoutHeader component?

<EuiFlyoutHeader hasBorder>

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sorenlouv sorenlouv force-pushed the add-parent-transaction-span-items branch from 4cc5841 to 9397bf0 Compare October 18, 2018 15:54
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sorenlouv sorenlouv force-pushed the add-parent-transaction-span-items branch from 5ee9bc1 to edd6d4c Compare October 18, 2018 19:58
@elasticmachine
Copy link
Contributor

💔 Build Failed

@jasonrhodes
Copy link
Member

I'm going to merge this in a few minutes, even though the build is failing, because the fix is in the apm/dt branch where this is merging. If there are still failing tests after it merges, I'll fix in apm/dt.

@sorenlouv
Copy link
Member Author

Merge away 👍

@jasonrhodes jasonrhodes merged commit 4d01d0b into elastic:apm/dt Oct 18, 2018
jasonrhodes pushed a commit that referenced this pull request Oct 23, 2018
* 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
jasonrhodes added a commit that referenced this pull request Oct 23, 2018
* Adds traces overview with mock data (#22628)

* 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

* [APM] Distributed tracing - Trace details (waterfall) (#22763)

* [APM] Add typescript to waterfall (#23635)

* [APM] Migrate get_trace and constants to Typescript (#23634)

* Bump lock files

* [APM] Add types for setup_request (#23762)

* [APM] Adds trace overview queries and some refactoring (#23605)

Refactoring includes:
* 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)

Fix tests

Make interfaces versioned

Fix ts issues

Rename eventType to docType

WIP

Working prototype

Fix tests

Limit get_transaction by trace.id

* Fixes trace links on traces overview (#24089)

* [APM] use react-redux-request (#24117)

* [APM] use react-redux-request

* Stricter type checking

* Bump yarn lock files

* Fix package.json

* 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

* Add links

* Fix tests

* Eui fixes

* Remove color from span type

* Fix failing tests

* Adds versions to transaction and span itmes when they are returned from the API (#24186)

* Timeline transaction flyout (#24027)

* 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

* [APM] Minor docs cleanup (#24325)

* [APM] Minor docs cleanup

* Fix tests

* Make `text` required

* Simplify documentation module

* Break docs into separate files

* Remove `translateAgentName` method

* Formatting

* [APM] Fix issues with v1 spans (#24332)

* [APM] Add agent marks (#24361)

* Typescript migration for the transaction endpoints (#24397)

* 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
@sorenlouv sorenlouv deleted the add-parent-transaction-span-items branch October 24, 2018 13:07
sorenlouv added a commit that referenced this pull request Oct 24, 2018
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:APM All issues that need APM UI Team support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants