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

Make it easier to manually report accumulated data #846

Merged
merged 1 commit into from
Jul 27, 2018

Conversation

knyar
Copy link
Contributor

@knyar knyar commented Jul 25, 2018

Report data for a given view when it is unregistered.

This might help with #773.

Copy link
Contributor

@semistrict semistrict left a comment

Choose a reason for hiding this comment

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

This new API surface (ReportUsage) probably needs to be discussed on opencensus-specs first so that other language implementations can also be aware of it. I would recommend that for now we split this PR into two: one that just adds the flush-on-unregister behavior and another with the new API.

@@ -129,6 +129,16 @@ func SetReportingPeriod(d time.Duration) {
<-req.c // don't return until the timer is set to the new duration.
}

// ReportUsage immediately reports collected data to all registered exporters
// outside of the regular reporting cycle.
func ReportUsage() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is new API surface area so probably needs to be discussed on opencensus-specs first so that other language implementations can also be aware of it. I would recommend that for now we split this PR into two: one that just adds the flush-on-unregister behavior and another with the new API.

m2 := stats.Int64("measure2", "desc", "unit")
view2 := &View{Name: "count2", Measure: m2, Aggregation: Count()}

SetReportingPeriod(5 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

For this test, I would recommend setting the reporting period to something high to emphasize that we don't want the usual reporting cycle to occur.

@knyar
Copy link
Contributor Author

knyar commented Jul 26, 2018

Thanks for the quick review! I've adjusted this PR to only flush when a view is unregistered.

I will also create a discussion issue similar to census-instrumentation/opencensus-specs#126 but focused on stats specifically.

@semistrict semistrict merged commit 78fb78a into census-instrumentation:master Jul 27, 2018
@semistrict
Copy link
Contributor

Thanks @knyar!

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

Successfully merging this pull request may close these issues.

None yet

2 participants