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

stats/view: implement Flush #922

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

odeke-em
Copy link
Member

Flush is a global function that immediately reports
all collected data by the worker, regardless of the
reporting duration or buffering.

Added tests but also ensured that calling (*worker).stop()
then Flush doesn't block and returns ASAP if the state
is "quit". The required protected w.quit from double close
but also using a select to detect cases of forever waiting
channel w.quit which was closed.

Fixes #862

Flush is a global function that immediately reports
all collected data by the worker, regardless of the
reporting duration or buffering.

Added tests but also ensured that calling (*worker).stop()
then Flush doesn't block and returns ASAP if the state
is "quit". The required protected w.quit from double close
but also using a select to detect cases of forever waiting
channel `w.quit` which was closed.

Fixes census-instrumentation#862
@odeke-em
Copy link
Member Author

/cc @lychung83

@odeke-em
Copy link
Member Author

The Travis CI test seems unrelated to this change, locally running

$ go test -run=TestEndToEnd_Single -count=100
PASS
ok  	go.opencensus.io/plugin/ocgrpc	0.153s

passes

@semistrict
Copy link
Contributor

@odeke-em if that is the case we should skip the test and file an issue

@semistrict
Copy link
Contributor

I think we need specs agreement before we can proceed here: census-instrumentation/opencensus-specs#152

@semistrict
Copy link
Contributor

semistrict commented Sep 21, 2018

In particular, I remember @bogdandrutu had reservations about providing an explicit Flush mechanism because of worries that it might be abused by users calling it inappropriately.

While in general I tend to err on the side of being enabling rather than directing, I proposed that we could provide a shutdown method instead of flush that could only be called once and so would not be open to the same abuse.

@semistrict
Copy link
Contributor

There is some good discussion on this PR: census-instrumentation/opencensus-specs#159

/cc @adriancole @bogdandrutu

@rakyll
Copy link
Contributor

rakyll commented Sep 21, 2018

Ah, I implemented before seeing this PR. Feel free to void it. #923

@bogdandrutu
Copy link
Contributor

@odeke-em I think (RetrieveData)[https://github.com/census-instrumentation/opencensus-go/blob/master/stats/view/worker.go#L94] is your answer for this. See how I used it https://github.com/census-instrumentation/opencensus-service/blob/master/observability/observabilitytest/observabilitytest.go#L105

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

Successfully merging this pull request may close these issues.

stats/view: add Flush to immediately report all collected data points regardless of buffering
4 participants