ajamato commented on pull request #14804:
URL: https://github.com/apache/beam/pull/14804#issuecomment-840938445


   I see, thinking about this more. I realize that we will hit bug that 
OpenCensus had when they tried to share a startTime
   
   If you don't report a startTime near and prior to the reporting time of the 
first 
[timeseries.create](https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.timeSeries/create
   ) call, you can get weird issues in the way that stackdriver does 
precomputes, interpolation, etc.
   Without the fix, we had flakey tests that would not be able to read back out 
the metric they had just written (it would be 0 in the first interval).
   
   I actually had to fix this in their github in the past
   https://github.com/census-instrumentation/opencensus-cpp/pull/444 
   
   I think the best thing to do is to set the startTime for each 
MonitoringInfo, in the process_wide MetricContainer only. Then it can be added 
and stored in the ShortIdMap. This would require the process_wide 
MetricContainer to somehow know it's the first time it's encountered the 
MonitoringInfo. So I will make the process_wide MetricContainer reference the 
ShortIdMap and check if the MonitoringInfo exists first before generating the 
startTime.
   
   What do you think of this approach?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to