Skip to content

Commit 7409bcd

Browse files
norberttechnorberttech
authored
feat(flow-php/phpunit-telemetry-bridge): configurable resource attributes (#2441)
- add `resource_attributes` parameter (env, urldecoded key=value pairs) - merge attributes onto the telemetry resource, overriding defaults - percent-decode `OTEL_RESOURCE_ATTRIBUTES` keys/values in telemetry EnvironmentDetector (OTel spec), dropping backslash escaping
1 parent f893cad commit 7409bcd

10 files changed

Lines changed: 286 additions & 129 deletions

File tree

‎documentation/components/bridges/phpunit-telemetry-bridge.md‎

Lines changed: 74 additions & 69 deletions
Large diffs are not rendered by default.

‎documentation/components/libs/telemetry.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,10 @@ The `EnvironmentDetector` reads standard OpenTelemetry environment variables:
923923
- `OTEL_SERVICE_NAME` - Sets the `service.name` attribute
924924
- `OTEL_RESOURCE_ATTRIBUTES` - Sets additional attributes in `key=value,key2=value2` format
925925

926+
Per the OpenTelemetry Resource SDK specification, `,` and `=` inside keys and values MUST be
927+
percent-encoded (other characters MAY be); both keys and values are percent-decoded. For example,
928+
`note=a%2Cb` yields the attribute `note` with value `a,b`.
929+
926930
```bash
927931
export OTEL_SERVICE_NAME=my-service
928932
export OTEL_RESOURCE_ATTRIBUTES=service.version=1.0.0,deployment.environment.name=production

‎documentation/upgrading.md‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ to_pgsql_table($client, 'users')->withTypesMap(new EntryTypesMap(
121121
));
122122
```
123123

124-
### 6) `flow-php/symfony-postgresql-messenger` - `messenger_messages` time columns use `timestamp` instead of `timestamptz`
124+
### 6) `flow-php/symfony-postgresql-messenger` - `messenger_messages` time columns use `timestamp` instead of
125+
`timestamptz`
125126

126127
| Column type for `created_at`, `available_at`, `delivered_at` | Before | After |
127128
|--------------------------------------------------------------|---------------|-------------|
@@ -132,11 +133,21 @@ Existing tables, realign the column type (UTC instants preserved):
132133

133134
```sql
134135
ALTER TABLE messenger_messages
135-
ALTER COLUMN created_at TYPE timestamp USING created_at AT TIME ZONE 'UTC',
136-
ALTER COLUMN available_at TYPE timestamp USING available_at AT TIME ZONE 'UTC',
137-
ALTER COLUMN delivered_at TYPE timestamp USING delivered_at AT TIME ZONE 'UTC';
136+
ALTER COLUMN created_at TYPE TIMESTAMP USING created_at AT TIME ZONE 'UTC',
137+
ALTER COLUMN available_at TYPE TIMESTAMP USING available_at AT TIME ZONE 'UTC',
138+
ALTER COLUMN delivered_at TYPE TIMESTAMP USING delivered_at AT TIME ZONE 'UTC';
138139
```
139140

141+
### 7) `flow-php/telemetry` - `OTEL_RESOURCE_ATTRIBUTES` keys and values are percent-decoded, not backslash-escaped
142+
143+
| Escaping a `,` or `=` in `OTEL_RESOURCE_ATTRIBUTES` | Before | After |
144+
|-----------------------------------------------------|---------------------------|-----------------------------|
145+
| literal comma in a value | `key=value\,with\,commas` | `key=value%2Cwith%2Ccommas` |
146+
| literal `=` in a value | not supported | `key=a%3Db` |
147+
148+
Re-encode any `OTEL_RESOURCE_ATTRIBUTES` that relied on backslash escaping; both keys and values are now
149+
percent-decoded.
150+
140151
---
141152

142153
## Upgrading from 0.37.x to 0.38.x

‎src/bridge/phpunit/telemetry/src/Flow/Bridge/PHPUnit/Telemetry/Configuration.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,9 @@
128128
'error_handler_severity',
129129
];
130130

131+
/**
132+
* @param array<string, string> $resourceAttributes
133+
*/
131134
public function __construct(
132135
public string $serviceName,
133136
public CurlTransportConfig|GrpcTransportConfig|StreamTransportConfig $transport,
@@ -138,6 +141,7 @@ public function __construct(
138141
public int $batchSize,
139142
public ErrorLogHandlerConfig|NullErrorHandlerConfig|StreamErrorHandlerConfig|SyslogErrorHandlerConfig|UdpSyslogErrorHandlerConfig $errorHandler,
140143
public bool $memoryRealUsage = false,
144+
public array $resourceAttributes = [],
141145
) {}
142146

143147
public static function fromParameters(ParameterCollection $parameters): self
@@ -168,6 +172,7 @@ public static function fromParameters(ParameterCollection $parameters): self
168172
batchSize: self::resolveInt($parameters, 'batch_size', self::DEFAULT_BATCH_SIZE),
169173
errorHandler: self::resolveErrorHandler($parameters),
170174
memoryRealUsage: self::resolveBool($parameters, 'memory_real_usage', false),
175+
resourceAttributes: self::parseResourceAttributes(self::resolve($parameters, 'resource_attributes') ?? ''),
171176
);
172177
}
173178

@@ -280,6 +285,44 @@ private static function parseHeaders(string $raw): array
280285
return $headers;
281286
}
282287

288+
/**
289+
* @return array<string, string>
290+
*/
291+
private static function parseResourceAttributes(string $raw): array
292+
{
293+
if ($raw === '') {
294+
return [];
295+
}
296+
297+
$attributes = [];
298+
299+
foreach (explode(',', $raw) as $entry) {
300+
$entry = trim($entry);
301+
302+
if ($entry === '') {
303+
continue;
304+
}
305+
306+
if (!str_contains($entry, '=')) {
307+
throw new InvalidArgumentException(sprintf(
308+
'Invalid resource attribute entry "%s", expected format "name=value".',
309+
$entry,
310+
));
311+
}
312+
313+
[$name, $value] = explode('=', $entry, 2);
314+
$name = urldecode(trim($name));
315+
316+
if ($name === '') {
317+
throw new InvalidArgumentException('Resource attribute name cannot be empty.');
318+
}
319+
320+
$attributes[$name] = urldecode($value);
321+
}
322+
323+
return $attributes;
324+
}
325+
283326
private static function readEnv(string $fullName): ?string
284327
{
285328
$value = $_ENV[$fullName] ?? $_SERVER[$fullName] ?? getenv($fullName);

‎src/bridge/phpunit/telemetry/src/Flow/Bridge/PHPUnit/Telemetry/TelemetryFactory.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public static function create(Configuration $config): Telemetry
4444
'service.name' => $config->serviceName,
4545
'telemetry.sdk.name' => 'flow-php-phpunit-telemetry',
4646
'telemetry.sdk.language' => 'php',
47-
]));
47+
]))
48+
->merge(resource($config->resourceAttributes));
4849

4950
$clock = new SystemClock();
5051
$contextStorage = memory_context_storage();

‎src/bridge/phpunit/telemetry/tests/Flow/Bridge/PHPUnit/Telemetry/Tests/Mother/ConfigurationMother.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,22 @@ public static function withRealMemoryUsage(): Configuration
154154
memoryRealUsage: true,
155155
);
156156
}
157+
158+
/**
159+
* @param array<string, string> $resourceAttributes
160+
*/
161+
public static function withResourceAttributes(array $resourceAttributes): Configuration
162+
{
163+
return new Configuration(
164+
serviceName: 'phpunit',
165+
transport: self::defaultTransport(),
166+
emitTraces: true,
167+
emitMetrics: true,
168+
emitTestSpans: true,
169+
emitTestCaseSpans: true,
170+
batchSize: Configuration::DEFAULT_BATCH_SIZE,
171+
errorHandler: self::defaultErrorHandler(),
172+
resourceAttributes: $resourceAttributes,
173+
);
174+
}
157175
}

‎src/bridge/phpunit/telemetry/tests/Flow/Bridge/PHPUnit/Telemetry/Tests/Unit/ConfigurationTest.php‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ public function test_default_configuration_uses_curl_transport_with_localhost_en
253253
static::assertTrue($config->emitTestSpans);
254254
static::assertTrue($config->emitTestCaseSpans);
255255
static::assertFalse($config->memoryRealUsage);
256+
static::assertSame([], $config->resourceAttributes);
256257

257258
static::assertInstanceOf(CurlTransportConfig::class, $config->transport);
258259
static::assertSame('http://localhost:4318', $config->transport->endpoint);
@@ -710,6 +711,89 @@ public function test_noop_error_handler_rejects_specific_params(): void
710711
]));
711712
}
712713

714+
public function test_resource_attributes_duplicate_name_last_wins(): void
715+
{
716+
$config = Configuration::fromParameters(ParameterCollection::fromArray([
717+
'resource_attributes' => 'team=first,team=second',
718+
]));
719+
720+
static::assertSame(['team' => 'second'], $config->resourceAttributes);
721+
}
722+
723+
public function test_resource_attributes_empty_name_throws(): void
724+
{
725+
$this->expectException(InvalidArgumentException::class);
726+
$this->expectExceptionMessage('Resource attribute name cannot be empty');
727+
728+
Configuration::fromParameters(ParameterCollection::fromArray([
729+
'resource_attributes' => '=value',
730+
]));
731+
}
732+
733+
public function test_resource_attributes_empty_string_is_empty_array(): void
734+
{
735+
$config = Configuration::fromParameters(ParameterCollection::fromArray([
736+
'resource_attributes' => '',
737+
]));
738+
739+
static::assertSame([], $config->resourceAttributes);
740+
}
741+
742+
public function test_resource_attributes_env_var_wins_over_parameter(): void
743+
{
744+
putenv('FLOW_PHPUNIT_OTEL_RESOURCE_ATTRIBUTES=service.version=2.0.0');
745+
746+
try {
747+
$config = Configuration::fromParameters(ParameterCollection::fromArray([
748+
'resource_attributes' => 'service.version=1.0.0',
749+
]));
750+
751+
static::assertSame(['service.version' => '2.0.0'], $config->resourceAttributes);
752+
} finally {
753+
putenv('FLOW_PHPUNIT_OTEL_RESOURCE_ATTRIBUTES');
754+
}
755+
}
756+
757+
public function test_resource_attributes_missing_equals_throws(): void
758+
{
759+
$this->expectException(InvalidArgumentException::class);
760+
$this->expectExceptionMessage('Invalid resource attribute entry "team", expected format "name=value"');
761+
762+
Configuration::fromParameters(ParameterCollection::fromArray([
763+
'resource_attributes' => 'team',
764+
]));
765+
}
766+
767+
public function test_resource_attributes_multiple_pairs_parsed(): void
768+
{
769+
$config = Configuration::fromParameters(ParameterCollection::fromArray([
770+
'resource_attributes' => 'service.version=1.0.0,deployment.environment.name=ci',
771+
]));
772+
773+
static::assertSame(
774+
['service.version' => '1.0.0', 'deployment.environment.name' => 'ci'],
775+
$config->resourceAttributes,
776+
);
777+
}
778+
779+
public function test_resource_attributes_single_pair_parsed(): void
780+
{
781+
$config = Configuration::fromParameters(ParameterCollection::fromArray([
782+
'resource_attributes' => 'service.version=1.0.0',
783+
]));
784+
785+
static::assertSame(['service.version' => '1.0.0'], $config->resourceAttributes);
786+
}
787+
788+
public function test_resource_attributes_url_encoded_value_decoded(): void
789+
{
790+
$config = Configuration::fromParameters(ParameterCollection::fromArray([
791+
'resource_attributes' => 'note=a%2Cb%20c',
792+
]));
793+
794+
static::assertSame(['note' => 'a,b c'], $config->resourceAttributes);
795+
}
796+
713797
public function test_server_superglobal_wins_over_getenv(): void
714798
{
715799
$_SERVER['FLOW_PHPUNIT_OTEL_ENDPOINT'] = 'https://from-server:4318';

‎src/bridge/phpunit/telemetry/tests/Flow/Bridge/PHPUnit/Telemetry/Tests/Unit/TelemetryFactoryTest.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,17 @@ public function test_create_with_null_error_handler(): void
140140
$telemetry->shutdown();
141141
}
142142

143+
public function test_create_with_resource_attributes(): void
144+
{
145+
$telemetry = TelemetryFactory::create(ConfigurationMother::withResourceAttributes([
146+
'service.version' => '1.0.0',
147+
'deployment.environment.name' => 'ci',
148+
]));
149+
150+
static::assertInstanceOf(Telemetry::class, $telemetry);
151+
$telemetry->shutdown();
152+
}
153+
143154
public function test_create_with_stream_error_handler(): void
144155
{
145156
$config = $this->configWithErrorHandler(new StreamErrorHandlerConfig(

‎src/lib/telemetry/src/Flow/Telemetry/Resource/Detector/EnvironmentDetector.php‎

Lines changed: 9 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,12 @@
88
use Flow\Telemetry\Resource\Attribute\ServiceAttribute;
99
use Flow\Telemetry\Resource\ResourceDetector;
1010

11-
use function count;
11+
use function explode;
1212
use function getenv;
13-
use function stripslashes;
14-
use function strlen;
1513
use function strpos;
1614
use function substr;
1715
use function trim;
16+
use function urldecode;
1817

1918
/**
2019
* Detects resource attributes from environment variables.
@@ -28,9 +27,11 @@
2827
* OTEL_RESOURCE_ATTRIBUTES=key1=value1,key2=value2
2928
* ```
3029
*
31-
* Special characters in values can be escaped with backslash:
30+
* Per the OpenTelemetry Resource SDK specification, the `,` and `=` characters in keys and
31+
* values MUST be percent-encoded (other characters MAY be percent-encoded); both keys and
32+
* values are percent-decoded:
3233
* ```
33-
* OTEL_RESOURCE_ATTRIBUTES=key=value\,with\,commas
34+
* OTEL_RESOURCE_ATTRIBUTES=key=value%2Cwith%2Ccommas
3435
* ```
3536
*
3637
* OTEL_SERVICE_NAME takes precedence over service.name in OTEL_RESOURCE_ATTRIBUTES.
@@ -83,9 +84,8 @@ private function parseResourceAttributes(): array
8384
}
8485

8586
$attributes = [];
86-
$pairs = $this->splitByComma($rawAttributes);
8787

88-
foreach ($pairs as $pair) {
88+
foreach (explode(',', $rawAttributes) as $pair) {
8989
$pair = trim($pair);
9090

9191
if ($pair === '') {
@@ -98,58 +98,15 @@ private function parseResourceAttributes(): array
9898
continue;
9999
}
100100

101-
$key = trim(substr($pair, 0, $equalsPos));
102-
$value = trim(substr($pair, $equalsPos + 1));
101+
$key = urldecode(trim(substr($pair, 0, $equalsPos)));
103102

104103
if ($key === '') {
105104
continue;
106105
}
107106

108-
$value = stripslashes($value);
109-
$attributes[$key] = $value;
107+
$attributes[$key] = urldecode(trim(substr($pair, $equalsPos + 1)));
110108
}
111109

112110
return $attributes;
113111
}
114-
115-
/**
116-
* Split a string by commas, respecting escaped commas.
117-
*
118-
* @return array<string>
119-
*/
120-
private function splitByComma(string $input): array
121-
{
122-
$result = [];
123-
$current = '';
124-
$length = strlen($input);
125-
$i = 0;
126-
127-
while ($i < $length) {
128-
$char = $input[$i];
129-
130-
if ($char === '\\' && ($i + 1) < $length) {
131-
$current .= $char . $input[$i + 1];
132-
$i += 2;
133-
134-
continue;
135-
}
136-
137-
if ($char === ',') {
138-
$result[] = $current;
139-
$current = '';
140-
$i++;
141-
142-
continue;
143-
}
144-
145-
$current .= $char;
146-
$i++;
147-
}
148-
149-
if ($current !== '' || count($result) > 0) {
150-
$result[] = $current;
151-
}
152-
153-
return $result;
154-
}
155112
}

0 commit comments

Comments
 (0)