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

Allow custom view.Meters to export metrics for other Resources #1212

Merged

Conversation

evankanderson
Copy link
Contributor

Working with #1196, I discovered that metricexport.Reader collects metrics from all registered Producers, but doesn't provide a way to distinguish between different Meters. (With RegisterExporter, I can hand off a different exporter for each Meter and curry the Resource that way.)

This fills in the existing metricdata.Resource field when available, which looks like it should already work with Stackdriver. Prometheus seems to ignore the Resource field; I can send a PR for that if desired.

ian-mi and others added 2 commits June 4, 2020 08:24
…ensus-instrumentation#1210)

Time is already recorded on the client side and stored in the currently unused recordReq.t
field. Avoiding these repeated calls to time.Now while the worker is blocked can significantly
reduce worker contention.
@evankanderson evankanderson requested review from rakyll, rghetia and a team as code owners June 4, 2020 17:15
@jjzeng-seattle
Copy link

We need a minor fix for stackdriver though. resource is overwritten by ResourceByDescriptor. https://github.com/census-ecosystem/opencensus-go-exporter-stackdriver/blob/master/metrics.go#L173

@evankanderson
Copy link
Contributor Author

I'm wondering if we can leverage resource.MultiDetector for this:

https://github.com/census-instrumentation/opencensus-go/blob/master/resource/resource.go#L146

@evankanderson
Copy link
Contributor Author

Alternatively, we could do the conversion on the recording side for Stackdriver in Knative, and retire the use of ResourceByDescriptor from Stackdriver.

@@ -619,3 +641,20 @@ func restart() {
defaultWorker = NewMeter().(*worker)
go defaultWorker.start()
}

// byTag implements sort.Interface for *metricdata.TimeSeries by Labels.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/byTag/byLabel/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, done.

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.

None yet

5 participants