Test successful collection by PeriodicReader

[izquierdo]

Changes

Add a success scenario test for PeriodicReader as suggested in #1481 (comment)

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (49f1be6) 61.9% compared to head (4b8f83e) 62.8%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1521     +/-   ##
=======================================
+ Coverage   61.9%   62.8%   +0.9%     
=======================================
  Files        144     144             
  Lines      19862   19891     +29     
=======================================
+ Hits       12303   12502    +199     
+ Misses      7559    7389    -170     

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

[cijothomas]

not sure I follow the name of the test, as the trigger for collection is the time interval only...
The test is validating that, during collection, triggered by time interval, the observable's callback(s) are invoked.

[izquierdo]

The aim is to test that creating the SdkMeterProvider instance triggers the start of the worker that will periodically collect (and export) metrics. This is the line being tested:

        let meter_provider = SdkMeterProvider::builder().with_reader(reader).build();

Perhaps a better name would be registration_triggers_worker?

Using the observable callback is a roundabout way to confirm that the worker thread is running. Maybe better to use meter_provider.force_flush() and exporter.get_finished_metrics() instead, as in other tests? I also want to avoid test flakiness here.

[cijothomas]

calling force_flush() would anyway flush the metric, even without the worker starting?... I need check in detail, but this testing logic sounds good to me.

[cijothomas]

builder_triggers_periodicreader_worker - as alternative suggestion. Or you can just add a code comment with your comment, as that was good explanation.

[lalitb]

Using the observable callback is a roundabout way to confirm that the worker thread is running.

I believe this test looks good to validate the periodic exporter metric collection cycle through the mpsc channel. We don't need to validate the measurements and their aggregation, as there are other tests for the same.

[cijothomas]

Thanks. Left a suggestion about rename and/or leaving a code comment to make it clear what are we testing with this particular test, given it may not be very obvious.