Skip to content
This repository was 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
rghetia merged 3 commits intocensus-instrumentation:masterfrom
evankanderson:read-resource-and-export
Jun 8, 2020
Merged

Allow custom view.Meters to export metrics for other Resources#1212
rghetia merged 3 commits intocensus-instrumentation:masterfrom
evankanderson:read-resource-and-export

Conversation

@evankanderson
Copy link
Copy Markdown
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 a team, rakyll and rghetia as code owners June 4, 2020 17:15
@jjzeng-seattle
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor Author

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

go defaultWorker.start()
}

// byTag implements sort.Interface for *metricdata.TimeSeries by Labels.
Copy link
Copy Markdown
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
Copy Markdown
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.

5 participants