Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/internal/telemetry: implement *wire.Metrics translator #33819

Closed
odeke-em opened this issue Aug 24, 2019 · 11 comments
Closed

x/tools/internal/telemetry: implement *wire.Metrics translator #33819

odeke-em opened this issue Aug 24, 2019 · 11 comments

Comments

@odeke-em
Copy link
Member

Opening this issue to remind myself to prioritize implementing the translator for metric to *wire.Metrics to enable serialization.

@gopherbot gopherbot added this to the Unreleased milestone Aug 24, 2019
@odeke-em odeke-em self-assigned this Aug 24, 2019
@odeke-em odeke-em added the Soon This needs to be done soon. (regressions, serious bugs, outages) label Aug 24, 2019
@agnivade
Copy link
Contributor

@odeke-em - Should this have the Soon label ? It is only reserved for serious bugs and outages.

@odeke-em
Copy link
Member Author

@odeke-em - Should this have the Soon label ? It is only reserved for serious bugs and outages.

Sorry about that, undone.

@odeke-em odeke-em removed the Soon This needs to be done soon. (regressions, serious bugs, outages) label Aug 25, 2019
@gopherbot
Copy link

Change https://golang.org/cl/199857 mentions this issue: ocagent: convert Int64Data and Float64Data metrics to *wire.Metric.

gopherbot pushed a commit to golang/tools that referenced this issue Oct 10, 2019
Histogram data still needs to be converted and
timestamps are not attached as they are not yet available.

What works:

* convertMetric will now convert Int64Data and Float64Data.

What does not work yet:

* Histogram64Int and Histogram64Float will still not be converted.
* StartTime and EndTime will not be attached to timeseries and points.
* MetricDescriptors will not have a unit attached.
* no labels will be attached to timeseries.

Updates golang/go#33819

Change-Id: I65f9af716ba6282e809d0a9d10777d70475e4c83
GitHub-Last-Rev: 10820a9
GitHub-Pull-Request: #170
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199857
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
@gopherbot
Copy link

Change https://golang.org/cl/200538 mentions this issue: internal/telemetry/export/ocagent: convert Histogram metrics

gopherbot pushed a commit to golang/tools that referenced this issue Oct 17, 2019
This change adds support for converting HistogramFloat64Data and
HistogramInt64Data to *wire.Metric. Timestamps are not attached
as they are not yet available.

What works:
* convertMetric will now convert HistogramInt64Data and
HistogramFloat64Data to *wire.Metric.

What does not work yet:
* StartTime and EndTime will not be attached to timeseries and
points.
* MetricDescriptors will not have a unit attached.
* No labels will be attached to timeseries.
* Distributions will not have SumOfSquaredDeviation attached.

Updates golang/go#33819

Change-Id: Iee52ab751542ee1ade07ef32120de853b41fd27b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200538
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
@gopherbot
Copy link

Change https://golang.org/cl/203060 mentions this issue: internal/telemetry/export/ocagent: attach timestamps to metrics

gopherbot pushed a commit to golang/tools that referenced this issue Oct 24, 2019
This change attaches start timestamps to timeseries and end
timestamps to the points in each timeseries. Int64Data,
Float64Data, HistogramInt64Data, and HistogramFloat64Data
have also had an EndTime field added to keep track of the last
time the metric was updated.

What works:
* Start and end timestamps will now be attached to timeseries.

What does not work yet:
* MetricDescriptors will not have a unit attached.
* No labels will be attached to timeseries.
* Distributions will not have SumOfSquaredDeviation attached.

Updates golang/go#33819

Change-Id: I692e1676bb1e31de26c1f799b96428fc9a55d6c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203060
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
@kirbyquerby
Copy link

@ianthehat @odeke-em Just as an update, I am currently working on functionally testing this with the OpenCensus agent.

@gopherbot
Copy link

Change https://golang.org/cl/207838 mentions this issue: internal/telemetry/export/ocagent: correctly marshal points to JSON

gopherbot pushed a commit to golang/tools that referenced this issue Nov 21, 2019
This change adds a custom MarshalJSON func to Point so that values
are formatted the same way jsonpb formats them. This allows the
OpenCensus service to determine the concrete type of the point's
value when unmarshaling.

What works:
* Points with Int64, Double, and Distribution values will marshal correctly.

What does not work:
* Points with Summary values will not marshal.

Updates golang/go#33819

Change-Id: Ia76ebff4e0e2b6ff2ddf72b8d6187f49069d4cad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207838
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@odeke-em
Copy link
Member Author

With all the CLs that have been submitted and most recently https://go-review.googlesource.com/c/tools/+/207838, this issue can now be closed.

@gopherbot
Copy link

Change https://golang.org/cl/208398 mentions this issue: internal/telemetry/export/ocagent: correctly marshal BucketOptionsExplicit to JSON

gopherbot pushed a commit to golang/tools that referenced this issue Nov 22, 2019
…nsExplicit

This change adds a custom MarshalJSON method to BucketOptionsExplicit for
marshalling parity with protobuf/jsonpb. This allows the OpenCensus service
to correctly decode a distribution's BucketOptions.

Updates golang/go#33819

Change-Id: Ia9dc868e1cbfc32a956f6a276dfd1591f7d4d31a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208398
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/208400 mentions this issue: internal/telemetry/export/ocagent: add tutorial for exporting metrics

@gopherbot
Copy link

Change https://golang.org/cl/208401 mentions this issue: internal/telemetry/export/ocagent: update usages of BucketOptions to use pointers

gopherbot pushed a commit to golang/tools that referenced this issue Nov 22, 2019
This change adds a README that describes how to export metrics from
Go tools to Prometheus via OpenCensus.

Updates golang/go#33819

Change-Id: Ic417b539fd5ef95fee642321bd30997146cbedb3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208400
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
gopherbot pushed a commit to golang/tools that referenced this issue Nov 22, 2019
…use pointers

This change updates initializations of DistributionValue to pass in a pointer for
BucketOptions rather than a value so that the custom MarshalJSON for
*BucketOptionsExplicit will be called when marshalling distributions.

Updates golang/go#33819

Change-Id: I6d1a393dfa67d626bf5cdd2cfbc0c931d376f60c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208401
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Nov 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants