erlang process.runtime attributes: remove TODO note and update descriptions #1445
Conversation
|
I don't know what the semantic convention failure is about. Are docs on this in the repo? |
|
@yurishkuro in the latest |
|
This reminded me to update the spec matrix, so I've now included that in this PR as well. |
| | Name | `process.runtime.name` | `process.runtime.version` | `process.runtime.description` | | ||
| | --- | --- | --- | --- | | ||
| | beam | BEAM | 11.0.3 | Erlang/OTP 24 erts-11.0.3 | | ||
| <!-- semconv process.runtime --> |
There was a problem hiding this comment.
This would add the same table as above when running the semantic conventions generator tool. Since this table is maintained manually, you'll have to remove those markers or the build fails. Currently it's complaining about a mismatch with the (potentially) generated files. See how it's done with the other tables below.
| | Name | `process.runtime.name` | `process.runtime.version` | `process.runtime.description` | | ||
| | --- | --- | --- | --- | | ||
| | beam | BEAM | 11.0.3 | Erlang/OTP 24 erts-11.0.3 | | ||
| <!-- semconv process.runtime --> |
There was a problem hiding this comment.
tsloughter
force-pushed
the
erlang-process.runtime
branch
from
985428c
to
ca9600b
Compare
|
Ah, woops, thanks @arminru and @SergeyKanzhelev :). I had copied from the earlier table in the document without thinking, should be fixed now. |
| | Name | `process.runtime.name` | `process.runtime.version` | `process.runtime.description` | | ||
| | --- | --- | --- | --- | | ||
| | beam | BEAM | 11.0.3 | Erlang/OTP 24 erts-11.0.3 | | ||
| | Attribute | Type | Description | Examples | Required | |
There was a problem hiding this comment.
To be consistent with other languages, I would keep the list form, as in the removed text.
There was a problem hiding this comment.
Looking at it again, this even repeats the Type and Required column. I think this is rather misleading. Please either:
- Revert to the list notation as above, just remove the TODO, maybe add a table with examples (as for other runtimes like Java) if you have more than one; or
- remove the Type and Required columns from the table.
| | Name | `process.runtime.name` | `process.runtime.version` | `process.runtime.description` | | ||
| | --- | --- | --- | --- | | ||
| | beam | BEAM | 11.0.3 | Erlang/OTP 24 erts-11.0.3 | | ||
| | Attribute | Type | Description | Examples | Required | |
There was a problem hiding this comment.
Looking at it again, this even repeats the Type and Required column. I think this is rather misleading. Please either:
- Revert to the list notation as above, just remove the TODO, maybe add a table with examples (as for other runtimes like Java) if you have more than one; or
- remove the Type and Required columns from the table.
tsloughter
force-pushed
the
erlang-process.runtime
branch
from
cef5e84
to
1eb74cd
Compare
|
Switched it back to a list and merged the spec matrix with the latest in |
|
Ok, added an example table. |
No real change here, simply removes the TODO since it is done and updates the attributes into a table.
Related issue: open-telemetry/opentelemetry-erlang#96