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.

Sign up for GitHub

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

Hide span details in structure tab #291

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Hide span details in structure tab #291

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b31780
Hide span details in structure tab
joey-grafana Jan 14, 2025
7f51789
Change spelling
joey-grafana Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/components/Explore/TracesByService/Tabs/Structure/StructureScene.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,21 @@ const getStyles = (theme: GrafanaTheme2) => {
'div[data-testid="span-detail-component"] > :nth-child(4) > :nth-child(1)': {
display: 'none',
},

// Hide span details row
'.span-detail-row': {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where's that class being used? 🤔 Couldn't find reference to it anywhere.

Hiding with CSS may break if class names change in the TraceView so if we want to go this route I think we should have a test that would ensure elements are really hidden. Also, wouldn't it be nicer if we added a new property to TraceView to control it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added it to the trace view in the Grafana repo PR.

I've also had a look at creating a test for this e.g. render the Traces panel and make sure that some of the child components of the span detail are not visible but it's kind of tricky to mock / provide dummy data for the Structure Tab :( do you have any suggestions?

Adding a new property is a nice touch and was almost going to do that but thought it's a bit overkill for this. We already hide the minimap, header, span and resource accordions via styling in Explore Traces and wanted to hide span details in the same way rather than introduce a new prop that will probably only be used by the app and may confuse users.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We already hide the minimap, header, span and resource accordions via styling in Explore Traces and wanted to hide span details in the same way rather than introduce a new prop that will probably only be used by the app and may confuse users.

The risk is that if TraceView is refactored it may affect the app. Adding tests could help to catch if it happens but only if we tested the app against the latest version of Grafana so it's still quite fragile. I think it's also confusing that we have CSS classes in Grafana that are used outside of Grafana. They become part of a public contract we rely on.

To me it looks like tech debt that can fire back at some point. For backwards compatibility we may need to keep CSS hacks anyway so feel free to go with it but I'd suggest switching to something more explicit in long run 👍

display: 'none',
},

// Remove cursor pointer as span details is hidden
'div[data-testid="TimelineRowCell"]': {
'button[role="switch"]': {
cursor: 'text',
}
},
'div[data-testid="span-view"]': {
cursor: 'text !important',
},
}),
longText: css({
maxWidth: '800px',
Expand Down
Loading