stats worker as metric producer.#1078
Conversation
stats/view/view_to_metric.go
Outdated
|
|
||
| func rowToTimeseries(row *Row, now time.Time, startTime time.Time) *metricdata.TimeSeries { | ||
| return &metricdata.TimeSeries{ | ||
| Points: []metricdata.Point{row.Data.toPoint(now)}, |
There was a problem hiding this comment.
You need to convert metric types here. For example SumData is always converted to Float64 point: https://github.com/census-instrumentation/opencensus-go/pull/1078/files#diff-a282032fe900298505848fd4d08399a2R96. But when measure is of type Int64Measure, metric type will be CumulativeInt64: https://github.com/census-instrumentation/opencensus-go/pull/1078/files#diff-0ac84b9d221fc10ebe99482beb04a4fbR45. This will be an error since the type of point doesn't match with metric type.
There was a problem hiding this comment.
fixed.
comparing serialized version made the test pass incorrectly. Removed the serialized comparision.
… type. fixed test. Specifically removed json comparision.
|
@songy23 PTAL. |
stats/view/aggregation_data.go
Outdated
| case metricdata.TypeCumulativeInt64: | ||
| return metricdata.NewInt64Point(t, a.Value) | ||
| case metricdata.TypeCumulativeFloat64: | ||
| return metricdata.NewFloat64Point(t, float64(a.Value)) |
There was a problem hiding this comment.
Count should always be Int64.
| case "By": | ||
| return metricdata.UnitBytes | ||
| } | ||
| return metricdata.UnitDimensionless |
There was a problem hiding this comment.
Why convert all other units to "1"?
There was a problem hiding this comment.
I'll open a new PR to fix this.
|
@songy23 PTAL |
stats/view/aggregation_data.go
Outdated
| } | ||
|
|
||
| func (a *DistributionData) toPoint(metricType metricdata.Type, t time.Time) metricdata.Point { | ||
| buckets := []metricdata.Bucket{} |
There was a problem hiding this comment.
nit: you can also check metricType here (expect to be cumulative distribution).
|
|
||
| gotMetric := viewToMetric(tc.vi, now, startTime) | ||
| if !reflect.DeepEqual(gotMetric, tc.wantMetric) { | ||
| // JSON format is strictly for checking the content when test fails. Do not use JSON |
There was a problem hiding this comment.
Consider using https://github.com/google/go-cmp.
Also replaced reflect.DeepEqual with cmp.Equal
* stats worker as metric producer. * fixed the conversion based on measure type in addition to aggregation type. fixed test. Specifically removed json comparision. * fixed review comments related to count float64. * add check for metricType in toPoint func for distribution Also replaced reflect.DeepEqual with cmp.Equal (cherry picked from commit ec71c97)
This change provides stats data (view data model) as metric data. Stats worker registers as a metric producer with global producer manager. Read() function is then invoked by Readers.