[mdatagen] Add semconv reference for attributes in metadata schema#14646
[mdatagen] Add semconv reference for attributes in metadata schema#14646osullivandonal wants to merge 17 commits intoopen-telemetry:mainfrom
Conversation
This update to mdatagen allows attributes and resource attributes to reference their semantic convention, this is then included in the generated documentation.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (65.89%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #14646 +/- ##
==========================================
- Coverage 91.45% 91.35% -0.11%
==========================================
Files 688 688
Lines 43824 43992 +168
==========================================
+ Hits 40081 40187 +106
- Misses 2631 2688 +57
- Partials 1112 1117 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The metdata schema has been updated to use relative url refs for linking attributes and metrics to their semantic-conventions. mdatagen now builds the url using the semantic-conventions version of the schema.
| "go.opentelemetry.io/collector/pdata/pcommon" | ||
| ) | ||
|
|
||
| const semConvURL = "https://github.com/open-telemetry/semantic-conventions/blob" |
There was a problem hiding this comment.
While I agree that not having to specify a full URL every time, this assumes all semantic conventions are on the semantic-conventions repository, which may be problematic in the future with open-telemetry/opentelemetry-specification#4906
There was a problem hiding this comment.
I think we can proceed with this as is now and revisit once things are more clear in general about how we will link/define semconv and registries in Collector components in the future.
There was a problem hiding this comment.
Sounds good to me.
ChrsMark
left a comment
There was a problem hiding this comment.
I think that addition will be helpful at the moment to link existing attributes to defined Semantic Conventions.
I would like to use this already in https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/k8sattributesprocessor/documentation.md as part of open-telemetry/opentelemetry-collector-contrib#44483.
| {{- $attribute := . | attributeInfo }} | ||
| | {{ $attribute.Name }} | {{ $attribute.Description }} | | ||
| {{- if $attribute.Enum }} {{ $attribute.Type }}: ``{{ stringsJoin $attribute.Enum "``, ``" }}``{{ else }} Any {{ $attribute.Type }}{{ end }} | {{ $attribute.RequirementLevel }} | | ||
| {{- if $attribute.Enum }} {{ $attribute.Type }}: ``{{ stringsJoin $attribute.Enum "``, ``" }}``{{ else }} Any {{ $attribute.Type }}{{ end }} | {{ $attribute.RequirementLevel }} |{{ if $attribute.SemanticConvention }} [{{ $attribute.Name }}]({{ $attribute.SemanticConvention.SemanticConventionRef }}) |{{ else }} |{{ end }} |
There was a problem hiding this comment.
Maybe we can be more explicit and use a - or something similar to state that there is no semantic convention definition for this. Having this just empty might raise questions.
There was a problem hiding this comment.
I agree, this is good to have, rows with an empty semantic_conventions ref now uses "-":
Attributes
| Name | Description | Values | Requirement Level | Semantic Convention |
|---|---|---|---|---|
| string_attr | Attribute with any string value. | Any Str | Recommended | - |
| state | Integer attribute with overridden name. | Any Int | Recommended | - |
| enum_attr | Attribute with a known set of string values. | Str: red, green, blue |
Recommended | - |
| slice_attr | Attribute with a slice value. | Any Slice | Recommended | - |
| map_attr | Attribute with a map value. | Any Map | Recommended | - |
| if_configured: | ||
| # Optional: the reference to a semantic convention | ||
| # Relative path to the semantic convention definition (e.g., "system.md#system-memory-state") | ||
| # The full URL is constructed using sem_conv_version: {base}/v{version}/docs/registry/attributes/{ref} |
There was a problem hiding this comment.
Can we also add the full generated url as an example here?
i.e. the https://github.com/open-telemetry/semantic-conventions/blob/v1.38.0/docs/registry/attributes/system.md#system-memory-state
There was a problem hiding this comment.
Yup this makes sense, I have updated all references of this to have a full example.
Description
This PR adds support in mdatagen for defining semantic conventions references for attributes.
Updating your attribute in
metadata.yaml:Important
This work also updates the
semantic_convention: ref:to use a relative URL, this was done as we want to ensure the same semconv version from the metadata schema is used, otherwise the end user could have a full url inrefwith one semconv version and the schema version being a different version. A new method has been introduced in mdatagen that builds the semconv url at runtime.Creates this table in the generated docs for that component:
Attributes
buffered,cached,inactive,free,slab_reclaimable,slab_unreclaimable,usedLink to tracking issue
This work is linked to #13297 but doesn't close it.
Testing
cpuandstate.cmd/mdatagen/internal/samplerecieverwas updated and generate code created which creates tests.Documentation
The documentation template has been updated to reference the semconv definition for attributes if provided in the schema.