Repository: open-telemetry/opentelemetry-php (php)

Issue:
Complex attributes must be allowed for LogRecord
Log data model (unlike traces and metrics) allows complex attribute values, non-homogeneous arrays, nested maps: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#type-any

The current implementation disallows such values, but should not.

**Steps to reproduce**
Use this code to create a log record:
```php
$record = (new LogRecord(['foo' => 'bar', 'baz' => 'bat', 'msg' => 'hello world']))
    ->setSeverityText('INFO')
    ->setSeverityNumber(9)
    ->setAttributes(['attr1' => 'val1','attr2' => 123, 'arrnum' => [1,2,3], 'complex' => [1, 'a'], 'nested' => [123, 'abc', ['sub'=>'val']]]);

$logger->emit($record);
```

**What is the expected behavior?**
All attributes should be output.

**What is the actual behavior?**
Complex attributes `complex` and `nested` are dropped:
```
                "attributes": {
                    "attr1": "val1",
                    "attr2": 123,
                    "arrnum": [
                        1,
                        2,
                        3
                    ]
                },
```

**Additional context**
Part of https://github.com/open-telemetry/community/issues/1537


Pull request:
refactor attribute validation
moving private method into a testable class, introduce attribute validation class which applies to most attribute. LogRecord attributes are allowed to be complex, so introduce a validator for these which lets everything through.

Fixes #1074 

Patch:
diff --git a/examples/logs/exporters/otlp_http.php b/examples/logs/exporters/otlp_http.php
index 1b6bc894..85cb00c1 100644
--- a/examples/logs/exporters/otlp_http.php
+++ b/examples/logs/exporters/otlp_http.php
@@ -35,7 +35,7 @@ $loggerProvider = new LoggerProvider(
         (new LogRecordLimitsBuilder())->build()->getAttributeFactory()
     )
 );
-$logger = $loggerProvider->getLogger('demo', '1.0', 'http://schema.url', ['foo' => 'bar']);
+$logger = $loggerProvider->getLogger('demo', '1.0', 'https://opentelemetry.io/schemas/1.7.1', ['foo' => 'bar']);
 $eventLogger = new EventLogger($logger, 'my-domain');
 
 $record = (new LogRecord(['foo' => 'bar', 'baz' => 'bat', 'msg' => 'hello world']))
diff --git a/proto/otel/VERSION b/proto/otel/VERSION
index 1847373e..0ec25f75 100644
--- a/proto/otel/VERSION
+++ b/proto/otel/VERSION
@@ -1,1 +1,1 @@
-v0.20.0
+v1.0.0
diff --git a/src/Contrib/Otlp/LogsConverter.php b/src/Contrib/Otlp/LogsConverter.php
index 07888769..1da53ad1 100644
--- a/src/Contrib/Otlp/LogsConverter.php
+++ b/src/Contrib/Otlp/LogsConverter.php
@@ -110,6 +110,7 @@ class LogsConverter
         $this->setAttributes($pInstrumentationScope, $instrumentationScope->getAttributes());
         $pInstrumentationScope->setDroppedAttributesCount($instrumentationScope->getAttributes()->getDroppedAttributesCount());
         $pScopeLogs->setScope($pInstrumentationScope);
+        $pScopeLogs->setSchemaUrl((string) $instrumentationScope->getSchemaUrl());
 
         return $pScopeLogs;
     }
diff --git a/src/SDK/Common/Attribute/AttributeValidator.php b/src/SDK/Common/Attribute/AttributeValidator.php
new file mode 100644
index 00000000..e9a1f733
--- /dev/null
+++ b/src/SDK/Common/Attribute/AttributeValidator.php
@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Attribute;
+
+class AttributeValidator implements AttributeValidatorInterface
+{
+    private const PRIMITIVES = [
+        'string',
+        'integer',
+        'double',
+        'boolean',
+    ];
+    private const NUMERICS = [
+        'double',
+        'integer',
+    ];
+
+    /**
+     * Validate whether a value is a primitive, or a homogeneous array of primitives (treating int/double as equivalent).
+     * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/common/README.md#attribute
+     */
+    public function validate($value): bool
+    {
+        if (is_array($value)) {
+            return $this->validateArray($value);
+        }
+
+        return in_array(gettype($value), self::PRIMITIVES);
+    }
+
+    private function validateArray(array $value): bool
+    {
+        if ($value === []) {
+            return true;
+        }
+        $type = gettype(reset($value));
+        if (!in_array($type, self::PRIMITIVES)) {
+            return false;
+        }
+        foreach ($value as $v) {
+            if (in_array(gettype($v), self::NUMERICS) && in_array($type, self::NUMERICS)) {
+                continue;
+            }
+            if (gettype($v) !== $type) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    public function getInvalidMessage(): string
+    {
+        return 'attribute with non-primitive or non-homogeneous array of primitives dropped';
+    }
+}
diff --git a/src/SDK/Common/Attribute/AttributeValidatorInterface.php b/src/SDK/Common/Attribute/AttributeValidatorInterface.php
new file mode 100644
index 00000000..afbfba6e
--- /dev/null
+++ b/src/SDK/Common/Attribute/AttributeValidatorInterface.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Attribute;
+
+interface AttributeValidatorInterface
+{
+    public function validate($value): bool;
+    public function getInvalidMessage(): string;
+}
diff --git a/src/SDK/Common/Attribute/AttributesBuilder.php b/src/SDK/Common/Attribute/AttributesBuilder.php
index fc5af3e6..5c115063 100644
--- a/src/SDK/Common/Attribute/AttributesBuilder.php
+++ b/src/SDK/Common/Attribute/AttributesBuilder.php
@@ -6,7 +6,6 @@ namespace OpenTelemetry\SDK\Common\Attribute;
 
 use function array_key_exists;
 use function count;
-use function gettype;
 use function is_array;
 use function is_string;
 use function mb_substr;
@@ -23,14 +22,20 @@ final class AttributesBuilder implements AttributesBuilderInterface
     private ?int $attributeCountLimit;
     private ?int $attributeValueLengthLimit;
     private int $droppedAttributesCount;
-    private const NUMERICS = ['integer', 'double'];
-
-    public function __construct(array $attributes, ?int $attributeCountLimit, ?int $attributeValueLengthLimit, int $droppedAttributesCount)
-    {
+    private AttributeValidatorInterface $attributeValidator;
+
+    public function __construct(
+        array $attributes,
+        ?int $attributeCountLimit,
+        ?int $attributeValueLengthLimit,
+        int $droppedAttributesCount,
+        ?AttributeValidatorInterface $attributeValidator
+    ) {
         $this->attributes = $attributes;
         $this->attributeCountLimit = $attributeCountLimit;
         $this->attributeValueLengthLimit = $attributeValueLengthLimit;
         $this->droppedAttributesCount = $droppedAttributesCount;
+        $this->attributeValidator = $attributeValidator ?? new AttributeValidator();
     }
 
     public function build(): AttributesInterface
@@ -66,8 +71,9 @@ final class AttributesBuilder implements AttributesBuilderInterface
 
             return;
         }
-        if (is_array($value) && !$this->isHomogeneous($value)) {
-            self::logWarning('attribute with non-homogeneous array values dropped: ' . $offset);
+        if (!$this->attributeValidator->validate($value)) {
+            self::logWarning($this->attributeValidator->getInvalidMessage() . ': ' . $offset);
+            $this->droppedAttributesCount++;
 
             return;
         }
@@ -111,26 +117,4 @@ final class AttributesBuilder implements AttributesBuilderInterface
 
         return $value;
     }
-
-    /**
-     * Test whether an array contains only values of the same type, treating int/double as equivalent.
-     * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/common/README.md#attribute
-     */
-    private function isHomogeneous(array $value): bool
-    {
-        if (count($value) <= 1) {
-            return true;
-        }
-        $type = gettype(reset($value));
-        foreach ($value as $v) {
-            if (in_array(gettype($v), self::NUMERICS) && in_array($type, self::NUMERICS)) {
-                continue;
-            }
-            if (gettype($v) !== $type) {
-                return false;
-            }
-        }
-
-        return true;
-    }
 }
diff --git a/src/SDK/Common/Attribute/AttributesFactory.php b/src/SDK/Common/Attribute/AttributesFactory.php
index 0db02cae..d53ab25a 100644
--- a/src/SDK/Common/Attribute/AttributesFactory.php
+++ b/src/SDK/Common/Attribute/AttributesFactory.php
@@ -18,13 +18,14 @@ final class AttributesFactory implements AttributesFactoryInterface
         $this->attributeValueLengthLimit = $attributeValueLengthLimit;
     }
 
-    public function builder(iterable $attributes = []): AttributesBuilderInterface
+    public function builder(iterable $attributes = [], ?AttributeValidatorInterface $attributeValidator = null): AttributesBuilderInterface
     {
         $builder = new AttributesBuilder(
             [],
             $this->attributeCountLimit,
             $this->attributeValueLengthLimit,
             0,
+            $attributeValidator,
         );
         foreach ($attributes as $key => $value) {
             $builder[$key] = $value;
diff --git a/src/SDK/Common/Attribute/AttributesFactoryInterface.php b/src/SDK/Common/Attribute/AttributesFactoryInterface.php
index 79ef3934..1b74461d 100644
--- a/src/SDK/Common/Attribute/AttributesFactoryInterface.php
+++ b/src/SDK/Common/Attribute/AttributesFactoryInterface.php
@@ -6,5 +6,5 @@ namespace OpenTelemetry\SDK\Common\Attribute;
 
 interface AttributesFactoryInterface
 {
-    public function builder(iterable $attributes = []): AttributesBuilderInterface;
+    public function builder(iterable $attributes = [], ?AttributeValidatorInterface $attributeValidator = null): AttributesBuilderInterface;
 }
diff --git a/src/SDK/Common/Attribute/FilteredAttributesFactory.php b/src/SDK/Common/Attribute/FilteredAttributesFactory.php
index 74026ad2..1d9c4ae1 100644
--- a/src/SDK/Common/Attribute/FilteredAttributesFactory.php
+++ b/src/SDK/Common/Attribute/FilteredAttributesFactory.php
@@ -21,9 +21,9 @@ final class FilteredAttributesFactory implements AttributesFactoryInterface
         $this->rejectedKeys = $rejectedKeys;
     }
 
-    public function builder(iterable $attributes = []): AttributesBuilderInterface
+    public function builder(iterable $attributes = [], ?AttributeValidatorInterface $attributeValidator = null): AttributesBuilderInterface
     {
-        $builder = new FilteredAttributesBuilder($this->factory->builder(), $this->rejectedKeys);
+        $builder = new FilteredAttributesBuilder($this->factory->builder([], $attributeValidator), $this->rejectedKeys);
         foreach ($attributes as $attribute => $value) {
             $builder[$attribute] = $value;
         }
diff --git a/src/SDK/Common/Attribute/LogRecordAttributeValidator.php b/src/SDK/Common/Attribute/LogRecordAttributeValidator.php
new file mode 100644
index 00000000..a09d2637
--- /dev/null
+++ b/src/SDK/Common/Attribute/LogRecordAttributeValidator.php
@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Attribute;
+
+class LogRecordAttributeValidator implements AttributeValidatorInterface
+{
+    public function validate($value): bool
+    {
+        return true;
+    }
+
+    public function getInvalidMessage(): string
+    {
+        //not required as this validator always returns true
+        return 'unused';
+    }
+}
diff --git a/src/SDK/Logs/ReadableLogRecord.php b/src/SDK/Logs/ReadableLogRecord.php
index 74666b30..5c653147 100644
--- a/src/SDK/Logs/ReadableLogRecord.php
+++ b/src/SDK/Logs/ReadableLogRecord.php
@@ -10,6 +10,7 @@ use OpenTelemetry\API\Trace\SpanContextInterface;
 use OpenTelemetry\Context\Context;
 use OpenTelemetry\Context\ContextInterface;
 use OpenTelemetry\SDK\Common\Attribute\AttributesInterface;
+use OpenTelemetry\SDK\Common\Attribute\LogRecordAttributeValidator;
 use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;
 use OpenTelemetry\SDK\Resource\ResourceInfo;
 
@@ -43,7 +44,7 @@ class ReadableLogRecord extends LogRecord
         $this->convertedAttributes = $this->loggerSharedState
             ->getLogRecordLimits()
             ->getAttributeFactory()
-            ->builder($logRecord->attributes)
+            ->builder($logRecord->attributes, new LogRecordAttributeValidator())
             ->build();
     }
 


Test patch:
diff --git a/tests/Unit/SDK/Common/Attribute/AttributeValidatorTest.php b/tests/Unit/SDK/Common/Attribute/AttributeValidatorTest.php
new file mode 100644
index 00000000..ba2389f6
--- /dev/null
+++ b/tests/Unit/SDK/Common/Attribute/AttributeValidatorTest.php
@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Tests\Unit\SDK\Common\Attribute;
+
+use OpenTelemetry\SDK\Common\Attribute\AttributeValidator;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \OpenTelemetry\SDK\Common\Attribute\AttributeValidator
+ */
+class AttributeValidatorTest extends TestCase
+{
+    private AttributeValidator $validator;
+
+    public function setUp(): void
+    {
+        $this->validator = new AttributeValidator();
+    }
+
+    /**
+     * @dataProvider primitiveProvider
+     */
+    public function test_validate_primitives($value): void
+    {
+        $this->assertTrue($this->validator->validate($value));
+    }
+
+    public static function primitiveProvider(): array
+    {
+        return [
+            'bool true' => [true],
+            'bool false' => [false],
+            'string' => ['hello otel'],
+            'int' => [4],
+            'double' => [3.14159],
+        ];
+    }
+
+    /**
+     * @dataProvider nonPrimitiveProvider
+     */
+    public function test_validate_non_primitives($value): void
+    {
+        $this->assertFalse($this->validator->validate($value));
+    }
+
+    public static function nonPrimitiveProvider(): array
+    {
+        return [
+            'object' => [new \stdClass()],
+            'null' => [null],
+            'resource' => [tmpfile()],
+        ];
+    }
+
+    /**
+     * @dataProvider arrayProvider
+     */
+    public function test_validate_array($value, bool $expected): void
+    {
+        $this->assertSame($expected, $this->validator->validate($value));
+    }
+
+    public static function arrayProvider(): array
+    {
+        return [
+            'empty array' => [[], true],
+            'array of strings' => [['one', 'two'], true],
+            'array of ints' => [[1, 2], true],
+            'array of double' => [[1.1, 1.2], true],
+            'mixed numerics' => [[2, 3.5, PHP_INT_MAX], true],
+            'array of bool' => [[true, false], true],
+            'mixed array' => [[true, 'one', 2], false],
+            'complex array' => [['one' => ['one']], false],
+        ];
+    }
+}
diff --git a/tests/Unit/SDK/Logs/ReadableLogRecordTest.php b/tests/Unit/SDK/Logs/ReadableLogRecordTest.php
index ac409a37..1c946278 100644
--- a/tests/Unit/SDK/Logs/ReadableLogRecordTest.php
+++ b/tests/Unit/SDK/Logs/ReadableLogRecordTest.php
@@ -19,34 +19,62 @@ use PHPUnit\Framework\TestCase;
  */
 class ReadableLogRecordTest extends TestCase
 {
-    public function test_getters(): void
+    private InstrumentationScopeInterface $scope;
+    private ContextInterface $context;
+    private LoggerSharedState $sharedState;
+    private ResourceInfo $resource;
+    public function setUp(): void
     {
-        $scope = $this->createMock(InstrumentationScopeInterface::class);
-        $sharedState = $this->createMock(LoggerSharedState::class);
-        $context = $this->createMock(ContextInterface::class);
-        $resource = $this->createMock(ResourceInfo::class);
+        $this->scope = $this->createMock(InstrumentationScopeInterface::class);
+        $this->sharedState = $this->createMock(LoggerSharedState::class);
+        $this->context = $this->createMock(ContextInterface::class);
+        $this->resource = $this->createMock(ResourceInfo::class);
         $limits = $this->createMock(LogRecordLimits::class);
         $attributeFactory = new AttributesFactory();
         $limits->method('getAttributeFactory')->willReturn($attributeFactory); //final
-        $sharedState->method('getResource')->willReturn($resource);
-        $sharedState->method('getLogRecordLimits')->willReturn($limits);
+        $this->sharedState->method('getResource')->willReturn($this->resource);
+        $this->sharedState->method('getLogRecordLimits')->willReturn($limits);
+    }
+
+    public function test_getters(): void
+    {
         $logRecord = (new LogRecord('body'))
             ->setSeverityNumber(5)
             ->setSeverityText('info')
             ->setTimestamp(11)
             ->setObservedTimestamp(22)
             ->setAttributes(['foo' => 'bar'])
-            ->setContext($context);
-        $record = new ReadableLogRecord($scope, $sharedState, $logRecord);
+            ->setContext($this->context);
+        $record = new ReadableLogRecord($this->scope, $this->sharedState, $logRecord);
 
-        $this->assertSame($scope, $record->getInstrumentationScope());
-        $this->assertSame($resource, $record->getResource());
+        $this->assertSame($this->scope, $record->getInstrumentationScope());
+        $this->assertSame($this->resource, $record->getResource());
         $this->assertSame(11, $record->getTimestamp());
         $this->assertSame(22, $record->getObservedTimestamp());
-        $this->assertSame($context, $record->getContext());
+        $this->assertSame($this->context, $record->getContext());
         $this->assertSame(5, $record->getSeverityNumber());
         $this->assertSame('info', $record->getSeverityText());
         $this->assertSame('body', $record->getBody());
         $this->assertEquals(['foo' => 'bar'], $record->getAttributes()->toArray());
     }
+
+    /**
+     * @group logs-compliance
+     */
+    public function test_log_record_can_accept_complex_attributes(): void
+    {
+        $homogeneous = [1,2,3,4,5];
+        $heterogeneous = ['one', 2, 3.14, true];
+        $complex = ['foo' => ['bar' => 'baz', 'bat' => 3.14]];
+        $logRecord = (new LogRecord())
+            ->setAttribute('homogeneous', $homogeneous)
+            ->setAttribute('heterogeneous', $heterogeneous)
+            ->setAttribute('complex', $complex);
+
+        $record = new ReadableLogRecord($this->scope, $this->sharedState, $logRecord);
+        $this->assertSame(0, $record->getAttributes()->getDroppedAttributesCount());
+        $this->assertSame($complex, $record->getAttributes()->get('complex'));
+        $this->assertSame($homogeneous, $record->getAttributes()->get('homogeneous'));
+        $this->assertSame($heterogeneous, $record->getAttributes()->get('heterogeneous'));
+    }
 }