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

Improve markdown table generation for metrics #129

Open
arminru opened this issue Jan 19, 2023 · 12 comments
Open

Improve markdown table generation for metrics #129

arminru opened this issue Jan 19, 2023 · 12 comments
Labels
semconv/md Related specifically to the markdown output of the semantic convention generator semconv Related to the semantic convention generator.

Comments

@arminru
Copy link
Member

arminru commented Jan 19, 2023

(split from #79)

See https://github.com/open-telemetry/build-tools/pull/79/files#r996803040 for the previous discussion and proposals.

Two possible options would be:

  1. A table of metric names with a (then shared) table of attribute names that apply.
  2. A table of the metric where "empty" rows are created while attributes are expanded.

Both of them are currently manually applied in the hand-crafted metrics tables in the spec repo.

cc @jamesmoessis @Oberon00 @joaopgrassi @jsuereth

@arminru arminru added semconv Related to the semantic convention generator. semconv/md Related specifically to the markdown output of the semantic convention generator labels Jan 19, 2023
@jamesmoessis
Copy link
Contributor

Hey @arminru - is anyone working on this? If not, you can assign to me and I can try get to it in the next few weeks

@arminru
Copy link
Member Author

arminru commented Jan 31, 2023

Not yet! Thank you, @jamesmoessis 🙂

@jamesmoessis
Copy link
Contributor

jamesmoessis commented Feb 2, 2023

I've written 3 options down below of how we might render the tables. I encourage everyone to post their thoughts and preferred option. Once we have a general consensus, I will go ahead and implement the chosen option.

Option 1

Option 1 consists of metrics tables containing multiple metrics, and one attributes table for each individual metric. Option 1 was proposed by @joaopgrassi in previous discussions.

## foo metrics
<!-- semconv metric.foo(metric_table) -->
| Name           | Instrument Type | Unit (UCUM) | Description                   |
|----------------|-----------------|-------------|-------------------------------|
| `foo.size`     | Histogram       | `{bars}`    | Measures the size of foo.     |
| `foo.duration` | Histogram       | `{bazs}`    | Measures the duration of foo. |
<!-- endsemconv -->

### Attributes for `metric.foo.size`
<!-- semconv metric.foo.size -->
| Attribute          | Type   | Description                    | Examples              | Requirement Level                                             |
|--------------------|--------|--------------------------------|-----------------------|---------------------------------------------------------------|
| `http.method`      | string | HTTP request method.           | `GET`; `POST`; `HEAD` | Required                                                      |
| `http.status_code` | int    | [HTTP response status code](). | `200`                 | Conditionally Required: if and only if one was received/sent. |
<!-- endsemconv -->

### Attributes for `metric.foo.duration`
<!-- semconv metric.foo.duration -->
| Attribute     | Type   | Description          | Examples              | Requirement Level |
|---------------|--------|----------------------|-----------------------|-------------------|
| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Optional          |
<!-- endsemconv -->

Option 2

Option 2 consists of a single-row metric table for each metric, with attributes listed below it. This is what currently exists in build-tools.

Similar to option 1, but each metric has a single-row table with the attributes table for that metric directly after it.
Option 2 creates more tables and is less concise, but it provides the advantage that the attributes table is always next to the metric definition, rather than being down the page like in option 1.

This option is similar to what is in the collector's generated metrics tables. Example.

## `foo.size`
<!-- semconv metric.foo.size(metric_table) -->
| Name       | Instrument Type | Unit (UCUM) | Description               |
|------------|-----------------|-------------|---------------------------|
| `foo.size` | Histogram       | `{bars}`    | Measures the size of foo. |
<!-- endsemconv -->

### Attributes for `foo.size`
<!-- semconv metric.foo.size -->
| Attribute          | Type   | Description                    | Examples              | Requirement Level                                             |
|--------------------|--------|--------------------------------|-----------------------|---------------------------------------------------------------|
| `http.method`      | string | HTTP request method.           | `GET`; `POST`; `HEAD` | Required                                                      |
| `http.status_code` | int    | [HTTP response status code](). | `200`                 | Conditionally Required: if and only if one was received/sent. |
<!-- endsemconv -->

## `foo.duration`
<!-- semconv metric.foo.duration(metric_table) -->
| Name           | Instrument Type | Unit (UCUM) | Description                   |
|----------------|-----------------|-------------|-------------------------------|
| `foo.duration` | Histogram       | `{bazs}`    | Measures the duration of foo. |
<!-- endsemconv -->

### Attributes for `foo.duration`
<!-- semconv metric.foo.duration -->
| Attribute     | Type   | Description          | Examples              | Requirement Level |
|---------------|--------|----------------------|-----------------------|-------------------|
| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Optional          |
<!-- endsemconv -->

Option 2b

As @Oberon00 mentioned in the previous PR, the single-row table could be expressed as a list of key-value pairs instead of a table, e.g.:

  • Name: foo.size
  • Instrument Type: Histogram
  • Unit: {bars}
  • Description: Measures the size of foo.

Option 3

Option 3 consists of a metric table where "empty" rows are created while attributes are expanded.

I didn't see this option fully fleshed out, so for those who mentioned it (mainly @jsuereth) I will do my best to provide an example of how I imagine it might look. Feel free to correct me and I can edit this comment.

I've made it so the attribute name and requirement level are specified with the metric table, but then link to a full attribute definition. The standalone attribute definition only needs to be defined once for all metrics, because the requirement level is specified on the metric table already.

This option is the most concise because it avoids repeatedly describing attributes.

<!-- semconv metric.foo.size(metric_table) -->
| Name       | Instrument Type | Unit (UCUM) | Description               | Attribute                                   | Attribute Requirement level                                   |
|------------|-----------------|-------------|---------------------------|---------------------------------------------|---------------------------------------------------------------|
| `foo.size` | Histogram       | `{bars}`    | Measures the size of foo. |                                             |                                                               |
|            |                 |             |                           | [`http.method`](#attribute-definition)      | Required                                                      |
|            |                 |             |                           | [`http.status_code`](#attribute-definition) | Conditionally Required: if and only if one was received/sent. |

...

### Attribute definitions
<!-- semconv metric.foo -->
| Attribute          | Type   | Description                    | Examples              |
|--------------------|--------|--------------------------------|-----------------------|
| `http.method`      | string | HTTP request method.           | `GET`; `POST`; `HEAD` |
| `http.status_code` | int    | [HTTP response status code](). | `200`                 |
<!-- endsemconv -->

@jamesmoessis
Copy link
Contributor

Here are rendered versions of the above options.

Option 1

foo metrics

Name Instrument Type Unit (UCUM) Description
foo.size Histogram {bars} Measures the size of foo.
foo.duration Histogram {bazs} Measures the duration of foo.

Attributes for metric.foo.size

Attribute Type Description Examples Requirement Level
http.method string HTTP request method. GET; POST; HEAD Required
http.status_code int HTTP response status code. 200 Conditionally Required: if and only if one was received/sent.

Attributes for metric.foo.duration

Attribute Type Description Examples Requirement Level
http.method string HTTP request method. GET; POST; HEAD Optional

Option 2:

foo.size

Name Instrument Type Unit (UCUM) Description
foo.size Histogram {bars} Measures the size of foo.

Attributes for foo.size

Attribute Type Description Examples Requirement Level
http.method string HTTP request method. GET; POST; HEAD Required
http.status_code int HTTP response status code. 200 Conditionally Required: if and only if one was received/sent.

foo.duration

Name Instrument Type Unit (UCUM) Description
foo.duration Histogram {bazs} Measures the duration of foo.

Attributes for foo.duration

Attribute Type Description Examples Requirement Level
http.method string HTTP request method. GET; POST; HEAD Optional

Option 2b

  • Name: foo.size
  • Instrument Type: Histogram
  • Unit: {bars}
  • Description: Measures the size of foo.

Option 3

foo.size

Name Instrument Type Unit (UCUM) Description Attribute Attribute Requirement level
foo.size Histogram {bars} Measures the size of foo.
http.method Required
http.status_code Conditionally Required: if and only if one was received/sent.

...

Attribute definitions

Attribute Type Description Examples
http.method string HTTP request method. GET; POST; HEAD
http.status_code int HTTP response status code. 200

@joaopgrassi
Copy link
Member

joaopgrassi commented Feb 8, 2023

With Option 1 you could also have a column that has a link to its table of attributes (anchor to the Attributes for foo.size section). That would make it a bit easier to navigate in case the metrics table gets long.

Option 3 is also a good approach but what I dislike is the potential "back-and-forth" that one might need to go in order to read the attributes. A side question to this: Would this mean we could have attributes in a central file and then link several metrics to it? I get how that's good for maintainability but for reading and consuming.. I think it's not good.

I'm probably biased, but I feel Option 2 (status-quo) is clear and good, apart from the downside of having duplication... but given it's all machine generating this I wonder is that even a problem?

@Oberon00
Copy link
Member

Oberon00 commented Feb 8, 2023

Please also consider moving away from tables entirely where possible, see open-telemetry/opentelemetry-specification#1925

@jsuereth
Copy link
Contributor

jsuereth commented Feb 8, 2023

I think I like option 3 the best, but could we include the attribute descriptions in the original table?

Like @joaopgrassi I'm worried if folks need to jump up/down a lot this will cause frustration using the data, regardless of how we store it in YAML.

Option 2 (if we take less visual separation for attributes or blend 2 + 2b) also appeals to me.

Josh's crazy suggestion

What about allowing for several of these with LINKAGE.

  • Option 1 becomes a table-of-contents where metric names link to a metric section about them.
  • A more unified Option 2 or Option 3 is what you provdie for each metric.

This would give you the best of "let me see all the metrics produced" and "let me see all the details of a metric at once".

@jamesmoessis
Copy link
Contributor

jamesmoessis commented Feb 10, 2023

Good points all around, thank you all.

In my opinion, Option 3 creates a very wide table which may not render nicely in a lot of cases, particularly if the attribute description is added too. There's also a lot of unused space due to the blank cells on the table.

I like @jsuereth's idea of using a table of contents to list the metrics, and linking to more detail on the metric. I like combining this with option 2b. I believe this gives the best of everything. An easy, readable overview of all metrics, whilst also being able to quickly drill down into a specific metric where its attributes are listed and described.

@joaopgrassi
Copy link
Member

@jamesmoessis how would you list the attributes in option 2b?

@jamesmoessis
Copy link
Contributor

@joaopgrassi the same as option 2. Option 2b the metric table changes to be a list of key-values, and the attribute table would be right below that.

@jamesmoessis
Copy link
Contributor

jamesmoessis commented Feb 20, 2023

I do wonder how we might generate the ToC in the correct order of appearance on the page. We don't generate the whole page and there are significant free-form sections, and each metric will be added individually via the <!-- semconv --> tags; so there's not currently a way for one piece of rendered markdown to know the contents and order of the rest of the page.

Generating a table of contents would likely need to be a post-processing step which looks at the markdown headers of the already rendered page.

@joaopgrassi
Copy link
Member

the same as option 2. Option 2b the metric table changes to be a list of key-values, and the attribute table would be right below that.

I see. That is probably the best of both worlds then to me :)

Generating a table of contents would likely need to be a post-processing step which looks at the markdown headers of the already rendered page.

Wouldn't you: 1. run the markdown generation, which will add all the headers and then you run the TOC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semconv/md Related specifically to the markdown output of the semantic convention generator semconv Related to the semantic convention generator.
Projects
Status: Template Generation Fixes
Development

No branches or pull requests

5 participants