Skip to content

feat(spans): Embed span trees of transactions that are direct children#26903

Merged
dashed merged 16 commits into
masterfrom
embedded-transaction
Jun 29, 2021
Merged

feat(spans): Embed span trees of transactions that are direct children#26903
dashed merged 16 commits into
masterfrom
embedded-transaction

Conversation

@dashed

@dashed dashed commented Jun 25, 2021

Copy link
Copy Markdown
Member

Using the information provided by the trace navigation feature, we can identify transactions that are direct children to the current transaction, and embed their span trees into the current span view.

For example, this enables customers to be able to see backend transactions inline with the frontend spans.

This product feature will be gated behind the organizations:unified-span-view feature flag.

Functionality that appears to be missing will be added in follow-up pull-requests.

Kapture.2021-06-25.at.17.27.33.mp4

@dashed dashed requested a review from a team June 25, 2021 21:33
@dashed dashed self-assigned this Jun 25, 2021

@Zylphrex Zylphrex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great! so much easier to follow than it used to be.

return null;
}

if (transactions && transactions.length === 1) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will only support spans with 1 child transaction at the moment right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's right. I aim to support the case of 2 or more children in a follow up pull request once this prototype validates well.

<DividerContainer>
{this.renderDivider(dividerHandlerChildrenProps)}
{this.renderErrorBadge(errors)}
{this.renderEmbeddedTransactionsBadge(transactions)}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume the case where there's is both an error and a child transaction will be left as a TODO item?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep. I'm going to follow up in another pull request that will include some hover state effects to be able to support this.


this.embeddedChildren = [parsedRootSpan];
})
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be a followup but should this handle the .catch case and display an error or something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There definitely should be a .catch() here. I refactored to roll up the loading and error states into this.fetchEmbeddedChildrenState.

@dashed dashed merged commit d75a984 into master Jun 29, 2021
@dashed dashed deleted the embedded-transaction branch June 29, 2021 19:39
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants