Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Add a public method to convert OpenCensus View to MetricDescriptor. #277

Merged

Conversation

yegle
Copy link
Contributor

@yegle yegle commented Nov 10, 2020

This is useful if you want to use your code as source of truth of
MetricDescritors.

Managing MetricDescriptors outside of the Go code (e.g. in Terraform
configs) is painful, as the changes to MD requires delete/recreate,
after deleting all alert policies associated with the metric. On the
other hand, the externally managed MDs are bound to be out of sync with
what's present in the code.

In the end we decide to just extract/collect views from our go code as
source of truth, and thus we need to have a public method to convert
views to MD.

This is useful if you want to use your code as source of truth of
MetricDescritors.

Managing MetricDescriptors outside of the Go code (e.g. in Terraform
configs) is painful, as the changes to MD requires delete/recreate,
after deleting all alert policies associated with the metric. On the
other hand, the externally managed MDs are bound to be out of sync with
what's present in the code.

In the end we decide to just extract/collect views from our go code as
source of truth, and thus we need to have a public method to convert
views to MD.
@google-cla google-cla bot added the cla: yes label Nov 10, 2020
@yegle
Copy link
Contributor Author

yegle commented Nov 10, 2020

/cc @rghetia PTAL.

@yegle
Copy link
Contributor Author

yegle commented Nov 30, 2020

Friendly ping :-)

@dashpole dashpole self-assigned this Nov 30, 2020
@dashpole
Copy link
Contributor

Doesn't OpenCensus already manage the creation of metric descriptors for you?

func (e *statsExporter) createMetricDescriptorFromView(ctx context.Context, v *view.View) error {
Is there something it does/doesn't do that makes you want to manage them yourself?

@yegle
Copy link
Contributor Author

yegle commented Dec 2, 2020

Hmm I think I workarounded the issue by adding proper dependency in our Terraform config.

It used to be an issue when there's no dependency between the Cloud Run service (where the metric is produced) and the alert/dashboard.

I'll close this. Thank you for taking a look!

@yegle yegle closed this Dec 2, 2020
@yegle yegle reopened this Jan 19, 2021
@yegle yegle closed this Jan 19, 2021
@yegle yegle reopened this Jan 20, 2021
@yegle
Copy link
Contributor Author

yegle commented Jan 20, 2021

Hmm I think we still want to have this feature.

The library does take care of the metric creation, but only happen when at lease one call to stat.Record. If the metric is incremented in a condition statement and rarely reaches, the metric descriptor would never be created, and everything depending on the metric (e.g. monitoring dashboard, alerting policy) would have to wait for a very long time before they can be created.

Given this, we plan to use the new function added in this PR to generate a list of metric descriptors and then we can create them manually.

Would it make sense to merge this PR? Thank you!

@codecov-io
Copy link

Codecov Report

Merging #277 (9254941) into master (0fc2674) will decrease coverage by 0.77%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
- Coverage   72.72%   71.95%   -0.78%     
==========================================
  Files          17       17              
  Lines        1712     1558     -154     
==========================================
- Hits         1245     1121     -124     
+ Misses        391      363      -28     
+ Partials       76       74       -2     
Impacted Files Coverage Δ
stackdriver.go 39.32% <0.00%> (+1.23%) ⬆️
propagation/http.go 58.62% <0.00%> (-2.67%) ⬇️
sanitize.go 84.61% <0.00%> (-2.06%) ⬇️
metrics.go 76.56% <0.00%> (-1.99%) ⬇️
trace_proto.go 61.58% <0.00%> (-1.83%) ⬇️
stats.go 76.68% <0.00%> (-1.72%) ⬇️
trace.go 46.15% <0.00%> (-1.64%) ⬇️
monitoredresource/aws/monitored_resources.go 66.66% <0.00%> (-1.52%) ⬇️
metrics_proto.go 86.92% <0.00%> (-1.24%) ⬇️
label.go 100.00% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fc2674...9254941. Read the comment docs.

@dashpole dashpole merged commit 1996040 into census-ecosystem:master Jan 20, 2021
@yegle
Copy link
Contributor Author

yegle commented Jan 20, 2021

I appreciate your help!

@yegle yegle deleted the create-metric-descriptor-from-view branch January 20, 2021 17:10
@yegle
Copy link
Contributor Author

yegle commented Jan 26, 2021

Would it be possible to cut a new release including this PR? Thank you!

@dashpole
Copy link
Contributor

release has been cut

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants