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

Table type should be more flexible #301

Closed
peterbourgon opened this issue Jul 2, 2015 · 15 comments · Fixed by #752
Closed

Table type should be more flexible #301

peterbourgon opened this issue Jul 2, 2015 · 15 comments · Fixed by #752
Assignees
Milestone

Comments

@peterbourgon
Copy link
Contributor

Right now the Table has a fixed and generic "schema" which isn't totally appropriate for everything we want to show in the details pane. Let's figure out a way to have different types of Tables for different types of information. Motivated by #298.

@2opremio
Copy link
Contributor

This ticket is rightly motivated by how we currently abuse Table rows to insert the Client - Server headings in the Connections table:

screen shot 2015-08-24 at 22 17 12

To sort this out and to allow other hierarchical representations such as Processes by host or Processes by container which I plan to add in the near future I propose adding support for nested tables by:

  1. Modifying the Table data structure from:

    type Table struct {
        Title   string `json:"title"`   // e.g. Bandwidth                                                                                                                                                                                    
        Numeric bool   `json:"numeric"` // should the major column be right-aligned?                                                                                                                                                         
        Rank    int    `json:"-"`       // used to sort tables; not emitted.                                                                                                                                                                 
        Rows    []Row  `json:"rows"`
    }

    to

    type Marshable interface {
        func Marshal(v interface{}) ([]byte, error)
    }
    
    type Table struct {
        Heading Row         `json:"heading"` // e.g. Bandwidth                                                                                                                                                                               
        Numeric bool        `json:"numeric"` // should the major column be right-aligned?                                                                                                                                                    
        Rank    int         `json:"-"`       // used to sort tables; not emitted.                                                                                                                                                            
        Rows    []Marshable `json:"rows"`
    }

    Note how this has the effect of allowing expandable columns in the tittle and nested tables, since the Rows slice can now also other tables, not just rows.

  2. Adding a type field (with value row or table) when serializing to make it easy to distinguish simple rows and nested tables on the client side.

Using the example image above, when embedding the connections as a subtable, the JSON representation of the Connections would change from:

{
    "title": "Connections",
    "numeric": false,
    "rows": [
        {
            "key": "TCP connections",
            "value_major": "2"
        },
        {
            "key": "Client",
            "value_major": "Server",
            "expandable": true
        },
        {
            "key": "127.0.0.1:59680",
            "value_major": "127.0.0.1:4040",
            "expandable": true
        },
        {
            "key": "127.0.0.1:59680",
            "value_major": "127.0.0.1:4040",
            "expandable": true
        }
    ]
}
{
    "heading": {
        "key": "Connections",                                                                                                                                                                                                                
        "value_major": ""
    },
    "numeric": false,
    "rows": [
        {
            "type": "row",
            "key": "TCP connections",
            "value_major": "2"
        },
        {
            "type": "table",
            "heading": {
                "key": "Client",
                "value_major": "Server",
                "expandable": true
            },
            "rows": [
                {
                    "type": "row",
                    "key": "127.0.0.1:3445",
                    "value_major": "127.0.0.1:80",
                    "expandable": true
                },
                {
                    "type": "row",
                    "key": "127.0.0.1:5647",
                    "value_major": "127.0.0.1:560",
                    "expandable": true
                }
            ]
        }
    ]
}

There is no equivalent of nested table in Google's Material Design so I am unsure on how to represent this graphically but I would propose indenting subtables to the right.

@tomwilkie @peterbourgon @davkal Please provide feedback (or a go ahead ) before I get down to coding it.

@peterbourgon
Copy link
Contributor Author

I might be commenting from ignorance, but I thought the idea was to eschew the JSON intermediate form altogether, and serve prerendered HTML directly to the UI? (Can discuss more in person...)

@tomwilkie
Copy link
Contributor

Fons and I discussed that approach - why did you favour this way, Fons?

On Tuesday, 25 August 2015, Peter Bourgon [email protected] wrote:

I might be commenting from ignorance, but I thought the idea was to eschew
the JSON intermediate form altogether, and serve prerendered HTML directly
to the UI? (Can discuss more in person...)


Reply to this email directly or view it on GitHub
#301 (comment).

@davkal
Copy link
Contributor

davkal commented Aug 25, 2015

I remember having a conversation where we agreed to not go generic and implement renderers for all kinds of data, e.g. a connectionRenderer would render something like source <-> target, and data would have a render type attached to know how to render it. Is that not feasible anymore?

If we wanna go generic, we could implement a couple of abstract render types, like tables, lists, labels, tags. In Cello we solved the nested data issue, by rendering the nested data differently, see how a "table" if return code counts is nested here:

screen shot 2015-08-25 at 11 13 48

@2opremio
Copy link
Contributor

re HTML: I was trying to leave the presentation to the client. If I understood it correctly, the reason behind forwarding HTML was to free up @davkal from adapting the client since he is overly busy nowadays. However, the change I am proposing doesn't seem too complicated to implement in the client. If it is, ( @davkal please raise your voice on this) I will reluctantly cave in and output HTML instead :)

@davkal Regarding the idea of independent renderers. Can you elaborate? Are you referring to renderers on the client/on the server? How would the data transferred from the server to the client look like?

@peterbourgon
Copy link
Contributor Author

It's less about this specific change, more about the fact this is likely just the beginning of many iterations on the details pane...

@davkal
Copy link
Contributor

davkal commented Aug 26, 2015

@2opremio I would strongly advise against sending HTML, it would be working around the react framework, plus if you construct HTML in the backend, you might as well just do it in the frontend right away.

Re Independent renderers:

They would be react components, eg

<Connection source={source} target={target} />

Which just need an input object like

{
"type": "connection",
"source": "1.2.3.4",
"target": "5.6.7.8"
}

It's up to the component to implement a render function, which could be as simple as outputting:

{source} <-> {target}

The data object for the whole details pane would then be a hierarchical json object. Each layer of the hierarchy would have a type attribute to indicate which renderer component it needs.
In that scheme, our current table renderer could be wrapped in a table component and it will be used when encountering an object with type: "table".

@2opremio
Copy link
Contributor

@davkal Then we wouldn't (yet) need subtables/headings but still we need heterogeneous rows. Just to be 100% clear is this the change which you would expect?

From:

{
    "title": "Connections",
    "numeric": false,
    "rows": [
        {
            "key": "TCP connections",
            "value_major": "2"
        },
        {
            "key": "Client",
            "value_major": "Server",
            "expandable": true
        },
        {
            "key": "127.0.0.1:59680",
            "value_major": "127.0.0.1:4040",
            "expandable": true
        },
        {
            "key": "127.0.0.1:59680",
            "value_major": "127.0.0.1:4040",
            "expandable": true
        }
    ]
}

To:

{
    "type": "table",
    "title": "Connections",
    "numeric": false,
    "rows": [
        {
            "type": "row",
            "key": "TCP connections",
            "value_major": "2"
        },
        {
            "type": "connection",
            "source": "127.0.0.1:59680",
            "destination": "127.0.0.1:4040"
        },
        {
            "type": "connection",
            "source": "127.0.0.1:59680",
            "destination": "127.0.0.1:4040"
        }
    ]
}

On the Go side this can be done by changing:

type Table struct {
        Title   string `json:"title"`   // e.g. Bandwidth                                                                                                                                                                                    
        Numeric bool   `json:"numeric"` // should the major column be right-aligned?                                                                                                                                                         
        Rank    int    `json:"-"`       // used to sort tables; not emitted.                                                                                                                                                                 
        Rows    []Row  `json:"rows"`
}

To:

type Marshable interface {
        func Marshal(v interface{}) ([]byte, error)
}

type Table struct {
        Title   string      `json:"title"`   // e.g. Bandwidth                                                                                                                                                                               
        Numeric bool        `json:"numeric"` // should the major column be right-aligned?                                                                                                                                                    
        Rank    int         `json:"-"`       // used to sort tables; not emitted.                                                                                                                                                            
        Rows    []Marshable `json:"rows"`
}

type Connection struct {
        Source      string `json:"source"`
        Destination string `json:"destination"`
}

@davkal
Copy link
Contributor

davkal commented Aug 28, 2015

@2opremio It still needs to be hierachical, as in

{
    "type": "table",
    "title": "Connections",
    "numeric": false,
    "rows": [
        {
            "type": "row",
            "key": "TCP connections",
            "value_major": "2",
            "details": [
                {
                    "type": "connections",
                    "connections": [
                        {
                            "type": "connection",
                            "source": "127.0.0.1:59680",
                            "destination": "127.0.0.1:4040"
                        },
                        {
                            "type": "connection",
                            "source": "127.0.0.1:59680",
                            "destination": "127.0.0.1:4040"
                        }
                    ]
                }
            ]
        }
    ]
}

The renderer then determines how each thing is rendered, e.g. details could be a collapsible row detail.

What do you think?

@2opremio
Copy link
Contributor

@davkal I like the collapsible-details concept but not so much having them in a row and them having a type (I think that only its inner content should have a type). How about this?

{
    "type": "table",
    "title": "Connections",
    "numeric": false,
    "rows": [
        {
            "type": "row",
            "key": "TCP connections",
            "value_major": "2"
        }
    ],
    "details": [
        {
            "type": "connection",
            "source": "127.0.0.1:59680",
            "destination": "127.0.0.1:4040"
        },
        {
            "type": "connection",
            "source": "127.0.0.1:59680",
            "destination": "127.0.0.1:4040"
        }
    ]
}

@davkal
Copy link
Contributor

davkal commented Sep 1, 2015

@2opremio the main point I wanted to make was that of ownership. Who owns those connection details? I would argue it's the row, because it's a generic table. We dont know what details other rows might have.

@2opremio
Copy link
Contributor

2opremio commented Sep 1, 2015

@davkal OK, I see what you mean. But what you are suggesting implies that the details always belong to a particular row and I am not sure that will always be the case.

In fact:

  1. As I understand it, all the connection details are not necessarily coming from TCP connections (we could split them in UDP/TCP but maybe we don't want to).
  2. I am assuming (please tell me if I'm wrong) that the details will be collapsible/expandable by clicking on the table's title not individual rows (because of (1)).

@davkal
Copy link
Contributor

davkal commented Sep 1, 2015

@2opremio I want this to be compose-able, nothing should always belong to something.

.1. As I understand it, all the connection details are not necessarily coming from TCP connections (we could split them in UDP/TCP but maybe we don't want to).

If we need adjust where the details reside, so be it, and then that new entity will own the details. The UI code change will then be minimal because I just use the same renderer in the new component.

.2. I am assuming (please tell me if I'm wrong) that the details will be collapsible/expandable by clicking on the table's title not individual rows (because of (1)).

I dont know yet, but I want the flexibility to try out both variants.

@2opremio
Copy link
Contributor

2opremio commented Sep 2, 2015

@davkal Makes sense. Will take this up in the coming days following what we have agreed on.

@tomwilkie
Copy link
Contributor

As discussed, Paul is going to working on this kinda issue next.

@tomwilkie tomwilkie assigned paulbellamy and unassigned 2opremio Nov 30, 2015
@tomwilkie tomwilkie added this to the 0.11.0 milestone Nov 30, 2015
@tomwilkie tomwilkie modified the milestones: 0.12.0, 0.11.0 Dec 10, 2015
@davkal davkal mentioned this issue Dec 17, 2015
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants