[chore] Add E2E test to assert internal metrics

[iblancasa]

Description

Add E2E test to ensure internal metrics remain with the same name after changes done to the collector or their dependencies.

Link to tracking issue

Relates #13456
Fixes #12918

[ArthurSens]

Wohoo, that is an awesome start! I wouldn't say it fixes #13456, though. What I had in mind for that issue is that we provide some kind of platform, where if a component becomes Stable, we'll automatically include it in our end-to-end tests.

Also, for a follow-up, metrics exposed by Prometheus may have different formats depending on content negotiation. Not sure if we want to assert only the scrape without negotiation or if we want to cover other kinds of content types.

[jade-guiton-dd]

Thanks for the PR! You'll want to fix the conflict so the CI can run.

[iblancasa]

Wohoo, that is an awesome start! I wouldn't say it fixes #13456, though. What I had in mind for that issue is that we provide some kind of platform, where if a component becomes Stable, we'll automatically include it in our end-to-end tests.

You are right. I modified the PR description to just refer to the ticket but not close it. As it was being discussed, very likely weaver is the solution for this, but we are not on that point yet. So, this ticket will help but not be the final

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.93%. Comparing base (8b73477) to head (e77380b).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13519      +/-   ##
==========================================
- Coverage   91.95%   91.93%   -0.03%     
==========================================
  Files         529      529              
  Lines       31446    31446              
==========================================
- Hits        28917    28909       -8     
- Misses       1998     2004       +6     
- Partials      531      533       +2     

☔ 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.

[ArthurSens]

Ok, I've tested this test manually and tried changing some metric names in metadata.yaml files just to verify it fails if the name changes. It does!

What I'm concerned now is that some metrics seem to be broken already, I see duplicated units like otelcol_exporter_sent_spans__spans__total (double span), and also duplicated underscores. This isn't what they were supposed to look 🤔

[codeboten]

Thanks @iblancasa! This is super exciting and something this repo has needed for some time

[ywwg]

Not sure if we want to assert only the scrape without negotiation or if we want to cover other kinds of content types.

I think we only need to test both NoTranslation and UnderscoreEscapingWithSuffixes. the other two should be fine if those two work

[ArthurSens]

Not sure if we want to assert only the scrape without negotiation or if we want to cover other kinds of content types.

I think we only need to test both NoTranslation and UnderscoreEscapingWithSuffixes. the other two should be fine if those two work

this configuration doesn't exist in Go SDK yet, so we need to implement that first

[songy23]

Please rebase or merge main, the metric names should no longer have duplicated units

[ArthurSens]

LGTM, thanks for working on this ❤️

[ArthurSens]

Alright, that looked weird, but looking at the metadata.yaml file, there really are two metrics with "same" name, just changing that the unit was added as suffix.

From my understanding, this goes against OTel's spec for metric names, but let's worry about that in the right time :P

[iblancasa]

@open-telemetry/collector-approvers could we have this merged?