-
Notifications
You must be signed in to change notification settings - Fork 2
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
Adds activeSpanSource #22
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 | ||
{ | ||
/** | ||
* 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: | ||
* - $span->deactivate() -> 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 |
---|---|---|
|
@@ -25,28 +25,61 @@ interface Tracer | |
const FORMAT_HTTP_HEADERS = 3; | ||
|
||
/** | ||
* @deprecated use either startActiveSpan or startManualSpan instead. | ||
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. Imo the initial official version shouldn't have anything deprecated. 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 problem is that we'd prefer the python one to be released before merging this one so either we wait or either we release the library without the active span record. 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 is the reason for that? |
||
* As implementor consider this as a backward compatibility alias for | ||
* startActiveSpan | ||
*/ | ||
public function startSpan($operationName, $options = []); | ||
|
||
/** | ||
* 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 SpanReference|null $parentReference | ||
* @param float|int|\DateTimeInterface|null $startTimestamp if passing float or int | ||
* it should represent the timestamp (including as many decimal places as you need) | ||
* @param array $tags | ||
* @param array|SpanOptions $options | ||
* @return Span | ||
* @throws InvalidSpanOption for invalid option | ||
*/ | ||
public function startSpan( | ||
$operationName, | ||
SpanReference $parentReference = null, | ||
$startTimestamp = null, | ||
array $tags = [] | ||
); | ||
public function startActiveSpan($operationName, $options = []); | ||
|
||
/** | ||
* Starts and returns a new Span representing a unit of work. | ||
* | ||
* @param string $operationName | ||
* @param array|SpanOptions $options | ||
* @return Span | ||
* @throws InvalidSpanOption for invalid option | ||
*/ | ||
public function startSpanWithOptions($operationName, $options); | ||
public function startManualSpan($operationName, $options = []); | ||
|
||
/** | ||
* @return ActiveSpanSource | ||
*/ | ||
public function 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. the coding style is inconsistent, some places use 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 tried to avoid all 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 you mentioned you consider get/set anti patterns, I'm curious why? It certainly is common in PHP libraries 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. Based on the changes on #30 I will change the methods here to use 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. yeah, totally agree with meaningful method names like |
||
|
||
/** | ||
* @return Span | ||
*/ | ||
public function activeSpan(); | ||
|
||
/** | ||
* @param SpanContext $spanContext | ||
|
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.
shouldn't
finish()
always be called in afinally
?