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

Implement exporterfactory #4

Merged
merged 11 commits into from
Jun 9, 2021
Merged
8 changes: 5 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
"grpc/grpc": "^1.30",
"google/protobuf": "^v3.3.0",
"psr/http-client-implementation": "^1.0",
"psr/http-factory-implementation": "^1.0"
},
"psr/http-factory-implementation": "^1.0",
"nyholm/dsn": "^2.0.0"
},
"authors": [
{
"name": "Bob Strecansky",
Expand Down Expand Up @@ -59,6 +60,7 @@
"guzzlehttp/guzzle": "^7.3",
"guzzlehttp/psr7": "^2.0@RC",
"symfony/http-client": "^5.2",
"nyholm/psr7": "^1.4"
"nyholm/psr7": "^1.4",
"nyholm/dsn": "^2.0.0"
}
}
166 changes: 166 additions & 0 deletions sdk/Trace/ExporterFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Sdk\Trace;

use GuzzleHttp\Client;
use GuzzleHttp\Psr7\HttpFactory;
use Nyholm\Dsn\DsnParser;
use OpenTelemetry\Contrib\Jaeger\Exporter as JaegerExporter;
use OpenTelemetry\Contrib\Newrelic\Exporter as NewrelicExporter;
use OpenTelemetry\Contrib\Otlp\Exporter as OtlpExporter;
use OpenTelemetry\Contrib\OtlpGrpc\Exporter as OtlpGrpcExporter;
use OpenTelemetry\Contrib\Zipkin\Exporter as ZipkinExporter;
use OpenTelemetry\Contrib\ZipkinToNewrelic\Exporter as ZipkinToNewrelicExporter;

class ExporterFactory
{
private $name;
Aneurysm9 marked this conversation as resolved.
Show resolved Hide resolved
private $allowedExporters = ['jaeger' => true, 'zipkin' => true, 'newrelic' => true, 'otlp' => true, 'otlpgrpc' => true, 'zipkintonewrelic' => true];

public function __construct(string $name)
{
$this->name = $name;
}

/**
* Returns the coresponding Exporter via the configuration string
*
* @param string $configurationString String containing unextracted information for Exporter creation
* Should follow the format: contribType+baseUrl?option1=a
* Query string is optional and based on the Exporter
*/
public function fromConnectionString(string $configurationString)
{
$strArr = explode('+', $configurationString);
// checks if input is given with the format type+baseUrl
if (sizeof($strArr) != 2) {
return null;
}

$contribName = strtolower($strArr[0]);
$endpointUrl = $strArr[1];

if (!$this->isAllowed($contribName)) {
return null;
}

$args = [];
// endpoint is only parsed if it is provided
$dsn = empty($endpointUrl) ? '' : DsnParser::parse($endpointUrl);
$endpointUrl = $this->parseBaseUrl($dsn);
// parameters are only retrieved if there was an endpoint given
$args = empty($dsn) ? '' : $dsn->getParameters();
alexperez52 marked this conversation as resolved.
Show resolved Hide resolved

switch ($contribName) {
case 'jaeger':
return $exporter = $this->generateJaeger($endpointUrl);
case 'zipkin':
return $exporter = $this->generateZipkin($endpointUrl);
case 'newrelic':
return $exporter = $this->generateNewrelic($endpointUrl, $args['licenseKey'] ?? null);
case 'otlp':
return $exporter = $this->_generateOtlp();
case 'otlpgrpc':
return $exporter = $this->generateOtlpGrpc();
case 'zipkintonewrelic':
return $exporter = $this->generateZipkinToNewrelic($endpointUrl, $args['licenseKey'] ?? null);
}
}

private function isAllowed(string $exporter)
{
return array_key_exists($exporter, $this->allowedExporters) && $this->allowedExporters[$exporter];
}

// constructs the baseUrl from
Copy link

Choose a reason for hiding this comment

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

(also another nit, but you may want to finish this comment)

private function parseBaseUrl($dsn)
{
if ($dsn == false) {
return null;
}
$parsedUrl = '';
$parsedUrl .= empty($dsn->getScheme()) ? '' : $dsn->getScheme() . '://';
$parsedUrl .= empty($dsn->getHost()) ? '' : $dsn->getHost();
$parsedUrl .= empty($dsn->getPort()) ? '' : ':' . $dsn->getPort();
$parsedUrl .= empty($dsn->getPath()) ? '' : $dsn->getPath();

return $parsedUrl;
}

private function generateJaeger(string $endpointUrl)
{
$exporter = new JaegerExporter(
$this->name,
$endpointUrl,
new Client(),
new HttpFactory(),
new HttpFactory()
);

return $exporter;
}
private function generateZipkin(string $endpointUrl)
{
$exporter = new ZipkinExporter(
$this->name,
$endpointUrl,
new Client(),
new HttpFactory(),
new HttpFactory()
);

return $exporter;
}
private function generateNewrelic(string $endpointUrl, $licenseKey)
{
if ($licenseKey == false) {
return null;
}
$exporter = new NewrelicExporter(
$this->name,
$endpointUrl,
$licenseKey,
new Client(),
new HttpFactory(),
new HttpFactory()
);

return $exporter;
}

private function _generateOtlp()
Copy link

Choose a reason for hiding this comment

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

might this underscore have been forgotten to be removed?

Copy link
Author

Choose a reason for hiding this comment

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

yes. thank you good catch

{
$exporter = new OtlpExporter(
$this->name,
new Client(),
new HttpFactory(),
new HttpFactory()
);

return $exporter;
}

private function generateOtlpGrpc()
{
return new OtlpGrpcExporter();
}

private function generateZipkinToNewrelic(string $endpointUrl, $licenseKey)
{
if ($licenseKey == false) {
return null;
}
$exporter = new ZipkinToNewrelicExporter(
$this->name,
$endpointUrl,
$licenseKey,
new Client(),
new HttpFactory(),
new HttpFactory()
);

return $exporter;
}
}
2 changes: 1 addition & 1 deletion sdk/Trace/Sampler.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
interface Sampler
{
/**
* Returns SamplingResult.
* Returns the sampling Decision that will be used in Span creation.
*
* @param Context $parentContext Context with parent Span. The Span's SpanContext may be invalid to indicate a root span.
* @param string $traceId TraceId of the Span to be created. It can be different from the TraceId in the SpanContext.
Expand Down
90 changes: 90 additions & 0 deletions tests/Sdk/Unit/Trace/ExporterFactoryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Tests\Sdk\Unit\Trace;

use OpenTelemetry\Contrib as Path;
use OpenTelemetry\Sdk\Trace\ExporterFactory as ExporterFactory;
use PHPUnit\Framework\TestCase;

class ExporterFactoryTest extends TestCase
{
/**
* @test
*/
public function testIfExporterHasCorrectEndpoint()

Choose a reason for hiding this comment

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

Be sure to add a test for invalid input as well. Perhaps even malicious input if you can construct one.

Copy link

Choose a reason for hiding this comment

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

When writing you tests, make sure to use inputs that are malicious like certain html entities. There are examples out there that give examples of malicious attacks (especially with sql injections and other such attacks) although I don't know how relevant they will be to an exporter used for tracing.

{
$input = 'zipkin+http://zipkin:9411/api/v2/spans';
$factory = new ExporterFactory('test.zipkin');
$exporter = $factory->fromConnectionString($input);
$this->assertInstanceOf(Path\Zipkin\Exporter::class, $exporter);

$input = 'jaeger+http://jaeger:9412/api/v2/spans';
$factory = new ExporterFactory('test.jaeger');
$exporter = $factory->fromConnectionString($input);
$this->assertInstanceOf(Path\Jaeger\Exporter::class, $exporter);

$input = 'newrelic+https://trace-api.newrelic.com/trace/v1?licenseKey="23423423';
$factory = new ExporterFactory('test.newrelic');
$exporter = $factory->fromConnectionString($input);
$this->assertInstanceOf(Path\Newrelic\Exporter::class, $exporter);

$input = 'otlp+';
$factory = new ExporterFactory('test.otlp');
$exporter = $factory->fromConnectionString($input);
$this->assertInstanceOf(Path\Otlp\Exporter::class, $exporter);

$input = 'otlpgrpc+';
$factory = new ExporterFactory('test.otlpgrpc');
$exporter = $factory->fromConnectionString($input);
$this->assertInstanceOf(Path\OtlpGrpc\Exporter::class, $exporter);

$input = 'zipkintonewrelic+https://trace-api.newrelic.com/trace/v1?licenseKey="23423423';
$factory = new ExporterFactory('test.zipkintonewrelic');
$exporter = $factory->fromConnectionString($input);
$this->assertInstanceOf(Path\ZipkinToNewrelic\Exporter::class, $exporter);
}

/**
* @test
*/
public function testInvalidInput()
{
$input = 'zipkinhttp://zipkin:9411/api/v2/spans';
$factory = new ExporterFactory('test.zipkin');
$exporter = $factory->fromConnectionString($input);
$this->assertNull($exporter);

$input = 'zipkin+http://zipkin:9411/api/v2/spans+extraField';
$factory = new ExporterFactory('test.zipkin');
$exporter = $factory->fromConnectionString($input);
$this->assertNull($exporter);

$input = 'zapkin+http://zipkin:9411/api/v2/spans';
$factory = new ExporterFactory('test.zipkin');
$exporter = $factory->fromConnectionString($input);
$this->assertNull($exporter);

$input = 'otlp';
$factory = new ExporterFactory('test.otlp');
$exporter = $factory->fromConnectionString($input);
$this->assertNull($exporter);
}

/**
* @test
*/
public function testMissingLicenseKey()
{
$input = 'newrelic+https://trace-api.newrelic.com/trace/v1';
$factory = new ExporterFactory('test.newrelic');
$exporter = $factory->fromConnectionString($input);
$this->assertNull($exporter);

$input = 'zipkintonewrelic+https://trace-api.newrelic.com/trace/v1';
$factory = new ExporterFactory('test.zipkintonewrelic');
$exporter = $factory->fromConnectionString($input);
$this->assertNull($exporter);
}
Copy link

Choose a reason for hiding this comment

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

In terms of code structure these tests look good

}