[chore][mdatagen] Add telemetry metric test

[crobert-1]

Description

As described in #10801, we're working on moving the collector's internal telemetry metric functionality to be generated by mdatagen, and handled by the generated code.

This is generally meant to be equivalent to the generated_metrics_test.go file which tests components' metrics in a unit test format. This is not meant to be an integration test that can confirm a component's telemetry metric matches whatever data it's consuming/processing/exporting. That work is being tracked in #10856.

Most (if not all) of the comments in telemetry_test.go will be removed once it's generated by mdatagen. The intention is also to simply append this added test to the existing generated_telemetry_test.go file rather than having its own file.

Link to tracking issue

Part of #10801. This is step 1 of the proposed break down of the issue.

Testing

This is a test only change. I assume adding the new public function GetMetric in component/componenttest/obsreporttest.godoes not require a changelog.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 91.48%. Comparing base (1295083) to head (d51eb5c).
Report is 143 commits behind head on main.

Files with missing lines	Patch %	Lines
component/componenttest/otelchecker.go	0.00%	12 Missing ⚠️
component/componenttest/obsreporttest.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11067      +/-   ##
==========================================
- Coverage   91.56%   91.48%   -0.09%     
==========================================
  Files         430      430              
  Lines       20242    20260      +18     
==========================================
  Hits        18535    18535              
- Misses       1331     1349      +18     
  Partials      376      376              

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

[dmitryax]

LGTM

[codeboten]

is it necessary to test these using componenttest or can they be tested using the code in generated_component_telemetry_test.go?

[codeboten]

See https://github.com/open-telemetry/opentelemetry-collector/blob/main/cmd/mdatagen/internal/samplereceiver/generated_component_telemetry_test.go as an example

[crobert-1]

It looks like it's possible to use that functionality, but there would be some level of impact.

A couple of things to call out:

1. The generated telemetry test from this PR would need to be moved up to each component's main directory, rather than internal/metadata. We could potentially append this testing to to the generated_component_telemetry_test.go file, but that may just make the logic there more confusing.
2. The current interface of passing in expected metrics would result in pretty verbose generated tests. We may need to add more helper functions that can aid in generating expected metrics, or just deal with verbose tests.

Question out of ignorance: Why do we need generated_component_telemetry_test.go for all of these different components? If it's always the same helper methods, why do we want to generate so much identical code rather than a central re-usable package?

Maybe another part of this is simply that we don't want to have to rely on componenttest for v1.0?

[codeboten]

Question out of ignorance: Why do we need generated_component_telemetry_test.go for all of these different components?

Totally valid question, I added it there as it would reduce the need for test package dependencies across component types. I didn't a processor to end up pulling in a dependency on receivers just to bring in a common test package. it would be possible to have these test separated per component type, but putting it in generated code seemed like a cheaper dependency.

[codeboten]

The generated telemetry test from this PR would need to be moved up to each component's main directory, rather than internal/metadata

Would it be possible to move the tests code in generated_component_telemetry_test down into the internal package instead?

The current interface of passing in expected metrics would result in pretty verbose generated tests.

I would expect that we will eventually move to a similar model that we use for scrapers for "golden" metric files (i.e. https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/mongodbatlasreceiver/testdata/alerts/golden/metric-threshold-closed.yaml)

[crobert-1]

I didn't a processor to end up pulling in a dependency on receivers just to bring in a common test package. it would be possible to have these test separated per component type, but putting it in generated code seemed like a cheaper dependency.

Was there a reason that this was needed in the first place though, rather than depending on componenttest? From my understanding it doesn't seem to have the issue of pulling in different component type dependencies. It has caller methods that specific component types wouldn't use (CheckExporterMetrics wouldn't be used by a receiver), but that doesn't seem to be the issue you're referencing here.

I realize this question is out of scope for these changes, just trying to understand context here.

Would it be possible to move the tests code in generated_component_telemetry_test down into the internal package instead?

Yes, I like that solution much better, just wasn't sure if it would be too big of a breaking change 👍

[codeboten]

Yes, I like that solution much better, just wasn't sure if it would be too big of a breaking change 👍

I don't think that's a huge change, would just require to make the funcs that are used by components public in the internal package, and update the references

[crobert-1]

I gave the move an attempt, but we need to make a decision around the tradeoffs. I've filed #11163 to discuss.

[codeboten]

Let's move this forward and figure out if there's optimization around how the code should be generated later

[codeboten]

@crobert-1 please rebase and we can get this merged

[crobert-1]

@crobert-1 please rebase and we can get this merged

Done. However, this PR was originally depending on functionality that has since been deleted in #11172. Please review the newly exposed methods in componenttest, feedback is welcome if you think there's a better way to expose the telemetry metrics. 👍

[crobert-1]

@codeboten I've updated this PR, it should be ready to merge if it still looks good to you.

[github-actions]

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

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.