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

fix(frontend): fix analyzer results in trace DAG #2674

Merged
merged 3 commits into from Jun 7, 2023

Conversation

jorgeepc
Copy link
Contributor

@jorgeepc jorgeepc commented Jun 5, 2023

This PR includes multiples fixes/improvements for the Trace Analyzer results in the DAG:

  • one single instance of the Analyzer errors component
  • fix close behavior
  • fix positioning in the DAG
  • add connector line between Analyzer errors component and the Span node

Changes

  • one single instance of the Analyzer errors component
  • fix close behavior
  • fix positioning in the DAG
  • add connector line between Analyzer errors component and the Span node

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

Loom

https://www.loom.com/share/40bc665d8ec3484b9b168d8706666150

@jorgeepc jorgeepc self-assigned this Jun 5, 2023
@jorgeepc jorgeepc marked this pull request as ready for review June 6, 2023 21:49
@jorgeepc jorgeepc requested a review from xoscar June 6, 2023 21:50
@olha23
Copy link

olha23 commented Jun 7, 2023

Thank you @jorgeepc for the update, looks good

Copy link
Collaborator

@xoscar xoscar left a comment

Choose a reason for hiding this comment

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

Hey @jorgeepc great job man!

import * as S from './SpanNode.styled';

interface IProps extends NodeProps<INodeDataSpan> {}

const SpanNode = ({data, id, selected}: IProps) => {
const dispatch = useAppDispatch();
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to consider cleaning up nodes to not have to be connected to all of this state and such, maybe something to think latter on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, agree with that. I have been thinking about that for a while. We need to parse everything (span related) and inject it in the data object.

@jorgeepc jorgeepc merged commit a25c379 into main Jun 7, 2023
24 checks passed
@jorgeepc jorgeepc deleted the 2638-fix-trace-dag-analyzer branch June 7, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve search input and graph positioning Bug with analyzer error
3 participants