-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
In process propagation #20
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<?php | ||
|
||
namespace OpenTracing; | ||
|
||
/** | ||
* Keeps track of the current active `Span`. | ||
*/ | ||
interface ActiveSpanSource | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we shift the ActiveSpanSource interface, why not shift a ActiveSpanSource implementation? In my opinion, the ActiveSpanSource is only like a container. And only one implementation is enough. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I understand it this interface would be important to implement for event loops There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jcchavezs is that correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I simplified this piece in #45 |
||
{ | ||
/** | ||
* Activates an `Span`, so that it is used as a parent when creating new spans. | ||
* The implementation must keep track of the active spans sequence, so | ||
* that previous spans can be resumed after a deactivation. | ||
* | ||
* @param Span $span | ||
*/ | ||
public function activate(Span $span); | ||
|
||
/** | ||
* Returns current active `Span`. | ||
* | ||
* @return Span | ||
*/ | ||
public function getActiveSpan(); | ||
|
||
/** | ||
* Deactivate the given `Span`, restoring the previous active one. | ||
* | ||
* This method must take in consideration that a `Span` may be deactivated | ||
* when it's not really active. In that case, the current active stack | ||
* must not be changed (idempotency). | ||
* | ||
* Do not confuse it with the finish concept: | ||
* - $tracer->getActiveSpanSource()->deactivate($span) the span is not active but still "running" | ||
* - $span->finish() the span is finished and deactivated | ||
* | ||
* @param Span $span | ||
*/ | ||
public function deactivate(Span $span); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
|
||
namespace OpenTracing; | ||
|
||
final class NoopActiveSpanSource implements ActiveSpanSource | ||
{ | ||
public static function create() | ||
{ | ||
return new self(); | ||
} | ||
|
||
public function activate(Span $span) | ||
{ | ||
} | ||
|
||
public function getActiveSpan() | ||
{ | ||
return NoopSpan::create(); | ||
} | ||
|
||
public function deactivate(Span $span) | ||
{ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,28 @@ | |
interface Tracer | ||
{ | ||
/** | ||
* Starts and returns a new `Span` representing a unit of work. | ||
* | ||
* This method differs from `startManualSpan` because it uses in-process | ||
* context propagation to keep track of the current active `Span` (if | ||
* available). | ||
* | ||
* Starting a root `Span` with no casual references and a child `Span` | ||
* in a different function, is possible without passing the parent | ||
* reference around: | ||
* | ||
* function handleRequest(Request $request, $userId) | ||
* { | ||
* $rootSpan = $this->tracer->startActiveSpan('request.handler'); | ||
* $user = $this->repository->getUser($userId); | ||
* } | ||
* | ||
* function getUser($userId) | ||
* { | ||
* // `$childSpan` has `$rootSpan` as parent. | ||
* $childSpan = $this->tracer->startActiveSpan('db.query'); | ||
* } | ||
* | ||
* @param string $operationName | ||
* @param array|SpanOptions $options A set of optional parameters: | ||
* - Zero or more references to related SpanContexts, including a shorthand for ChildOf and | ||
|
@@ -25,7 +47,26 @@ interface Tracer | |
* @throws InvalidSpanOption for invalid option | ||
* @throws InvalidReferencesSet for invalid references set | ||
*/ | ||
public function startSpan($operationName, $options = []); | ||
public function startActiveSpan($operationName, $options = []); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reference type between the new created active span and the current active span? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ping @jcchavezs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When creating a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about other span relations than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @felixfbecker that is what |
||
|
||
/** | ||
* @param string $operationName | ||
* @param array|SpanOptions $options | ||
* @return Span | ||
* @throws InvalidSpanOption for invalid option | ||
* @throws InvalidReferencesSet for invalid references set | ||
*/ | ||
public function startManualSpan($operationName, $options = []); | ||
|
||
/** | ||
* @return ActiveSpanSource | ||
*/ | ||
public function getActiveSpanSource(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The introduction of ActiveSpanSource is not bad. Because, I implemented the same function in my company's php framework yesterday. However, the opentracing specification only defines the references. And how to trace the reference is all business of user. |
||
|
||
/** | ||
* @return Span | ||
*/ | ||
public function getActiveSpan(); | ||
|
||
/** | ||
* @param SpanContext $spanContext | ||
|
@@ -54,9 +95,8 @@ public function extract($format, Reader $carrier); | |
* | ||
* This method might not be needed depending on the tracing implementation | ||
* but one should make sure this method is called after the request is finished. | ||
* As an implementor, a good idea would be to use an asynchronous message bus | ||
* or use the call to fastcgi_finish_request in order to not to delay the end | ||
* of the request to the client. | ||
* As an implementor, a good idea would be to use register_shutdown_function | ||
* or fastcgi_finish_request in order to not to delay the end of the request to the client. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not an "or" imho, since you can use both. register_shutdown_function is still waiting to finish the communication with the Webserver if you don't use fastcgi_finish_request. |
||
* | ||
* @see register_shutdown_function() | ||
* @see fastcgi_finish_request() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You name it
startActiveSpan
here and then the example usesstartManualSpan
though. Is this right?