[APM] Add span subtype and action to Span Flyout #30041
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ction to the flyout if available
💔 Build Failed |
|
retest |
💚 Build Succeeded |
alvarolobato
added
[zube]: Inbox
Team:APM
|
Pinging @elastic/apm-ui |
sorenlouv
reviewed
Feb 6, 2019
| const { type, subtype, action } = getSpanTypes(span); | ||
| const spanTypeLabel = getSpanLabel(type); | ||
| const spanSubtypeLabel = getSpanLabel(subtype); | ||
| const spanActionLabel = getSpanLabel(action); |
There was a problem hiding this comment.
Would it make sense to split getSpanLabel into getSpanTypeLabel, getSpanSubTypeLabel and getSpanActionLabel ?
There was a problem hiding this comment.
... and can we drop the "Label" suffix? Makes me think it's the literal "Type", "Subtype" labels - but it's actually the values
sorenlouv
reviewed
Feb 6, 2019
...nsactionDetails/Transaction/WaterfallContainer/Waterfall/SpanFlyout/StickySpanProperties.tsx
Show resolved
Hide resolved
sorenlouv
reviewed
Feb 6, 2019
| function getSpanTypes(span: Span) { | ||
| const { type, subtype, action } = span.span; | ||
|
|
||
| const [primaryType, subtypeFromType, actionFromType] = type.split('.'); |
There was a problem hiding this comment.
Maybe add a comment, that we do this to support 6.x data.
sorenlouv
approved these changes
Feb 6, 2019
💚 Build Succeeded |
sorenlouv
reviewed
Feb 7, 2019
| const spanTypeLabel = getSpanLabel(getPrimaryType(span.span.type)); | ||
| const { type, subtype, action } = getSpanTypes(span); | ||
| const spanType = formatType(type); | ||
| const spanSubtype = formatSubtype(subtype); |
There was a problem hiding this comment.
Last thing (I promise): can you move these formatters inside getSpanTypes (unless we need the unformatted for anything)
There was a problem hiding this comment.
I prefer spanType, spanSubtype and spanAction as the exported names over type, subtype and action for clarity
💚 Build Succeeded |
ogupte
added a commit
to ogupte/kibana
that referenced
this pull request
Feb 7, 2019
* [APM] closes elastic#26247 by conditionally adding span subtype and action to the flyout if available * renamed vars for better readability and tossed out the formatting on span action * [APM] renamed some variables for clarity
ogupte
added a commit
to ogupte/kibana
that referenced
this pull request
Feb 7, 2019
* [APM] closes elastic#26247 by conditionally adding span subtype and action to the flyout if available * renamed vars for better readability and tossed out the formatting on span action * [APM] renamed some variables for clarity
ogupte
added a commit
that referenced
this pull request
Feb 8, 2019
ogupte
added a commit
that referenced
this pull request
Feb 8, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #26247 by conditionally adding span subtype and action to the flyout if available
