metrics: OpenCensus-Go stats+view to Metrics converter #32

odeke-em · 2018-11-26T00:53:17Z

This is the first step of piecemeal updates to allow
OpenCensus-Go to add metrics exporting before
https://github.com/census-instrumentation/opencensus-go#956
is resolved, as that issue is going to take a long time and
a lot of work.

After that, a follow-up change will be to then implement the
metrics service that will enable this exporter to
be an OpenCensus-Go stats+view exporter, then transform
that data into metricspb.Metric

Updates #31

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em · 2018-11-26T06:49:13Z

I've also uploaded the part 2 of it. Unfortunately Github can't base a PR off one that hasn't yet been merged otherwise it'd have been a great candidate for splitting up. Anyways please take a look at the two different commits.

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em · 2018-11-30T06:44:26Z

Kindly paging @bogdandrutu @songy23 @rakyll, this issue is blocking the OpenCensus-PHP stats implementation, so please help me review it, thanks!

bogdandrutu

This PR looks good on the high level. Some suggestions:

Can you split it into a smaller PR than can be reviewed easily?
In the same PR you include changes in the mod/sum - please move them in a separate PR.
Ask a person with a good golang skill to review the language specific things.

odeke-em · 2018-11-30T22:27:53Z

Thanks for the comments @bogdandrutu!

Can you split it into a smaller PR than can be reviewed easily?

Sure, I can do that

In the same PR you include changes in the mod/sum - please move them in a separate PR.

That go mod/sum change will have to be merged in before this otherwise CI won't build

Ask a person with a good golang skill to review the language specific things.

Yes, I requested @rakyll and @songy23 for reviews. There aren't too many people working on OpenCensus-Go unfortunately.

As requested in #32, splitting this up to prepare for the larger change. However this change needs to be merged in first before any Metrics related one.

odeke-em · 2018-11-30T22:34:10Z

@bogdandrutu I've mailed #33 for go.mod and go.sum changes.

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

Code and tests to convert from OpenCensus-Go view.Data to metricsproto.Metric. This will then allow us to hook ocagent-exporter into OpenCensus-Go consuming programs that export view.Data. Updates #31 Cut from #32

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em · 2018-12-09T00:20:48Z

Alright, since #32 has been merged into master and then rebased in this PR, what remains is just the basic uploading logic. PTAL @bogdandrutu @songy23

Addressed splitting up PRs, thank you.

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

songy23

LGTM overall. Consider filing issues for adding resource support since it's needed in multiple places.

songy23 · 2018-12-09T00:31:28Z

nodeinfo.go

+//  Pid from the current process
+//  StartTimestamp from the start time of this process
+//  Language and library information.
+func NodeWithStartTime(nodeName string) *commonpb.Node {


I would say I prefer the old method name :) Not only start_time is derived from env vars.

Gotcha. The reason why I changed it is because all the other variables can be retrieved alright from the environment but we don't expose startTime so it is the most important one.

odeke-em · 2018-12-09T00:43:09Z

Thank you @songy23 for the review!

LGTM overall. Consider filing issues for adding resource support since it's needed in multiple places.

In deed, I was actually just working on the resource inclusion but it has to come after this PR since we'll create a resource once when the exporter is started and then update each metric that's produced instead of creating a resource for every metric.

odeke-em · 2018-12-09T01:43:30Z

@songy23 I've mailed out PR #35 which adds resources to initial trace and metrics message and after that we'll be gucci!

odeke-em requested a review from songy23 November 26, 2018 00:53

googlebot added the cla: yes label Nov 26, 2018

odeke-em requested a review from bogdandrutu November 26, 2018 00:53

odeke-em added a commit that referenced this pull request Nov 26, 2018

metrics: the exporter can now convert from OpenCensus-Go stats to met…

2f146a1

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em mentioned this pull request Nov 26, 2018

cmd: add OpenCensus metrics receiver. census-instrumentation/opencensus-service#216

Closed

basvanbeek mentioned this pull request Nov 27, 2018

OpenCensus Stats implementation census-instrumentation/opencensus-php#220

Merged

odeke-em force-pushed the stats-to-metrics-converter branch from 2f146a1 to c5f1f46 Compare November 27, 2018 20:43

odeke-em added a commit that referenced this pull request Nov 27, 2018

metrics: the exporter can now convert from OpenCensus-Go stats to met…

c5f1f46

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em force-pushed the stats-to-metrics-converter branch from c5f1f46 to a8a9cff Compare November 27, 2018 20:53

odeke-em added a commit that referenced this pull request Nov 27, 2018

metrics: the exporter can now convert from OpenCensus-Go stats to met…

a8a9cff

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em requested a review from rakyll November 30, 2018 06:49

bogdandrutu previously requested changes Nov 30, 2018

View reviewed changes

odeke-em added a commit that referenced this pull request Nov 30, 2018

go.{mod, sum}: preparing for Metrics service

48c455c

As requested in #32, splitting this up to prepare for the larger change. However this change needs to be merged in first before any Metrics related one.

odeke-em mentioned this pull request Nov 30, 2018

Update opencensus core and proto libs in go.{mod, sum}. #33

Merged

odeke-em force-pushed the stats-to-metrics-converter branch from a8a9cff to 8cd2c5e Compare November 30, 2018 22:53

odeke-em added a commit that referenced this pull request Nov 30, 2018

metrics: the exporter can now convert from OpenCensus-Go stats to met…

8cd2c5e

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em mentioned this pull request Nov 30, 2018

Adapter to transform view.Data to metricsproto.Metric #34

Merged

odeke-em force-pushed the stats-to-metrics-converter branch from 8cd2c5e to 1213671 Compare December 9, 2018 00:19

odeke-em added a commit that referenced this pull request Dec 9, 2018

metrics: the exporter can now convert from OpenCensus-Go stats to met…

1213671

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

metrics: the exporter can now convert from OpenCensus-Go stats to met…

59ea6a2

…rics We now have a fully functioning view.Exporter --> metricspb.Exporter converter. Depends on #32 Updates #31

odeke-em force-pushed the stats-to-metrics-converter branch from 1213671 to 59ea6a2 Compare December 9, 2018 00:24

songy23 approved these changes Dec 9, 2018

View reviewed changes

odeke-em merged commit 59ea6a2 into master Dec 9, 2018

odeke-em deleted the stats-to-metrics-converter branch December 9, 2018 01:43

This was referenced Dec 9, 2018

metrics: implement an exporter #31

Closed

Is adding stats.Exporter into ocagent on the schedule? #28

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

metrics: OpenCensus-Go stats+view to Metrics converter #32

metrics: OpenCensus-Go stats+view to Metrics converter #32

odeke-em commented Nov 26, 2018

odeke-em commented Nov 26, 2018

odeke-em commented Nov 30, 2018

bogdandrutu left a comment

odeke-em commented Nov 30, 2018

odeke-em commented Nov 30, 2018

odeke-em commented Dec 9, 2018

songy23 left a comment

songy23 Dec 9, 2018

odeke-em Dec 9, 2018

odeke-em commented Dec 9, 2018

odeke-em commented Dec 9, 2018

metrics: OpenCensus-Go stats+view to Metrics converter #32

metrics: OpenCensus-Go stats+view to Metrics converter #32

Conversation

odeke-em commented Nov 26, 2018

odeke-em commented Nov 26, 2018

odeke-em commented Nov 30, 2018

bogdandrutu left a comment

Choose a reason for hiding this comment

odeke-em commented Nov 30, 2018

odeke-em commented Nov 30, 2018

odeke-em commented Dec 9, 2018

songy23 left a comment

Choose a reason for hiding this comment

songy23 Dec 9, 2018

Choose a reason for hiding this comment

odeke-em Dec 9, 2018

Choose a reason for hiding this comment

odeke-em commented Dec 9, 2018

odeke-em commented Dec 9, 2018