At the moment, every trace (a set of spans) from a request will end up being one http request to the reporter. This is not ideal but limitation in the language as PHP is, in many real-world contexts, single-threaded and does not have an event loop like Node.js so we must flush when the request finishes. This is something convenient at small scale but not much at a big scale. As the outcome of this issue we should come up with a solution to avoid the overhead on the immediate reporter vs. bulk reporter.

Ping @adriancole @basvanbeek @beberlei @felixfbecker

Hi,

I've noticed that there are 2 PR's already on this topic (#200 and #198 ), so I thought it might be ok to also have an issue about the challenges around this. Also being selfish and all I was hit by the incompatibility with PHP 8 as we're going through a migration process with a couple of projects that also use zipkin.

Given that both PHP 7.1 & PHP 7.2 are past their EOL dates, is there a support target/plan with respect to backwards compatibility of various PHP versions?

Thanks

This PR adds memcached reporter. So basically it is similar to InMemory but it stores the value in a memcached server. It also should work as expected in case of concurrent reads and writes. it uses this feature from memcached https://www.php.net/manual/en/memcached.cas.php. This requires php-memcached extension, so it will be a breaking change.

Usage

In application, we use Memcached reporter not HTTP reporter.

<?php
// app.php
include_once dirname(__FILE__) . "/vendor/autoload.php";

use Zipkin\Annotation;
use Zipkin\Endpoint;
use Zipkin\Samplers\BinarySampler;
use Zipkin\TracingBuilder;
use Zipkin\Reporters\Aggregation\MemcachedClient;
use Zipkin\Reporters\Memcached;

$endpoint = Endpoint::create('clivern');

$reporter = new Memcached([], new MemcachedClient('127.0.0.1'));

$sampler = BinarySampler::createAsAlwaysSample();
$tracing = TracingBuilder::create()
    ->havingLocalEndpoint($endpoint)
    ->havingSampler($sampler)
    ->havingReporter($reporter)
    ->build();

$tracer = $tracing->getTracer();

for ($i=1; $i <= 10000; $i++) {
  $span = $tracer->newTrace();
  $span->setName(sprintf("span_%d", $i));
  $span->start();

  try {
    // something quite fast
    var_dump("test");
  } finally {
    $span->finish();
  }

  $tracer->flush();
}

Then we create a cron job or a command to read from Memcached reporter and send to Zipkin using HTTP reporter. We may even add this as application middleware with a time interval.

<?php
// daemon.php
include_once dirname(__FILE__) . "/vendor/autoload.php";

use Zipkin\Reporters\Aggregation\MemcachedClient;
use Zipkin\Reporters\Memcached;
use Zipkin\Reporters\Http;

$reporter = new Memcached([], new MemcachedClient('127.0.0.1'));
$httpReporter = new Http(['endpoint_url' => 'http://zipkin.local:9411/api/v2/spans']);

while (true) {
    $spans = $reporter->flush();

    if (!empty($spans)){
        $httpReporter->report($spans);
    }

    sleep(5);
}

Flushing can be based on a time interval or the stored value size (Memcached support value up to 1MB by default that's ~1000000 chars). i didn't add any implementation since it is totally up to the developer and the scale zipkin is used. reporting 2 spans/second will have a different implementation than 1000 spans/second and I think this shouldn't be part of the package. in case you will hit the 1MB size limit in a second, you can either increase memcached maximum object size or report with different memcached keys, so basically you split the collected spans across different memcached keys to get more space before flushing.

Please let me know if something need to be adjusted!

Fix #22

Update:

To run as single process

<?php

include_once dirname(__FILE__) . "/vendor/autoload.php";

use Zipkin\Annotation;
use Zipkin\Endpoint;
use Zipkin\Samplers\BinarySampler;
use Zipkin\TracingBuilder;
use Zipkin\Reporters\Aggregation\MemcachedClient;
use Zipkin\Reporters\Memcached;
use Zipkin\Reporters\Http;


$endpoint = Endpoint::create('orders_service');

$httpReporter = new Http([
  'endpoint_url' => 'http://zipkin.local:9411/api/v2/spans'
]);

$reporter = new Memcached(
    [],
    $httpReporter,
    new MemcachedClient('127.0.0.1')
);

$sampler = BinarySampler::createAsAlwaysSample();
$tracing = TracingBuilder::create()
    ->havingLocalEndpoint($endpoint)
    ->havingSampler($sampler)
    ->havingReporter($reporter)
    ->build();

$tracer = $tracing->getTracer();

for ($i=1; $i <= 400; $i++) {
  $span = $tracer->newTrace();
  $span->setName(sprintf("span_%d", $i));
  $span->start();

  try {
    var_dump("report");
  } finally {
    $span->finish();
  }

  $tracer->flush();
}

This will let me implement "firehose mode", similar to what we have in py_zipkin.

Basically what I need is a way to call 2 different Reporters:

• one gets called only when sampled is True
• one gets called every time (this is the firehose recorder) This cannot be accomplished with just one Reporter, because once we're inside the reporter we've lost any info on whether the trace was sampled or not.

I can almost do this in zipkin-php, there are only 2 things missing.

• if sampled is False, we create a NoopSpan. So I need an extra flag to inform the tracer that I always want to create a RealSpan
• the Recorder is hardcoded inside the Tracer class, so I cannot pass my own.

Once those 2 things are supported, I can write my own Recorder subclass that calls the right Reporters as needed with minimal changes to the core zipkin-php code.

cc @adriancole

This PR

• [x] normalizes composer.json

💁‍♂️ I ran

$ composer global require localheinz/composer-normalize

followed by

$ composer normalize

For reference, see https://github.com/localheinz/composer-normalize.

Proposal for issue #202

• there was a bit of a snowball there with respect to aligning dependencies to work with minor PHP versions (I did not try too hard to find a way to keep 7.2 as per issue comments).
• See https://github.com/php-http/discovery/issues/164 - guzzlehttp doesn't provide expected PSR-17 factories (there's code available ready for their v2, but that would mean dropping some stability constraints) => switched to nyholm (it claims some performance boost as well, though probably not too relevant since we're talking tests)
• some tweaks around the tests: eg dechex() needs an int, but hexdec() returns int|float, running afoul of strong type check.
• updated phpunit also due to version constraints (phpunit 8 was still complaining about running on php 8) which led to lots of warnings about prophecy having to use a trait for integration with phpunit (could ignore those I guess to aim for 7.2 compatibility), which requires phpspec/prophecy-phpunit which in turn requires min. 7.3
• rename class that did not conform to PSR-4

Sometimes you need to instrument operations that are being performed by legacy code. E.g. http call to a service but instead of using a http standard library like guzzle, it is being done with raw curl. Those times, the effort to actually replace the curl call and use a new shiny and instrumented library like guzzle is already a big effort and it is very error prone.

In those cases, having a Tracer::inSpan($options, $callable, $args) method would be helpful as that method is going to do tracing around that legacy call without forcing you to touch the logic (more than passing the right variables into the callable). Does that sound a reasonable use case?

Initial feedback from @adriancole:

I think there have been similar discussions around this.. sleuth has the ability to do this also py_zipkin. to a degree zipkin-js does (promise tracing)

Ping @adriancole @basvanbeek

Hello,

While trying to use zipkin-php in a docker-compose context, I found that traces sent to a zipkin server running inside compose network from a php script also running in compose would not be picked up, whereas the same script sent from local machine to the same zipkin (but running with exposed port 9411) would work ok.

I found that:

• as per zipkin-php-example code, simply creating the endpoint with $endpoint = Endpoint::create($endpointName, $ipv4, null, $port); doesn't have an impact on where the HttpReporter makes api calls (I would be expecting it calls a server found at $ipv4 ip)
• passing an options array specifying the url to the Http reporter constructor works (endpoint_url key)

How create_tracing method looks now:

$endpoint = Endpoint::create($endpointName, $ipv4, null, $port); $http = \Zipkin\Reporters\Http\CurlFactory::create(); $reporter = new Zipkin\Reporters\Http($http, [ 'endpoint_url' => sprintf('http://%s:%s/api/v2/spans', $ipv4, $port) ]); $sampler = BinarySampler::createAsAlwaysSample(); $tracing = TracingBuilder::create() ->havingLocalEndpoint($endpoint) ->havingSampler($sampler) ->havingReporter($reporter) ->build();

I'm not entirely sure whether my understanding is completely off (eg: is the $ipv4 where api calls should be made?) or whether this is a bug and what would be a way to handle it.

I believe at the very least the docs should reflect this.

Cheers

Currently it is hard to debug/understand when our spans are not accepted by the zipkin Server.

For instance my Zipkin client was sending a null value in a tag by doing $span->tag('curl.total_time', $info['total_time']); //total_time was null

Zipkin Server response was 400 but that was hidden from me because of the try/catch block here https://github.com/openzipkin/zipkin-php/blob/master/src/Zipkin/Reporters/Http.php#L63

I suggest adding Debug mode or Verbosity config to the client so that we know when things start failing for some reason.

Generally speaking, the Zipkin reporting call should be non-blocking and asynchronous. But since PHP almost doesn't have either of these properties, it's good to at least be sure that reporting of the Zipkin Trace is not causing a major delay in sampled traces.

Closes #94

This PR removes the dependency on the guzzle client using curl by default. The factory interface allows users to create their own clients tho.

Ping @beberlei @felixfbecker @cc5092

Closes #27

            $span->log([
                'event'=>'error',
                'stack'=> (string)$exception,
            ]);

event and stack valid

Hello everyone,

I have a php app which communicates with other microservices for its operations.

Can i use Zipkin to observe every message between microservices and their size?

Essentially, i need to have a clear picture for the affinities between the microservices of my architecture.

How can i do it?

If you don't mind, please move some of the logic inlined into ci.yml into build-bin, with a README and RELEASE.md like the other projects. Also, if you can, please change the GH actions file and job names so that they are basically the same (makes badges easier also).

Here's a couple examples:

• https://github.com/openzipkin/zipkin-ruby/pull/180
• https://github.com/openzipkin/zipkin-api

Motivation

We are standardizing build script and job names for easy reference when people switch from one project to another. It is important that the tasks done in CI do not heavily rely on CI specific tools as in the future we may get into trouble like this again. For example, in the past we had to switch from travis to circle back to travis. Using scripts we control as entrypoints gives us flexibility as CI providers change their policies around open source communities.

Currently reporter is not providing any feedback about the call to the zipkin server, if it fails it logs the error and don't fail and more so if it success, it won't notify the user about it.

This is alright in normal situations but can be a bit obscure in tooling, for example: https://github.com/jcchavezs/jaeger2zipkin/blob/master/Main.php#L53

One option is to add a debug log line on success. Any ideas @anuraaga @adriancole @basvanbeek ?

Initially Map was a Getter/Setter supposed to act as a usual map (i.e. set/get by key, see https://github.com/openzipkin/zipkin-php/blob/master/src/Zipkin/Propagation/Map.php), unfortunately I also promoted its use for HTTP headers (for things like Symfony) and that is how Map also supported case insensitiviness and array access.

Nowadays the lesson learnt is that each different Getter/Setter format deserve its own implementation rather than trying to reuse Map and hence to not add crap into existing abstractions. We can't change Map to be simpler now but we can create a very simple class for a truly simple map and use for the messaging abstraction (and deprecate Map), not sure how to call it tho:

• SimpleMap
• Array
• ?

Any idea @adriancole @anuraaga