[DRAFT] [mdatagen] Move attributes under each metric definition

[dmitryax]

As part of #16374, I'd like to consider removing attributes section in metadata.yaml and defining attributes for each metric separately. The main reason for that is aligning the config interface for re-aggregation (enabling/disabling metric attributes) with metrics attributes representation in metadata.yaml. It also removes the confusion around figuring out what the actual attribute name is: whether it's a key name in the attributes section or a value field in an attributes item.

On the other side, this move introduces a couple of complications:

1. If the same attribute is needed for different metrics, the same config section has to be copied or defined with YAML anchors as shown in this PR.
2. It complicates the enum attribute entities in the developer's API. In order to avoid long names for the enum attributes constants build from <metric_name>Attribute<attribute_name><enum>, this PR suggests a bit different approach based on interfaces that introduce a shorter form Attribute<enum>. But the suggested approach may be seen as an excessive complication, so maybe better to keep <metric_name>Attribute<attribute_name><enum> and have a constant for each enum in a metric attribute like metadata.SystemCPUUtilizationAttributeStateSystem

[dmitryax]

@djaglowski I would like to hear your opinion on this. If you agree on this approach, I'll move forward

[djaglowski]

I like the overall goal of #16374 but I'm not sure that reworking attributes like this is an improvement.

Is it technically necessary? My understanding is that it is not.

It also removes the confusion around figuring out what the actual attribute name is: whether it's a key name in the attributes section or a value field in an attributes item.

Solving this is the main goal right? If we remove confusion about the actual attribute name, then users can easily apply re-aggregation.

Personally, I think that relying on anchors is less readable, both for developers and users. I'd much rather we find a solution that allows us to define shared attributes once. Could we just clarify the current attributes section?

attributes:
  network_direction: # only used in metadata.yaml - no longer show in documentation
    attribute_name: direction # required, formerly "value"
    description: The direction of network data.
    type: string
    enum:
    - received
    - sent

We can also improve the documentation of metrics in a similar way to what you've proposed:

---

Metrics

elasticsearch.node.cluster.io

The number of bytes sent and received on the network for internal cluster communication.

Units	Data Type	Metric Type	Monotonic	Aggregation Temporality
By	Int	Sum	True	Cumulative

Attributes

Name	Description	Values
direction	The direction of network data.	received, sent

elasticsearch.other

The number of bytes sent and received on the network for something else.

Units	Data Type	Metric Type	Monotonic	Aggregation Temporality
By	Int	Sum	True	Cumulative

Attributes

Name	Description	Values
direction	The direction of network data.	received, sent
type	The type of thing.	foo, bar

[dmitryax]

Is it technically necessary? My understanding is that it is not.

No, it is not.

Could we just clarify the current attributes section?

I like your suggestion about making value->attribute_name field required, but we don't have that field in resource_attributes section which otherwise has the same set of fields and I don't think we should introduce it there. So keeping it as optional and relying on the key name makes it more consistent with the resource_attributes section. Maybe we can keep it optional but rename it to something like name_override or name?

Also, I was thinking about a pretty similar rendering of documentation.md :)

[djaglowski]

Maybe we can keep it optional but rename it to something like name_override or name?

Seems reasonable to me. It's a simple improvement upon what we currently have. Changing the docs to be unambiguous about the name is the more important part of addressing user confusion.

[dmitryax]

@djaglowski Sounds good. I've submitted the first PR to improve the generated documentation #16556 PTAL

Will do values->override_name filed rename after that.

Thanks for your feedback!

[dmitryax]

Submitted another PR to rename the field #16561 . I belive it'll be a much better shape after that, so we can close this PR