[mdatagen] Re-Aggregate Metric by Attributes

[shalper2]

Reopens 13431

Description

Modified cmd/mdatagen to allow for two new fields in the configuration yaml of metrics. These new configuration options would allow for a user to enable or disable attributes (i.e. reduce dimensionality of metrics being generated) from their collector configuration. The modified MetricsBuilder generated by mdatagen automatically re-aggregates metrics based on the enabled set of attributes. Additionally, different aggregation strategies are supported and can be specified by the collector administrator.

The changes to the config.yaml are:

receiver:
  someMetricReceiver:
    ...
    metrics:
    ...
    attributes: # new field
      some.attribute.name:
         enabled: < true | false >
      aggregation_strategy: < sum | avg | min | max >

A new optional field has been added to metadata.yaml which will allow users to define default "enabled" behavior for attributes. For backwards compatibility any attribute defined in the metadata.yaml will have a default value of enabled: true.

...
attributes:
  some.attribute:
    description: example attributes are interesting
    type: string
    enabled: false # new field, optional

Link to tracking issue

Fixes 10726

Testing

Some testing has been done using the modified mdatagen with existing contrib receivers (i.e. hostmetricsreceiver). Generated tests have been updated to test new aggregation behavior.

Documentation

Updated metadata-schema.yaml to include new optional field.
ran make gogenerate
ran make generate

[codecov]

Codecov Report

❌ Patch coverage is 58.00000% with 357 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.99%. Comparing base (cb421ec) to head (4c964c7).
⚠️ Report is 84 commits behind head on main.

Files with missing lines	Patch %	Lines
...leconnector/internal/metadata/generated_metrics.go	54.14%	86 Missing and 8 partials ⚠️
...plereceiver/internal/metadata/generated_metrics.go	54.58%	86 Missing and 8 partials ⚠️
...mplescraper/internal/metadata/generated_metrics.go	54.14%	86 Missing and 8 partials ⚠️
cmd/mdatagen/internal/metadata.go	27.47%	62 Missing and 4 partials ⚠️
...pleconnector/internal/metadata/generated_config.go	92.68%	2 Missing and 1 partial ⚠️
...mplereceiver/internal/metadata/generated_config.go	93.61%	2 Missing and 1 partial ⚠️
...amplescraper/internal/metadata/generated_config.go	92.68%	2 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (58.00%) 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   #13900      +/-   ##
==========================================
- Coverage   91.65%   90.99%   -0.67%     
==========================================
  Files         652      652              
  Lines       42506    43217     +711     
==========================================
+ Hits        38960    39324     +364     
- Misses       2737     3054     +317     
- Partials      809      839      +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dmitryax]

looks like this is also applied to the resource attributes. The resource attributes with missing enabled fields now get enabled=true. That's why tests are failing.

I think we should make the enabled required and fill all missing spots in contrib to unblock this PR

[dmitryax]

While looking into the problem above, I found that telemetry attributes will be also affected. We expose all attributes in the user config including those that are applied to internal telemetry, but they don't have any aggregation capabilities at this moment... This is another blocker for merging the PR

I see couple of options here:

1. Move attributes that are applied to telemetry.metrics under the telemetry section in metadata.yaml. They won't have the enabled option and won't be exposed in the user config.
2. Move the attributes config interface under metrics.<metric_name> similar to what @rogercoll suggested here, but it has to be the same object with enabled and aggregation_strategy fields, not a list. This option seems reasonable, but it doesn't help with solving [mdatagen] Re-Aggregate Metric by Attributes #13900 (comment)

[dmitryax]

Ok, I think we should rework it closer to #13431 (comment) with the following changes:

• aggregation_strategy should be part of a metric config interface, not attributes.
• We can reflect metrics.<metric-name>.attributes as a list in the user config interface, but we need a way to specify attributes which are disabled by default for a metric. For that, I suggest we add a new field metrics.<metric-name>.disabled_attributes in metadata.yaml. Fo example, we will have:

metrics:
  system.cpu.time:
    attributes: [state]
    disabled_attributes: [cpu]

@shalper2 @rogercoll WDYT?

[shalper2]

I do like @rogercoll 's proposed configuration scheme so I'm not opposed to moving attributes under metrics at all. Is the idea with attributes and disabled_attributes fields that the total set of attributes present for the metric is the union of the two fields? So then in configuration if you omit the attributes field you get your default enabled attributes but if you specify you only enable the specified attributes?

[dmitryax]

Is the idea with attributes and disabled_attributes fields that the total set of attributes present for the metric is the union of the two fields?

Yes, that's the idea. That will map the values of attributes to the user interface exactly as is.

So then in configuration if you omit the attributes field you get your default enabled attributes but if you specify you only enable the specified attributes?

Yes, user will be able to pick any subset of the attributes | disabled_attributes union

[dmitryax]

Ok I started with a bit different approach. Instead of every metric having the disabled_attributes field, I suggest we have a new availability fields on the attribute #13913. Otherwise, everything is the same

[rogercoll]

We can reflect metrics..attributes as a list in the user config interface, but we need a way to specify attributes which are disabled by default for a metric

And extending the current optional metadata yaml field sounds like a good solution to me.

[github-actions]

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