Added display_name property to all registry attribute groups

[AlexanderWert]

Fixes #978

[AlexanderWert]

To make CI happy we need also: open-telemetry/build-tools#316

[trisch-me]

thanks for quick implementing it. @jsuereth could add this now into the final PR to populate correct name

[ChrsMark]

LGTM

[trisch-me]

One question though - should we enforce then a presence of this property for registry?

[AlexanderWert]

One question though - should we enforce then a presence of this property for registry?

yeah, I guess we could extend open-telemetry/build-tools#316 to include it in the validity checks

[lmolkova]

Could you please create an issue to update schema definition (or just update the schema definition) in the https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/semconv.schema.json to include it?

We want to move it to this repo #916, but it's not done yet

[noahfalk]

LGTM

[noahfalk]

@JamesNK (Just fyi - This looks like a minor cosmetic change)

[jsuereth]

One question though - should we enforce then a presence of this property for registry?

Once we get the rule-engine/checker working in weaver, this is a simple configuration for semantic-conventions. Let's see if we can get that prioritized as a the next feature we need from weaver for semconv.

[jsuereth]

Thanks for adding all of these.

We need to update both build-tools and weaver. I merged your build-tools change (will need a release) but we'll also need to update weaver.

Since the weaver PR merged, you should be able to actually update the weaver attribute_group.md.j2 template to use this value in this PR.

[AlexanderWert]

Could you please create an issue to update schema definition (or just update the schema definition) in the https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/semconv.schema.json to include it?

Done, see: open-telemetry/build-tools#317

[AlexanderWert]

Since the weaver PR merged, you should be able to actually update the weaver attribute_group.md.j2 template to use this value in this PR.

@jsuereth Done with the last commit.

[lmolkova]

@AlexanderWert I think you need to run make table-generation to re-generate registry MD files with display name.

[AlexanderWert]

@AlexanderWert I think you need to run make table-generation to re-generate registry MD files with display name.

That's not working yet, I'm getting a validation error from weaver. So, as @jsuereth mentioned above, we need to fix weaver validation first and release a new docker image, I guess.

@jsuereth What exactly needs to be done to fix the weaver validation? Is it just about adding display_name as a property in this struct?: https://github.com/open-telemetry/weaver/blob/261e4f42518fee67a00b1a81be4029a82fd19799/crates/weaver_semconv/src/group.rs#L21

[jsuereth]

@AlexanderWert Yes - I think that change (and propagation of its value) should be what you need to do in weaver.

There's also a TemplateGroup: https://github.com/open-telemetry/weaver/blob/main/crates/weaver_forge/src/registry.rs#L29

Once you have Group / TemplateGroup updated and wired - we can immediately start rendering this attribute in the registry.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.