Add a metric batcher which preallocates batch sizes. #194
Conversation
| if metric == nil || metric.MetricDescriptor == nil { | ||
| return nil, errNilMetricOrMetricDescriptor | ||
| mb.recordDroppedTimeseries(len(metric.GetTimeseries()), errNilMetricOrMetricDescriptor) |
There was a problem hiding this comment.
metric.GetTimeseries() may produce a nil pointer error.
There was a problem hiding this comment.
No:
func (m *Metric) GetTimeseries() []*TimeSeries {
if m != nil {
return m.Timeseries
}
return nil
}
metrics_batcher.go
Outdated
| // Send create time series requests to Stackdriver. | ||
| for _, req := range mb.allReqs { | ||
| if err := createTimeSeries(ctx, mc, req); err != nil { | ||
| mb.droppedTimeSeries += len(req.TimeSeries) |
There was a problem hiding this comment.
Use mb.recordDroppedTimeseries instead?
There was a problem hiding this comment.
Fixed the code now, I think this makes more sense.
There was a problem hiding this comment.
recordDroppedTimeseries will double count receivedTimeSeries, isn't it?
metrics_batcher.go
Outdated
| func (mb *metricsBatcher) addTimeSeries(ts *monitoringpb.TimeSeries) { | ||
| mb.receivedTimeSeries++ | ||
| mb.allTss = append(mb.allTss, ts) | ||
| if len(mb.allReqs) == maxTimeSeriesPerUpload { |
There was a problem hiding this comment.
should this be len(mb.allTss)?
metrics_batcher.go
Outdated
| // Send create time series requests to Stackdriver. | ||
| for _, req := range mb.allReqs { | ||
| if err := createTimeSeries(ctx, mc, req); err != nil { | ||
| mb.droppedTimeSeries += len(req.TimeSeries) |
There was a problem hiding this comment.
recordDroppedTimeseries will double count receivedTimeSeries, isn't it?
|
PTAL. Added a method that returns number of dropped timeseries. So we can use the helper for metrics recording in open-telemetry/opentelemetry-collector#315 |
|
LGTM |
No description provided.