Page MenuHomePhabricator

Initial pass at structured log display in the run view
ClosedPublic

Authored by bengotow on Jul 3 2019, 5:39 PM.

Details

Summary

Fix bug where scrolling to the top scrolls you all the way to the bottom of the logs

Initial + hacky fixes to the log viewer to incorporate structured logs

Restructure log viewer to separate table implementation details from content

WIP

Styling as of July 2

Slightly improved styling

More UI iteration

More cleanup and refactoring

Test Plan

Run tests

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bengotow created this revision.Jul 3 2019, 5:39 PM
bengotow updated this revision to Diff 2886.Jul 15 2019, 8:04 PM
  • Put input / output names in the label column
  • Use typecheck metadata rather than valueRepl (there appears to be none yet)

can you upload a screenshot?

Hey folks! Here's are a few screenshots of the latest changes. (Note the new table UI for the metadata entries)

bengotow updated this revision to Diff 2891.Jul 15 2019, 9:43 PM
  • Fix truncation bug and placement of “read entire message” text
  • Update failing snapshot tests
schrockn requested changes to this revision.Jul 16 2019, 1:19 PM

Ok this is *almost* there.

A couple things.

  1. I think the "Label" and "Value" column are a bit superfluous and it makes it look more clutter because it is another font in this ui. I think we could communicate that these are label in another more subtle way, even if it just appending a colon to labels. We could also make the tables slightly more visually heavy in order to make their structured nature very clear.

  1. For inputs and outputs specifically we are going to want to render the type information.

The words "PandasDataFrame" should appear somewhere in the following screenshot.

  1. I think we need column headers and adjustable column widths now that we have more columns. This can be in a followup.
  1. For things like expectations -- or anything else with structured metadata -- there will be a user-provided free text description field. We will need to render this somewhere.
This revision now requires changes to proceed.Jul 16 2019, 1:19 PM

This should address 1-3 (table styling is more apparent and does not have headers, entire log view now has headers and the dividers can be dragged left/right to adjust column widths. sizing is saved between page loads. The input/output types are displayed in the standard blue) You can also now filter by node type by typing type:expectation or `type: step start, etc. (It just does a partial string match against the event graphQL typenames for now)

There's no description field integration yet, but I imagine it could go in a tooltip somewheere. If it's really just another piece of structured metadata we could also list it in the table with a pop-out to view (perhaps with a markdown formatter or something.)

Screenshot looking a lot better.

  1. Add a timestamp label for the last column.
  2. It would be nice if for short input names and type names it could be on one line like "result: FileHandle" The other option would be to have the type be a row in the metadata entry table.

With those minor changes let's merge this and start iterating.

bengotow updated this revision to Diff 3034.Jul 18 2019, 3:11 PM
  • Displaying IO types in the structured logs, filtering by step type
  • Switch to using virtualized list instead of grid, since we do not want all grid features
  • Add draggable headers and switch to react-virtualized list component
bengotow added a comment.EditedJul 18 2019, 3:12 PM

Updated screenshot above!

Minor note: The timestamp column is not resizable - the content in that column is fixed width so it seemed like it'd be odd. We can change it if you want though! Fixing that column also allows us to position the "Show Entire Message" button on truncated lines more efficiently since it's a fixed inset from the right edge.

Doesn't seem like the buildkite errors above are relevant to this diff, not sure what's going on with the python tests.

@bengotow rebase off master. it's a dep issue. library we depend on release breaking changes

schrockn accepted this revision.Jul 18 2019, 4:16 PM

ok this version is amazing

This revision is now accepted and ready to land.Jul 18 2019, 4:16 PM
bengotow updated this revision to Diff 3036.Jul 18 2019, 7:15 PM
  • Put input / output names in the label column
  • Use typecheck metadata rather than valueRepl (there appears to be none yet)
  • Improved metadata table layout
  • Fix truncation bug and placement of “read entire message” text
  • Displaying IO types in the structured logs, filtering by step type
  • Switch to using virtualized list instead of grid, since we do not want all grid features
  • Add draggable headers and switch to react-virtualized list component
  • Add timestamp heading, adjust styles and column names
  • Rebase

Updating release notes with a description of this feature would be great