Extend Curl integration attributes

[mpskovvang]

I really appreciate how this project makes tracing applications so much easier but I had to extend the Curl integration to get even deeper insight.

public static function handleCurlResource($resource)
{
  return [
    'attributes' => [
      'uri' => curl_getinfo($resource, CURLINFO_EFFECTIVE_URL),
      'total_time' => curl_getinfo($resource, CURLINFO_TOTAL_TIME),
      'namelookup_time' => curl_getinfo($resource, CURLINFO_NAMELOOKUP_TIME),
      'connect_time' => curl_getinfo($resource, CURLINFO_CONNECT_TIME),
      'pretransfer_time' => curl_getinfo($resource, CURLINFO_PRETRANSFER_TIME),
      'starttransfer_time' => curl_getinfo($resource, CURLINFO_STARTTRANSFER_TIME),
      'size_upload' => curl_getinfo($resource, CURLINFO_SIZE_UPLOAD),
      'size_download' => curl_getinfo($resource, CURLINFO_SIZE_DOWNLOAD)
    ],
    'kind' => Span::KIND_CLIENT
  ];
}

https://github.com/census-instrumentation/opencensus-php/blob/master/src/Trace/Integrations/Curl.php

Even though it's easy to extend I just thought this could perhaps be configurable when calling Curl::load();

Thanks for a great project!

[chingor13]

Thanks for this! Would you like to include this in a PR and we can get it in?

[mpskovvang]

Sure, I'll submit a PR one of the following days.

[mpskovvang]

I have two ideas I would like your opinion on.

First option is to have a public method from there you can provide your custom attributes. Because we need the $resource variable we can only provide CURLINFO_ constants:

public static function setAttributes(array $attributes);

Curl::setAttributes([
    'uri' => CURLINFO_EFFECTIVE_URL,
    'total_time' => CURLINFO_TOTAL_TIME
]);

Second option is to provide a more generic solution there you pass a callback. This way you have access to the $resource variable in the Curl integration. Other integrations like PDO can give you access the $statement variable:

public static function setAttributesCallback(callable $callback);

Curl::setAttributesCallback(function ($resource) {
    return [
        'uri' => curl_getinfo($resource, CURLINFO_EFFECTIVE_URL),
        'total_time' => curl_getinfo($resource, CURLINFO_TOTAL_TIME)
    ];
});

The PDO integration comes with multiple handles so either there should be a callback method for each handle or a generic callback with first argument being the handle name.

The benefits from the second option is flexibility as the user can return different attributes for each trace method. He can even include some extra data from the global scope. I'm not sure if there be any negative impact from the callbacks. Each integration would require a default callback or at least a check for a user-provided callback existence.

[chingor13]

I would say that we should skip the customization for the integration. The entire handleCurlResource function is the customization and we are not forcing anyone to use the integration. This integration should be a simple drop-in version and if the developer wants to extend it further, they can do so with their own call to opencensus_trace_function('func name', $callback).

That said, I think we should add more common, useful attributes to this default integration.

Also, for size_upload and size_download, these should probably be added as message events with opencensus_trace_add_message_event for TYPE_SENT and TYPE_RECEIVED.

[mpskovvang]

That makes perfect sense.

I haven't been playing with the message events yet and I'm a little unsure on how to implement the opencensus_trace_add_message_event in the handleCurlResource method, however I'll be happy to submit a PR with just the timing metrics as attributes.