BREAKING: Generate System metrics semconv from YAML + move attributes to their own namespace

[joaopgrassi]

Closes #73

This PR does:

• Moves the system metrics definitions to the YAML model file.
• Because of the "limitations" of tooling AND because the existing metrics share attributes with the same name, this PR also changes the attribute namespaces according with the decision from Should metric attributes be required to be namespaced? #51

Summary of breaking changes

A summary of the changes can be found in this comment. It can also be seen in other "formats" in the changelog + schema file.

---

Still need resolution from #51. There are several attributes in system metrics (e.g., state) that are used in different metrics but with different enum values.

If we want to keep the markdown output as "short" state, we will probably have to see what can be done with the tooling. With the current version of the PR, I have the attributes "namespaced" by using the attribute_group feature in order to re-use the attribute across metrics.

[jsuereth]

As far as what was done in this PR, this LGTM.

• I agree with the namespaces you chose for attributes, as they line up w/ the metrics.
• I agree with the structure/breakdown in the YAML
• I think the final markdown looks good.

We should do one of two things with this PR:

1. Open a (blocking) bug about providing guidance on how to deal with the breaking changes in system metrics.
2. Advise instrumentation to continue using the older System metrics specification until the system metrics working group has had a chance to walk through their issues and declare this stable.

What does everyone think?

[arminru]

This needs to be rebased after #99 is merged (or the other way round).

[joaopgrassi]

I rebased and added the new metrics introduced in #99.

@jsuereth As far as the plan goes, I think option 2 makes sense:

Advise instrumentation to continue using the older System metrics specification until the system metrics working group has had a chance to walk through their issues and declare this stable.

I think once the working group has looked at these metrics and decided they are good, then we can follow up with option 1 and add a issue to track how we will communicate the breaking change and add instructions.

Additionally, we can also add a Notice to the markdown here, saying these have changed and for instrumentations to continue to use the previous version (point to the last release version). What do you think?

[frzifus]

Just to verify if I understood it correctly, this PR changes the attribute values of example system.processes from e.g.: running to system.processes.running ?

[AlexanderWert]

LGTM. Only two minor questions regarding state value members

[dmitryax]

In my opinion, it's hard to track the breaking changes in this PR. I'd suggest separate PR's: 1 for YAML generation with no changes and another one for the adoption of #51. But I'm good to merge as is if @open-telemetry/specs-semconv-maintainers disagree

[joaopgrassi]

@dmitryax

In my opinion, it's hard to track the breaking changes in this PR. I'd suggest separate PR's: 1 for YAML generation with no changes and another one for the adoption of #51.

I definitely agree, but the "problem" is that since multiple metrics use the same attribute key with different semantics there's no way to define them once and share in YAML. The only way I see to convert the existing metrics to YAML keeping the attributes as-is is to duplicate them all over in the YAML definition. If folks think that's the best way to go, I can do that. But it's not a trivial amount of work for just being removed right after.

I will work on this now, to add a changelog entry + the schema file that should highlight the breaking changes more easily. Maybe that helps.

[joaopgrassi]

@dmitryax as we discussed in the SIG meeting yesterday, I changed the PR to revert the system.processes.* pluralization change. The PR is back now to "just" moving the conventions to YAML + introducing the namespaces for the attributes.

I updated the changelog/schema/my wall-of-text comment with the breaking changes summary to reflect this.

Once you create the issue about your concern with the network attributes, please link it here.

[dmitryax]

@joaopgrassi I submitted #308. We should probably do the same for other metric groups

[dmitryax]

LGTM to unblock further changes

[mx-psi]

cc @open-telemetry/semconv-system-approvers the cpu attribute got renamed to system.cpu.logical_number (see more in #89 (comment) ), please speak out if you disagree with the change

[ChrsMark]

cc @open-telemetry/semconv-system-approvers the cpu attribute got renamed to system.cpu.logical_number (see more in #89 (comment) ), please speak out if you disagree with the change

Don't want to block the merge of this but shouldn't the system.cpu.logical_number be under the host. namespace since it's a resource attribute? So it would be host.cpu.logical_number or host.cpu.id similar to what we have for the host.id?

That's related to open-telemetry/opentelemetry-collector-contrib#26533 (comment).

In order to not keep this one blocked we can revisit this as part of System SIG along with the others' attributes stabilization.

[joaopgrassi]

In order to not keep this one blocked we can revisit this as part of System SIG along with the others' attributes stabilization.

@ChrsMark I added your concerns to #308 where we are grouping things to re-visit. Thanks!

[joaopgrassi]

Discussed how to move forward with this PR in the semconv sig meeting (Sept 11th).

• The system semconv WG agrees with this PR and gave the approvals to merge.
• Migration plan: I will send a PR to update the system-metrics.md document and add a section similar to what we have in the HTTP about instrumentations to keep using the old version while it's being stabilized. System semconv WG will then fill in the gaps of the actual migration plan

@open-telemetry/specs-semconv-approvers @open-telemetry/specs-semconv-maintainers this is good to merge!