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

Add support for multicolumn tables from the probes #1980

Closed
2opremio opened this issue Nov 4, 2016 · 9 comments · Fixed by #2109
Closed

Add support for multicolumn tables from the probes #1980

2opremio opened this issue Nov 4, 2016 · 9 comments · Fixed by #2109
Assignees
Labels
chore Related to fix/refinement/improvement of end user or new/existing developer functionality component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer
Milestone

Comments

@2opremio
Copy link
Contributor

2opremio commented Nov 4, 2016

Right now we only support two-column tables.

As a workaround you can merge columns but it results in an ugly layout.

We should support multiple columns for both prefix-tables and fixed-tables.

We should also support setting headings, alignment and datatype metadata (for ordering and alignment).

@2opremio 2opremio added the chore Related to fix/refinement/improvement of end user or new/existing developer functionality label Nov 4, 2016
@2opremio 2opremio added this to the November2016 milestone Nov 4, 2016
@2opremio
Copy link
Contributor Author

2opremio commented Nov 4, 2016

Motivated by #1981

@rade rade added the component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer label Nov 17, 2016
@fbarl
Copy link
Contributor

fbarl commented Dec 19, 2016

The current table definition we use looks like this (taken from report/table.go):

// Table is the type for a table in the UI.
type Table struct {
	ID              string        `json:"id"`
	Label           string        `json:"label"`
	Rows            []MetadataRow `json:"rows"`
	TruncationCount int           `json:"truncationCount,omitempty"`
}

where MetadataRow is essentially a key-value entry.

I see two ways we could add support for proper multi-column tables:

1. Extending current Table structure

// Table is the type for a table in the UI.
type Table struct {
	ID              string   `json:"id"`
	Label           string   `json:"label"`
	Columns         []Column `json:"columns"`
	Rows            []Row    `json:"rows"`
	TruncationCount int      `json:"truncationCount,omitempty"`
}

To use this structure the old way, we would convert all the MetadataRow entries to 2-column Rows and simply omit the Columns field.

good: We could control the order of tables rendered without adding extra logic for that on frontend.

bad: Table and Labels (a list of key-value pairs) are not semantically that similar so it might be bad to use the same structure to describe them.

2. Adding a separate Table structure

In that case, it would make sense to rename current Table to something more accurate (maybe Labels):

type Labels struct {
	ID              string        `json:"id"`
	Label           string        `json:"label"`
	Rows            []MetadataRow `json:"rows"`
	TruncationCount int           `json:"truncationCount,omitempty"`
}

type Table struct {
	ID              string   `json:"id"`
	Columns         []Column `json:"columns"`
	Rows            []Row    `json:"rows"`
	TruncationCount int      `json:"truncationCount,omitempty"`
}

good: Structures' names better describe what they are and there are no optional/unused attributes.

bad: If we wanted to preserve the flexibility of rendering these structures in a mixed order, we would need to send an additional array of IDs to the frontend and add some logic that handles that in node_details.js.

In both cases, Column and Row could be something like:

type Column struct {
	ID        string `json:"id"`
	Label     string `json:"label"`
	DataType  string `json:"dataType"`
	Alignment string `json:"alignment"`
}

type Row struct {
	Entries map[string]string `json:"entries"`
}

where Row.Entries would map from Column.IDs to table values (although I didn't give this much thought yet so I'm not sure that's optimal).


I think I prefer the second approach, but not sure if it would take more time to implement. @fons, @davkal: I'm curious about your input here, as this decision will determine the path of my PR :)

@2opremio
Copy link
Contributor Author

@fbarl Thanks for this! Your analysis doesn't coverTableTemplate which is how the tables are passed on from the probes to the app (and what I thought would be extended when fixing this issue).

Also, for the connections usecase, @tomwilkie mentioned we should model connections as children but never elaborated.

@davkal
Copy link
Contributor

davkal commented Dec 19, 2016

From the UI perspective we should offer 2 display variants: a table for key-value pairs (aka Labels or Property Lists) as well as generic table. It's the UI's concern to render these, and even render them differently (as they are well-established UI patterns). The generic table has a special case where each row is a representative of another Scope entity (e.g. child rows in container details are processes, same for connections tables which are are either children or peers of the current node). The data structures sent to the frontend need to carry that additional information, to choose a renderer.

If it helps, these can all be tables, if only they are tagged with a type of something like {table,nodetable,labels}.

@davkal
Copy link
Contributor

davkal commented Dec 19, 2016

To clarify, this is a backend decision on how to model this data.

@fbarl
Copy link
Contributor

fbarl commented Dec 19, 2016

@fons The way I see table templates being used in the code, I think their definition could "naturally" follow the corresponding models that are sent to the UI.

So, in the case of extended Table model, we could have something like:

type TableTemplate struct {
	ID        string            `json:"id"`
	Label     string            `json:"label"`
	Prefix    string            `json:"prefix"`
	Columns   []Column          `json:"columns"`
	FixedRows map[string]string `json:"fixedRows"`
}

and with a separate Table model we could leave TableTemplate -> PropertyListTemplate unchanged and introduce a new TableTemplate like this:

type PropertyListTemplate struct {
	ID        string            `json:"id"`
	Label     string            `json:"label"`
	Prefix    string            `json:"prefix"`
	FixedRows map[string]string `json:"fixedRows"`
}

type TableTemplate struct {
	ID        string   `json:"id"`
	Prefix    string   `json:"prefix"`
	Columns   []Column `json:"columns"`
}

Does that make sense or I'm still missing something?


I'm not so sure anymore which approach I prefer, but (unless @fons has a preference) I think I will go with extending the current Table model, also introducing Type field with values {table, labels}, similar to what @davkal mentioned (trying to bring the existing node tables under the same roof would definitely be out of the scope of this issue).

@2opremio
Copy link
Contributor Author

2opremio commented Dec 19, 2016

@fbarl Thanks. From your analysis it's unclear how Prefix and FixedRows will now be used (since they assume a single column table).

Prefix currently indicates the LatestMap prefix used to obtain the rows (with a single column so one entry per row).

And FixedRows uses the same concept. However, the FixedRows keys indicate the exact LatestMap key to obtain the row values and the FixedRows values indicate what row labels to use.

@fbarl
Copy link
Contributor

fbarl commented Dec 21, 2016

@2opremio I'm actually having trouble seeing how @davkal's comment about having 2 display variants (property lists and generic tables) fits in the idea of supporting a multi-column table (possibly with headings) with fixed rows (assuming fixed still means labeled) on the UI.

Maybe it would be helpful for me to be given a concrete use-case of data that we would want to display multi-column with fixed rows, just so that I know what the exact scope of the new generic table should be.

@davkal
Copy link
Contributor

davkal commented Dec 22, 2016

@2opremio could you please explain or ASCII-draw both a prefix-table and a fixed-table so we can see the difference?

From a UI point of view I would require a generic table:

TITLE

header 1 | header 2 | ...
-------------------------
cell 1,1 | cell 2,1 | ....
cell 1,2 | cell 2,2 | ....
....

as well as a property list aka key-value list (currently used to display labels, env vars etc, but is misnamed a "table" in the backend):

TITLE

key1 | value 1
key2 | value 2
...

Notice how the property list has not table headers. A special case of the generic table is the node table where every row corresponds to another entity in Scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to fix/refinement/improvement of end user or new/existing developer functionality component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants