OpenCensus Stats implementation #220
Conversation
- Userland Stats API for PHP 7.1+ - Userland DaemonClient for sending all PHP Stats (and Trace/Span export) over a Unix Socket - Daemon Service written in Go to consume PHP Stats directives and Span Exports - Relies on OC Agent functionality for exporting out stats and traces. It is possible to add regular OpenCensus-Go exporters to the Daemon however we desire to have the census-ecosystem/opencensus-go-exporter-ocagent to be finalized as all data should go through the OC Agent (thus needing as little configuration and no duplication of effort on supporting exporters)
There was a problem hiding this comment.
This is awesome @basvanbeek! I've done the first pass of the Go code and skimmed over the PHP one, please take a look at my comments.
…ency calculation, rate limit dropped message errors
@basvanbeek we've finished working on the ocagent-go-exporter so it can now upload stats and transform them into metrics. I plan on wrapping up at least one Metrics exporter by the end of this week. |
| ) | ||
| if remainder < 0 { | ||
| // truncate data as it holds more then we need | ||
| data = data[0 : msgLen-offset] |
There was a problem hiding this comment.
I think this operation could become a hotspot and we can improve it to have a constant behavior
whether remainder < 0 or not
i, max := 0, len(data)
if nr := msgLen - (offset + len(data)); nr < 0 {
// Truncate data indices to limit what we need.
max = msgLen-offset
}
// TODO: (@basvanbeek) investigate if this reslicing is needed.
// Re-slice RawPayload to fit additional data.
mh.RawPayload = mh.RawPayload[0: offset+max]
copy(mh.RawPayload[offset:], data[i:max])…llet for the heads up
| private final function send(string $type, string $msg = ''): bool | ||
| { | ||
| $start = microtime(true); | ||
| $maxEnd = $start + $this->maxSendTime; |
There was a problem hiding this comment.
If we exceed maxSendTime X times during a single request, maybe we should have a per-request circuit-breaker that skips send calls during the rest of the request?
I'm thinking of a case where the agent is hanging and we end up waiting maxSendTime for each measurement.
There was a problem hiding this comment.
Good idea. I'll add it. Also there will be an extension version of the daemon client which will allow stats recording to be handled outside the php request path.
Some background:
Typically a single call to send is returned very quickly either with success or an error (temporary failure if buffer is full) as we're using non blocking sockets. The reason the code is cycling here and setting a max send time is to set a limit to the amount of time allowed to transport large payloads needing multiple calls to send (ex. very elaborate tracing spans).
On Mac OS X the default (and unchangeable) size is 8192 bytes so it potentially needs to be called a couple of times. Most stats record operations are well below this limit and OpenCensus TagContext itself is limited to 8192 bytes (so a maxed out stats record call probably won't go beyond 2 calls to send).
There was a problem hiding this comment.
there will be an extension version of the daemon client which will allow stats recording to be handled outside the php request path.
That's terrific.
There was a problem hiding this comment.
@greggdonovan initial version of php extension powered daemonclient transport has been added to this PR.
…etween versions, see: php/php-src#2523
…g a call happen to the wordpress sidebar function
There was a problem hiding this comment.
Thank you for working on this @basvanbeek! Thank you @greggdonovan for the feedback too.
We've let it simmer for a while and I think with our discussions and tests this is good to go. I am making the approval to get the ball rolling as we haven't heard much back. We can keep updating as we go on.
Here is the initial work on OpenCensus Stats for PHP.