Skip to content

Commit

Permalink
support url-encoded OTEL_EXPORTER_OTLP_HEADERS values (open-telemetry…
Browse files Browse the repository at this point in the history
…#1242)

Per a recent spec clarification in open-telemetry/opentelemetry-specification#3832 OTLP
headers should support url-encoded header values.

Co-authored-by: Tobias Bachert <[email protected]>
  • Loading branch information
brettmc and Nevay authored Feb 28, 2024
1 parent b20c45d commit 258e586
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 24 deletions.
5 changes: 1 addition & 4 deletions src/Contrib/Otlp/LogsExporterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ private function buildTransport(string $protocol): TransportInterface
{
$endpoint = $this->getEndpoint($protocol);

$headers = Configuration::has(Variables::OTEL_EXPORTER_OTLP_LOGS_HEADERS)
? Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_LOGS_HEADERS)
: Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_HEADERS);
$headers += OtlpUtil::getUserAgentHeader();
$headers = OtlpUtil::getHeaders(Signals::LOGS);
$compression = $this->getCompression();

$factoryClass = Registry::transportFactory($protocol);
Expand Down
5 changes: 1 addition & 4 deletions src/Contrib/Otlp/MetricExporterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ private function buildTransport(string $protocol): TransportInterface
*/
$endpoint = $this->getEndpoint($protocol);

$headers = Configuration::has(Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS)
? Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS)
: Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_HEADERS);
$headers += OtlpUtil::getUserAgentHeader();
$headers = OtlpUtil::getHeaders(Signals::METRICS);
$compression = $this->getCompression();

$factoryClass = Registry::transportFactory($protocol);
Expand Down
17 changes: 17 additions & 0 deletions src/Contrib/Otlp/OtlpUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace OpenTelemetry\Contrib\Otlp;

use OpenTelemetry\API\Signals;
use OpenTelemetry\SDK\Common\Configuration\Configuration;
use OpenTelemetry\SDK\Common\Configuration\Variables;
use OpenTelemetry\SDK\Resource\Detectors\Sdk;
use OpenTelemetry\SemConv\ResourceAttributes;
use UnexpectedValueException;
Expand All @@ -20,6 +22,11 @@ class OtlpUtil
Signals::METRICS => '/opentelemetry.proto.collector.metrics.v1.MetricsService/Export',
Signals::LOGS => '/opentelemetry.proto.collector.logs.v1.LogsService/Export',
];
private const HEADER_VARS = [
Signals::TRACE => Variables::OTEL_EXPORTER_OTLP_TRACES_HEADERS,
Signals::METRICS => Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS,
Signals::LOGS => Variables::OTEL_EXPORTER_OTLP_LOGS_HEADERS,
];

public static function method(string $signal): string
{
Expand All @@ -30,6 +37,16 @@ public static function method(string $signal): string
return self::METHODS[$signal];
}

public static function getHeaders(string $signal): array
{
$headers = Configuration::has(self::HEADER_VARS[$signal]) ?
Configuration::getMap(self::HEADER_VARS[$signal]) :
Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_HEADERS);
$headers += self::getUserAgentHeader();

return array_map('rawurldecode', $headers);
}

/**
* @link https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#user-agent
*/
Expand Down
11 changes: 1 addition & 10 deletions src/Contrib/Otlp/SpanExporterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private function buildTransport(): TransportInterface
$protocol = $this->getProtocol();
$contentType = Protocols::contentType($protocol);
$endpoint = $this->getEndpoint($protocol);
$headers = $this->getHeaders();
$headers = OtlpUtil::getHeaders(Signals::TRACE);
$compression = $this->getCompression();

$factoryClass = Registry::transportFactory($protocol);
Expand Down Expand Up @@ -78,15 +78,6 @@ private function getEndpoint(string $protocol): string
return HttpEndpointResolver::create()->resolveToString($endpoint, Signals::TRACE);
}

private function getHeaders(): array
{
$headers = Configuration::has(Variables::OTEL_EXPORTER_OTLP_TRACES_HEADERS) ?
Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_TRACES_HEADERS) :
Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_HEADERS);

return $headers + OtlpUtil::getUserAgentHeader();
}

private function getCompression(): string
{
return Configuration::has(Variables::OTEL_EXPORTER_OTLP_TRACES_COMPRESSION) ?
Expand Down
19 changes: 17 additions & 2 deletions tests/Unit/Contrib/Otlp/LogsExporterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function test_unknown_protocol_exception(): void
/**
* @dataProvider configProvider
*/
public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = []): void
public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = [], array $expectedValues = []): void
{
foreach ($env as $k => $v) {
$this->setEnvironmentVariable($k, $v);
Expand All @@ -61,8 +61,11 @@ public function test_create(array $env, string $endpoint, string $protocol, stri
->with(
$this->equalTo($endpoint),
$this->equalTo($protocol),
$this->callback(function ($headers) use ($headerKeys) {
$this->callback(function ($headers) use ($headerKeys, $expectedValues) {
$this->assertEqualsCanonicalizing($headerKeys, array_keys($headers));
foreach ($expectedValues as $key => $value) {
$this->assertSame($value, $headers[$key]);
}

return true;
}),
Expand Down Expand Up @@ -161,6 +164,18 @@ public static function configProvider(): array
'compression' => 'none',
'headerKeys' => array_merge($defaultHeaderKeys, ['key3', 'key4']),
],
'url-encoded headers' => [
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'Authorization=Basic%20AAA',
],
'endpoint' => 'http://localhost:4318/v1/logs',
'protocol' => 'application/x-protobuf',
'compression' => 'none',
'headerKeys' => array_merge($defaultHeaderKeys, ['Authorization']),
'expectedValues' => [
'Authorization' => 'Basic AAA',
],
],
];
}
}
23 changes: 21 additions & 2 deletions tests/Unit/Contrib/Otlp/MetricExporterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public static function temporalityProvider(): array
/**
* @dataProvider configProvider
*/
public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = []): void
public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = [], array $expectedValues = []): void
{
foreach ($env as $k => $v) {
$this->setEnvironmentVariable($k, $v);
Expand All @@ -114,8 +114,11 @@ public function test_create(array $env, string $endpoint, string $protocol, stri
->with(
$this->equalTo($endpoint),
$this->equalTo($protocol),
$this->callback(function ($headers) use ($headerKeys) {
$this->callback(function ($headers) use ($headerKeys, $expectedValues) {
$this->assertEqualsCanonicalizing($headerKeys, array_keys($headers));
foreach ($expectedValues as $key => $value) {
$this->assertSame($value, $headers[$key]);
}

return true;
}),
Expand Down Expand Up @@ -213,6 +216,22 @@ public static function configProvider(): array
'protocol' => 'application/x-protobuf',
'compression' => 'none',
'headerKeys' => array_merge($defaultHeaderKeys, ['key3', 'key4']),
'expectedValues' => [
'key3' => 'foo',
'key4' => 'bar',
],
],
'url-encoded headers' => [
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'Authorization=Basic%20AAA',
],
'endpoint' => 'http://localhost:4318/v1/metrics',
'protocol' => 'application/x-protobuf',
'compression' => 'none',
'headerKeys' => array_merge($defaultHeaderKeys, ['Authorization']),
'expectedValues' => [
'Authorization' => 'Basic AAA',
],
],
];
}
Expand Down
102 changes: 102 additions & 0 deletions tests/Unit/Contrib/Otlp/OtlpUtilTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,24 @@

namespace OpenTelemetry\Tests\Unit\Contrib\Otlp;

use AssertWell\PHPUnitGlobalState\EnvironmentVariables;
use OpenTelemetry\API\Signals;
use OpenTelemetry\Contrib\Otlp\OtlpUtil;
use OpenTelemetry\SDK\Common\Configuration\Variables;
use PHPUnit\Framework\TestCase;

/**
* @covers \OpenTelemetry\Contrib\Otlp\OtlpUtil
*/
class OtlpUtilTest extends TestCase
{
use EnvironmentVariables;

public function tearDown(): void
{
$this->restoreEnvironmentVariables();
}

public function test_get_user_agent_header(): void
{
$header = OtlpUtil::getUserAgentHeader();
Expand Down Expand Up @@ -43,4 +52,97 @@ public static function methodProvider(): array
[Signals::LOGS, 'LogsService'],
];
}

/**
* @dataProvider headersProvider
*/
public function test_get_headers(string $signal, array $env, array $expected): void
{
foreach ($env as $var => $value) {
$this->setEnvironmentVariable($var, $value);
}
$headers = OtlpUtil::getHeaders($signal);

$this->assertGreaterThanOrEqual(count($expected), $headers);
foreach ($expected as $key => $value) {
$this->assertArrayHasKey($key, $headers);
$this->assertSame($value, $headers[$key]);
}
}

public static function headersProvider(): array
{
return [
'trace' => [
'signal' => Signals::TRACE,
'env' => [
Variables::OTEL_EXPORTER_OTLP_TRACES_HEADERS => 'foo=bar,baz=bat',
],
'expected' => [
'foo' => 'bar',
'baz' => 'bat',
],
],
'trace with default' => [
'signal' => Signals::TRACE,
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'foo=bar,baz=bat',
],
'expected' => [
'foo' => 'bar',
'baz' => 'bat',
],
],
'metrics' => [
'signal' => Signals::METRICS,
'env' => [
Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS => 'foo=bar,baz=bat',
],
'expected' => [
'foo' => 'bar',
'baz' => 'bat',
],
],
'metrics with default' => [
'signal' => Signals::METRICS,
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'foo=bar,baz=bat',
],
'expected' => [
'foo' => 'bar',
'baz' => 'bat',
],
],
'logs' => [
'signal' => Signals::LOGS,
'env' => [
Variables::OTEL_EXPORTER_OTLP_LOGS_HEADERS => 'foo=bar,baz=bat',
],
'expected' => [
'foo' => 'bar',
'baz' => 'bat',
],
],
'logs with default' => [
'signal' => Signals::LOGS,
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'foo=bar,baz=bat',
],
'expected' => [
'foo' => 'bar',
'baz' => 'bat',
],
],
'url-encoded values' => [
'signal' => Signals::TRACE,
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'Authorization=Bearer%20secret,foo=%21%40%23%24%25%5E%26%2A%28%29',
],
'expected' => [
'Authorization' => 'Bearer secret',
'foo' => '!@#$%^&*()',
],
],
];
}
}
23 changes: 21 additions & 2 deletions tests/Unit/Contrib/Otlp/SpanExporterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function test_unknown_protocol_exception(): void
/**
* @dataProvider configProvider
*/
public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = []): void
public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = [], array $expectedValues = []): void
{
foreach ($env as $k => $v) {
$this->setEnvironmentVariable($k, $v);
Expand All @@ -58,8 +58,11 @@ public function test_create(array $env, string $endpoint, string $protocol, stri
->with(
$this->equalTo($endpoint),
$this->equalTo($protocol),
$this->callback(function ($headers) use ($headerKeys) {
$this->callback(function ($headers) use ($headerKeys, $expectedValues) {
$this->assertEqualsCanonicalizing($headerKeys, array_keys($headers));
foreach ($expectedValues as $key => $value) {
$this->assertSame($headers[$key], $value);
}

return true;
}),
Expand Down Expand Up @@ -157,6 +160,22 @@ public static function configProvider(): array
'protocol' => 'application/x-protobuf',
'compression' => 'none',
'headerKeys' => array_merge($defaultHeaderKeys, ['key3', 'key4']),
'expectedValues' => [
'key3' => 'foo',
'key4' => 'bar',
],
],
'url-encoded headers' => [
'env' => [
Variables::OTEL_EXPORTER_OTLP_HEADERS => 'Authorization=Basic%20AAA',
],
'endpoint' => 'http://localhost:4318/v1/traces',
'protocol' => 'application/x-protobuf',
'compression' => 'none',
'headerKeys' => array_merge($defaultHeaderKeys, ['Authorization']),
'expectedValues' => [
'Authorization' => 'Basic AAA',
],
],
];
}
Expand Down

0 comments on commit 258e586

Please sign in to comment.