Skip to content

Commit

Permalink
Add comments need to be noted for future works (#32)
Browse files Browse the repository at this point in the history
* Add comments need to be noted for future works

* Update method name in comments
  • Loading branch information
ymotongpoo authored May 26, 2020
1 parent 2c699b1 commit ad51e85
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
5 changes: 5 additions & 0 deletions example/metric/example.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ func main() {
// prepare credential file following the instruction described in this doc.
// https://pkg.go.dev/golang.org/x/oauth2/google?tab=doc#FindDefaultCredentials
opts := []mexporter.Option{}

// NOTE: In current implementation of exporter, this resource is ignored because
// the function to handle the common resource just ignore the passed resource and
// it returned hard coded "global" resource.
// This should be fixed in #29.
resOpt := push.WithResource(
resource.New(
kv.String("instance_id", "abc123"),
Expand Down
17 changes: 17 additions & 0 deletions exporter/metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,23 @@ func recordToTypedValueAndTimestamp(r *export.Record) (*monitoringpb.TypedValue,

// TODO: Ignoring the case for Min, Max and Distribution to simply
// the first implementation.
//
// Currently the selector used in the integrator is `simple.NewWithExactDistribution`
// which should return array.New(), where it is ambiguous how the aggregator is treated inside.
// https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric/aggregator/array?tab=doc#New
//
// Now this function only returns count for Counter and Observer as it is set as 1st condition,
// and float64 type obersever is trimmed to int64, unfortunately.
// If we'd like to handle all possible aggregations, we need to have intermediate result type,
// and return the specified aggrataion value, which is done in stdout exporter.
// https://github.com/open-telemetry/opentelemetry-go/blob/21d094af43/exporters/metric/stdout/stdout.go#L72-L84
// https://github.com/open-telemetry/opentelemetry-go/blob/21d094af43/exporters/metric/stdout/stdout.go#L167-L221
//
// Views API should provide better interface that does not require the complicated codition handling
// done in this function.
// https://github.com/open-telemetry/opentelemetry-specification/issues/466
// In OpenCensus, view interface provided the bundle of name, measure, labels and aggregation in one place,
// and it should return the appropriate value based on the aggregation type specified there.
if count, ok := agg.(aggregator.Count); ok {
return countToTypeValueAndTimestamp(count, kind, now)
} else if lv, ok := agg.(aggregator.LastValue); ok {
Expand Down

0 comments on commit ad51e85

Please sign in to comment.