[service] fix opencensus bridge in periodic readers #9361
Conversation
The periodic metric readers were not connecting to the opencensus bridge prior to this change, meaning anyone trying to emit metrics via the console or OTLP from components that used opencensus were failing to do so. Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <aboten@lightstep.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9361 +/- ##
==========================================
- Coverage 90.27% 90.24% -0.03%
==========================================
Files 343 343
Lines 18003 18005 +2
==========================================
- Hits 16252 16249 -3
- Misses 1424 1428 +4
- Partials 327 328 +1 ☔ View full report in Codecov by Sentry. |
|
I tested this manually to confirm this fixes the bug. Not sure the best way to test this in a unit test, i would need a destination where I can send periodic reader data configured, which i don't have in a test here... I figured this was a temporary piece of code anyways, until the remaining contrib components are migrated away from opencensus |
| @@ -71,7 +71,9 @@ func InitMetricReader(ctx context.Context, reader config.MetricReader, asyncErro | |||
| return initPullExporter(reader.Pull.Exporter, asyncErrorChannel) | |||
| } | |||
| if reader.Periodic != nil { | |||
| opts := []sdkmetric.PeriodicReaderOption{} | |||
| opts := []sdkmetric.PeriodicReaderOption{ | |||
| sdkmetric.WithProducer(opencensus.NewMetricProducer()), | |||
There was a problem hiding this comment.
I dont think I understand what WithProducer does, how was this doing anything before? Or has it been very broken without this?
There was a problem hiding this comment.
it has been very broken for periodic metric readers that cares about components that produce opencensus metrics.
The periodic metric readers were not connecting to the opencensus bridge prior to this change, meaning anyone trying to emit metrics via the console or OTLP from components that used opencensus were failing to do so.