Skip to content
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

Update PSR-22 - Application Tracing #1301

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 97 additions & 8 deletions proposed/tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,103 @@ interfaces and functionality first.

[RFC 2119]: http://tools.ietf.org/html/rfc2119

## Goal
## 1. Definitions

TBD
* **Framework** - An application framework (or micro-framework) that runs a developers code.
Eg. Laravel, Symphony, CakePHP, Slim

Choose a reason for hiding this comment

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

Symphony
Symfony

* **Library** - Any library that developers may include that adds additional functionality.
Eg. Image Manipulation, HTTP Clients, 3rd Party SDKs
* **Provider** - An organization that offers APM (Application Performance Monitoring) as a service.
Typically, via a composer package

## Definitions
## 2. Interfaces

* **Framework** - An application framework (or micro-framework) that runs a developers code.
Eg. Laravel, Symfony, CakePHP, Slim
* **Library** - Any library that developers may include that adds additional functionality
Eg. Image Manipulation, HTTP Clients, 3rd Party SDK's
* **Provider** - An organization that offers APM as a service. Typically via a composer package
## 2.1 Span

See: [AAllport/psr-tracing - SpanInterface.php](https://github.com/AAllport/psr-tracing/blob/main/src/SpanInterface.php)

A span represents a single operation within a trace. A trace is a collection of spans.
A fully formed span will consist of the following data:

- Name
- Attributes, an associative array representing key-value pairs
- Start Time
- End Time
- Outcome
- Exceptions

Upon creation, a new span will not be activated or have any attributes set.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's good to keep mutability to a minimum on value objects.
How about?:

  • activate the span and start time as constructor time
  • have an ending method to fill in the outcome and eventual exceptions, end the time and deactivate the span.
    • I used something in a wrapper like end() and endWithError($exception) to accomplish this.

Choose a reason for hiding this comment

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

Activate and start are two different operations. In the majority of cases they are executed at the same time and immediately after the construction, but not always:
For example, you can just prepare an HTTP request via a client and construct the span but without firing it or processing it (no start nor activate should be called).
Then, when sending the request, you should call the start method, but you may not call activate because the request is in progress in background. Only when you need to wait and process the response you should call activate (which marks the span as the currently active span in the context).

The same applies for the outcome and exceptions: adding an exception does not mean that the outcome of the operation is not OK and does not imply the ending of the span; just like ending a span without adding any exception does not imply that the operation has been completed without errors.

Outcomes, exceptions and start/end times are completely unrelated concepts and, unfortunately, simplifying them leaves out some useful cases.

This allows the user to populate the span with data.
It is RECOMMENDED that users supply a span with all relevant attributes shortly after creation, notably before
activation.
However, Providers SHOULD make every effort to persist attribute data if provided after a span is activated.

To track the time a piece of work takes, a user will call `SpanInterface::activate()`.
This will set the start time and push the current span into whichever context propagation system the provider chooses.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to getCurrentSpan() from TracerInterface? After "activation" the tracer should return it?

Users MAY call `SpanInterface::start()` to populate the span with a start time, but not pushed into context propagation.
Copy link
Member

Choose a reason for hiding this comment

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

Why two methods? A SpanInterface::start($time = null) would do.

Choose a reason for hiding this comment

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

Neither start nor activate accept the $time parameter.
start sets the start time of the span, activate pushes the span into the context, marking it as the "currently active span" and sets the start time if not already set.

Once complete, the user would call SpanInterface::finish() to set the end-time and “pop” the span from the current
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Once complete, the user would call SpanInterface::finish() to set the end-time and “pop” the span from the current
Once complete, the user would call `SpanInterface::finish()` to set the end-time and “pop” the span from the current

stack.

Spans MUST support creating a child span via `SpanInterface::createChild(string $spanName): SpanInterface`.
This SHOULD use `TracerInterface::createSpan()`, then set the subsequent span's parent appropriately.

A typical use would look like the following:

```php
function imgResize($size=100) {
$span = $this->tracer->createSpan('image.resize')
->setAttribute('size',$size)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
->setAttribute('size',$size)
->setAttribute('size', $size)

->activate();

try{
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
try{
try {

//Resize the image
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//Resize the image
// Resize the image

return $resizedImage;

} catch (Exception $e) {
// Ideally, you would attach the exception to the span here
$span->setStatus(SpanInterface::STATUS_ERROR)
->addException($e);
} finally {
$span->finish();
}
}
```

This PSR does not dictate how a span’s internals should be represented, other than it MUST implement the SpanInterface.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This PSR does not dictate how a span’s internals should be represented, other than it MUST implement the SpanInterface.
This PSR does not dictate how a span’s internals should be represented, other than it MUST implement the `SpanInterface`.

However, it is RECOMMENDED to conform to the [W3C TraceContext specification](https://www.w3.org/TR/trace-context/).
Providers MUST implement the `SpanInterface::toTraceContextHeaders()` method.
At a minimum, this should return an empty array.
Most commonly, providers will return traceParent and baggage headers to pass on to child services.

Providers MUST add the following function signatures to allow data retrieval from their span:
Copy link
Member

Choose a reason for hiding this comment

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

It is better to provide an interface similar to what other PSRs do instead of defining a set of methods w/o it. See https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-7-http-message.md#3-interfaces

Copy link
Contributor Author

@AAllport AAllport May 15, 2023

Choose a reason for hiding this comment

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

Worth noting these methods do exist in the interface.
I will leave this comment open until I rephrase it slightly to clarify that the paragraph is drawing attention as opposed to an additional requirement.

E.g.

The following methods are aimed at retrieving data from a providers span for programmatic use

- `getAttribute(string $key): null|string|int|float|bool|Stringable;`
- `getAttributes(): iterable;`
- `getParent(): SpanInterface;`
- `getChildren(): array;`

The purpose of these functions is to allow data to be read from providers’ spans in a clear, uniform manner.
`SpanInterface::getParent` MAY return null if no parent is present or the span has been instantiated outside the relevant adapter’s methods.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`SpanInterface::getParent` MAY return null if no parent is present or the span has been instantiated outside the relevant adapter’s methods.
`SpanInterface::getParent()` MAY return null if no parent is present or the span has been instantiated outside the relevant adapter’s methods.

However, a reasonable attempt SHOULD be made to return a `SpanInterface` where possible.
Similarly, `SpanInterface::getChildren()` MAY return an empty array if child spans have been created outside the relevant adapter's methods.
For continual use of a secondary provider, it is RECOMMENDED to create a separate Tracer and use the `MultiTracer` described rather than relying on these methods.

## 2.2 Tracer

See: [AAllport/psr-tracing - TracerInterface.php](https://github.com/AAllport/psr-tracing/blob/main/src/TracerInterface.php)

SDKs are expected to provide a concrete Tracer via whichever methods are most appropriate for the frameworks they support (eg, PSR-11 container).
Libraries using this PSR SHOULD implement [TracerAwareTrait](#31-tracerawaretrait).
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess here we should mention TracerAwareInterface instead of the trait.
I believe that it might be better to avoid the AwareInterface pattern and have just a TracerManager class that can be global. My assumption is that providers are provided as a php extension or as a wrapper over that so exposing it as an global class might fit better. Or inject it in the container and obtain it where is needed.

Choose a reason for hiding this comment

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

I would personally avoid a TracerManager: DI containers can handle it well enough.

Maybe this can be rephrased as:
It is RECOMMENDED for the libraries using this PSR implement TracerAwareInterface in order to ease the injection of a tracer

or something similar


When providing a tracer, it is RECOMMENDED to check whether a different Tracer has already been provided.
This PSR provides a MultiTracer via the `psr/tracing-utils` package, which MAY be used in the event of a tracer already being provided.
It is out of the scope of this PSR to dictate whether a Tracer should replace the already provided tracer.

Providers should consider relocate the Tracer from the container to allow other providers to listen to any spans added to the Tracer (eg, if replaced with a MultiTracer).

A tracer implementation MUST provide a method for creating a [Span](#21-span) via the signature `createSpan(string $spanName): SpanInterface`.

## 3 Traits

## 3.1 TracerAwareTrait

See: [AAllport/psr-tracing - TracerAwareTrait.php](https://github.com/AAllport/psr-tracing/blob/main/src/TracerAwareTrait.php)