Skip to content

Commit

Permalink
fixup! allow setting service name from auto instrumentation context
Browse files Browse the repository at this point in the history
  • Loading branch information
Przemek Delewski committed Dec 6, 2022
1 parent 2a789e0 commit 0b04c02
Show file tree
Hide file tree
Showing 21 changed files with 69 additions and 87 deletions.
12 changes: 6 additions & 6 deletions src/Contrib/Jaeger/AgentExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

namespace OpenTelemetry\Contrib\Jaeger;

use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
use OpenTelemetry\SDK\Trace\Behavior\SpanExporterTrait;
use OpenTelemetry\SDK\Trace\SpanExporterInterface;
use OpenTelemetry\SemConv\ResourceAttributes;

/**
* @package OpenTelemetry\Exporter
Expand All @@ -14,22 +16,17 @@ class AgentExporter implements SpanExporterInterface
{
use SpanExporterTrait;

private string $serviceName;

private SpanConverter $spanConverter;

private JaegerTransport $jaegerTransport;

public function __construct(
$name,
string $endpointUrl
) {
$parsedEndpoint = (new ParsedEndpointUrl($endpointUrl))
->validateHost() //This is because the host is required downstream
->validatePort(); //This is because the port is required downstream

$this->serviceName = $name;

$this->spanConverter = new SpanConverter();
$this->jaegerTransport = new JaegerTransport($parsedEndpoint);
}
Expand All @@ -43,9 +40,12 @@ public function doExport(iterable $spans): bool
{
// UDP Transport begins here after converting to thrift format span
foreach ($spans as $span) {
$serviceName = $span->getResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME)
??
ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME);
$this->jaegerTransport->append(
$this->spanConverter->convert([$span])[0],
$this->serviceName
$serviceName
);
}

Expand Down
2 changes: 0 additions & 2 deletions src/Contrib/Jaeger/HttpCollectorExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class HttpCollectorExporter implements SpanExporterInterface

public function __construct(
string $endpointUrl,
string $name,
ClientInterface $client,
RequestFactoryInterface $requestFactory,
StreamFactoryInterface $streamFactory
Expand All @@ -32,7 +31,6 @@ public function __construct(
$client,
$requestFactory,
$streamFactory,
$name,
$parsedEndpoint
);
}
Expand Down
9 changes: 3 additions & 6 deletions src/Contrib/Jaeger/HttpSender.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OpenTelemetry\Contrib\Jaeger\TagFactory\TagFactory;
use OpenTelemetry\SDK\Behavior\LogsMessagesTrait;
use OpenTelemetry\SDK\Resource\ResourceInfo;
use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
use OpenTelemetry\SemConv\ResourceAttributes;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;
Expand All @@ -22,8 +23,6 @@ class HttpSender
{
use LogsMessagesTrait;

private string $serviceName;

private TProtocol $protocol;

private BatchAdapterFactoryInterface $batchAdapterFactory;
Expand All @@ -32,12 +31,9 @@ public function __construct(
ClientInterface $client,
RequestFactoryInterface $requestFactory,
StreamFactoryInterface $streamFactory,
string $serviceName,
ParsedEndpointUrl $parsedEndpoint,
BatchAdapterFactoryInterface $batchAdapterFactory = null
) {
$this->serviceName = $serviceName;

$this->protocol = new TBinaryProtocol(
new ThriftHttpTransport(
$client,
Expand Down Expand Up @@ -103,7 +99,8 @@ private function createBatchesPerResource(array $spansGroupedByResource): array

private function createProcessFromResource(ResourceInfo $resource): Process
{
$serviceName = $this->serviceName; //Defaulting to (what should be) the default resource's service name
//Defaulting to (what should be) the default resource's service name
$serviceName = ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME);

$tags = [];
foreach ($resource->getAttributes() as $key => $value) {
Expand Down
6 changes: 2 additions & 4 deletions src/Contrib/Newrelic/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,23 @@
*/
class Exporter implements SpanExporterInterface
{
public $name;
use LogsMessagesTrait;
use UsesSpanConverterTrait;

public const DATA_FORMAT_VERSION_DEFAULT = '1';

private TransportInterface $transport;
private string $name;
private string $endpointUrl;

public function __construct(
$name,
TransportInterface $transport,
string $endpointUrl,
SpanConverter $spanConverter = null
) {
$this->name = $name;
$this->endpointUrl = $endpointUrl;
$this->transport = $transport;
$this->setSpanConverter($spanConverter ?? new SpanConverter($name));
$this->setSpanConverter($spanConverter ?? new SpanConverter());
}

/**
Expand Down
15 changes: 7 additions & 8 deletions src/Contrib/Newrelic/SpanConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
namespace OpenTelemetry\Contrib\Newrelic;

use OpenTelemetry\SDK\Common\Time\Util as TimeUtil;
use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
use OpenTelemetry\SDK\Trace\SpanConverterInterface;
use OpenTelemetry\SDK\Trace\SpanDataInterface;
use OpenTelemetry\SemConv\ResourceAttributes;

/**
* @see https://docs.newrelic.com/docs/distributed-tracing/trace-api/report-new-relic-format-traces-trace-api/#new-relic-guidelines
Expand All @@ -16,13 +18,6 @@ class SpanConverter implements SpanConverterInterface
const STATUS_CODE_TAG_KEY = 'otel.status_code';
const STATUS_DESCRIPTION_TAG_KEY = 'otel.status_description';

private string $serviceName;

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

public function convert(iterable $spans): array
{
$aggregate = [];
Expand All @@ -40,12 +35,16 @@ private function convertSpan(SpanDataInterface $span): array
$startTimestamp = TimeUtil::nanosToMillis($span->getStartEpochNanos());
$endTimestamp = TimeUtil::nanosToMillis($span->getEndEpochNanos());

$serviceName = $span->getResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME)
??
ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME);

$row = [
'id' => $span->getSpanId(),
'trace.id' => $span->getTraceId(),
'attributes' => [
'name' => $span->getName(),
'service.name' => $this->serviceName,
'service.name' => $serviceName,
'parent.id' => $spanParent->isValid() ? $spanParent->getSpanId() : null,
'timestamp' => $startTimestamp,
'duration.ms' => (float) $endTimestamp - $startTimestamp,
Expand Down
7 changes: 1 addition & 6 deletions src/Contrib/Newrelic/SpanExporterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace OpenTelemetry\Contrib\Newrelic;

use OpenTelemetry\SDK\Common\Configuration\Configuration;
use OpenTelemetry\SDK\Common\Configuration\Variables;
use OpenTelemetry\SDK\Common\Export\Http\PsrTransportFactory;
use OpenTelemetry\SDK\Trace\SpanExporter\SpanExporterFactoryInterface;
use OpenTelemetry\SDK\Trace\SpanExporterInterface;
Expand All @@ -24,10 +23,6 @@ public function create(): SpanExporterInterface
'Data-Format-Version' => $dataFormatVersion,
]);

$serviceName = Configuration::has(Variables::OTEL_SERVICE_NAME)
? Configuration::getString(Variables::OTEL_SERVICE_NAME)
: 'newrelic';

return new Exporter($serviceName, $transport, $endpointUrl);
return new Exporter($transport, $endpointUrl);
}
}
3 changes: 1 addition & 2 deletions src/Contrib/Zipkin/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@ class Exporter implements SpanExporterInterface
private TransportInterface $transport;

public function __construct(
$name,
TransportInterface $transport,
SpanConverterInterface $spanConverter = null
) {
$this->transport = $transport;
$this->setSpanConverter($spanConverter ?? new SpanConverter($name));
$this->setSpanConverter($spanConverter ?? new SpanConverter());
}

/**
Expand Down
15 changes: 7 additions & 8 deletions src/Contrib/Zipkin/SpanConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
use OpenTelemetry\API\Trace\StatusCode;
use OpenTelemetry\Contrib\Zipkin\SpanKind as ZipkinSpanKind;
use OpenTelemetry\SDK\Common\Time\Util as TimeUtil;
use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
use OpenTelemetry\SDK\Trace\EventInterface;
use OpenTelemetry\SDK\Trace\SpanConverterInterface;
use OpenTelemetry\SDK\Trace\SpanDataInterface;
use OpenTelemetry\SemConv\ResourceAttributes;

class SpanConverter implements SpanConverterInterface
{
Expand All @@ -32,13 +34,6 @@ class SpanConverter implements SpanConverterInterface

const NET_PEER_IP_KEY = 'net.peer.ip';

private string $serviceName;

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

private function sanitiseTagValue($value)
{
// Casting false to string makes an empty string
Expand Down Expand Up @@ -78,11 +73,15 @@ private function convertSpan(SpanDataInterface $span): array
$startTimestamp = TimeUtil::nanosToMicros($span->getStartEpochNanos());
$endTimestamp = TimeUtil::nanosToMicros($span->getEndEpochNanos());

$serviceName = $span->getResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME)
??
ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME);

$row = [
'id' => $span->getSpanId(),
'traceId' => $span->getTraceId(),
'localEndpoint' => [
'serviceName' => $this->serviceName,
'serviceName' => $serviceName,
],
'name' => $span->getName(),
'timestamp' => $startTimestamp,
Expand Down
6 changes: 1 addition & 5 deletions src/Contrib/Zipkin/SpanExporterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ public function create(): SpanExporterInterface
$endpoint = Configuration::getString(Variables::OTEL_EXPORTER_ZIPKIN_ENDPOINT);
$transport = PsrTransportFactory::discover()->create($endpoint, 'application/json');

$serviceName = Configuration::has(Variables::OTEL_SERVICE_NAME)
? Configuration::getString(Variables::OTEL_SERVICE_NAME)
: 'zipkin';

return new Exporter($serviceName, $transport);
return new Exporter($transport);
}
}
6 changes: 2 additions & 4 deletions src/Contrib/ZipkinToNewrelic/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ class Exporter implements SpanExporterInterface
private TransportInterface $transport;

public function __construct(
string $name,
TransportInterface $transport,
SpanConverter $spanConverter = null
) {
$this->transport = $transport;
$this->setSpanConverter($spanConverter ?? new SpanConverter($name));
$this->setSpanConverter($spanConverter ?? new SpanConverter());
}

public static function create(
string $name,
string $endpointUrl,
string $licenseKey
): self {
Expand All @@ -49,7 +47,7 @@ public static function create(
'Data-Format-Version' => '2',
]);

return new self($name, $transport);
return new self($transport);
}

/**
Expand Down
14 changes: 7 additions & 7 deletions src/Contrib/ZipkinToNewrelic/SpanConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@

use function max;
use OpenTelemetry\SDK\Common\Time\Util as TimeUtil;
use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
use OpenTelemetry\SDK\Trace\SpanConverterInterface;
use OpenTelemetry\SDK\Trace\SpanDataInterface;
use OpenTelemetry\SemConv\ResourceAttributes;

class SpanConverter implements SpanConverterInterface
{
const STATUS_CODE_TAG_KEY = 'otel.status_code';
const STATUS_DESCRIPTION_TAG_KEY = 'otel.status_description';
private string $serviceName;

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

private function sanitiseTagValue($value)
{
Expand Down Expand Up @@ -59,12 +55,16 @@ private function convertSpan(SpanDataInterface $span): array
$startTimestamp = TimeUtil::nanosToMicros($span->getStartEpochNanos());
$endTimestamp = TimeUtil::nanosToMicros($span->getEndEpochNanos());

$serviceName = $span->getResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME)
??
ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME);

$row = [
'id' => $span->getSpanId(),
'traceId' => $span->getTraceId(),
'parentId' => $spanParent->isValid() ? $spanParent->getSpanId() : null,
'localEndpoint' => [
'serviceName' => $this->serviceName,
'serviceName' => $serviceName,
],
'name' => $span->getName(),
'timestamp' => $startTimestamp,
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Contrib/Jaeger/AgentExporterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class AgentExporterTest extends TestCase
{
public function test_happy_path()
{
$exporter = new AgentExporter('someServiceName', 'http://127.0.0.1:80');
$exporter = new AgentExporter('http://127.0.0.1:80');

$status = $exporter->export([new SpanData()])->await();

Expand Down
3 changes: 0 additions & 3 deletions tests/Unit/Contrib/Jaeger/JaegerExporterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@
*/
class JaegerExporterTest extends AbstractExporterTest
{
private const EXPORTER_NAME = 'test.jaeger';

public function createExporterWithTransport(TransportInterface $transport): Exporter
{
return new Exporter(
self::EXPORTER_NAME,
$transport
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public function test_happy_path()
*/
$exporter = new HttpCollectorExporter(
'https://hostOfJaegerCollector.com/post',
'nameOfThisService',
$this->getClientInterfaceMock(),
$this->getRequestFactoryInterfaceMock(),
$this->getStreamFactoryInterfaceMock()
Expand Down
8 changes: 6 additions & 2 deletions tests/Unit/Contrib/Jaeger/JaegerHttpSenderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use OpenTelemetry\Contrib\Jaeger\ParsedEndpointUrl;
use OpenTelemetry\SDK\Common\Attribute\Attributes;
use OpenTelemetry\SDK\Resource\ResourceInfo;
use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
use OpenTelemetry\SemConv\ResourceAttributes;
use OpenTelemetry\Tests\Unit\Contrib\UsesHttpClientTrait;
use OpenTelemetry\Tests\Unit\SDK\Util\SpanData;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -37,7 +39,6 @@ private function createSenderAndMocks(array $inputs): array
$this->getClientInterfaceMock(),
$this->getRequestFactoryInterfaceMock(),
$this->getStreamFactoryInterfaceMock(),
$serviceName,
$this->createParsedEndpointUrlMock(),
$mockBatchAdapterFactory
);
Expand Down Expand Up @@ -139,7 +140,10 @@ public function test_process_service_names_are_correctly_set_from_resource_attri
$interceptedValues = $mockBatchAdapterFactory->getInterceptedValues();

//1st batch
$this->assertSame('nameOfThe1stLogicalApp', $interceptedValues[0]['process']->serviceName);
$this->assertSame(
ResourceInfoFactory::defaultResource()->getAttributes()->get(ResourceAttributes::SERVICE_NAME),
$interceptedValues[0]['process']->serviceName
);

//2nd batch
$this->assertSame('nameOfThe2ndLogicalApp', $interceptedValues[1]['process']->serviceName);
Expand Down
Loading

0 comments on commit 0b04c02

Please sign in to comment.