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

Issue:
SDK should drop `enduser.*` attributes by default
[General identity attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/span-general.md#general-identity-attributes):
> Given the sensitive nature of this information, SDKs and exporters SHOULD drop these attributes by default and then provide a configuration parameter to turn on retention for use cases where the information is required and would not violate any policies or regulations.

Pull request:
Drop general identity attributes by default
Resolves #842.

Patch:
diff --git a/src/SDK/Common/Attribute/FilteredAttributesBuilder.php b/src/SDK/Common/Attribute/FilteredAttributesBuilder.php
new file mode 100644
index 00000000..d79cff96
--- /dev/null
+++ b/src/SDK/Common/Attribute/FilteredAttributesBuilder.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Attribute;
+
+use function in_array;
+
+/**
+ * @internal
+ */
+final class FilteredAttributesBuilder implements AttributesBuilderInterface
+{
+    private AttributesBuilderInterface $builder;
+    private array $rejectedKeys;
+    private int $rejected = 0;
+
+    /**
+     * @param list<string> $rejectedKeys
+     */
+    public function __construct(AttributesBuilderInterface $builder, array $rejectedKeys)
+    {
+        $this->builder = $builder;
+        $this->rejectedKeys = $rejectedKeys;
+    }
+
+    public function __clone()
+    {
+        $this->builder = clone $this->builder;
+    }
+
+    public function build(): AttributesInterface
+    {
+        $attributes = $this->builder->build();
+        $dropped = $attributes->getDroppedAttributesCount() + $this->rejected;
+
+        return new Attributes($attributes->toArray(), $dropped);
+    }
+
+    public function offsetExists($offset): bool
+    {
+        return $this->builder->offsetExists($offset);
+    }
+
+    /**
+     * @phan-suppress PhanUndeclaredClassAttribute
+     */
+    #[\ReturnTypeWillChange]
+    public function offsetGet($offset)
+    {
+        return $this->builder->offsetGet($offset);
+    }
+
+    /**
+     * @phan-suppress PhanUndeclaredClassAttribute
+     */
+    #[\ReturnTypeWillChange]
+    public function offsetSet($offset, $value)
+    {
+        if ($value !== null && in_array($offset, $this->rejectedKeys, true)) {
+            $this->rejected++;
+
+            return;
+        }
+
+        $this->builder->offsetSet($offset, $value);
+    }
+
+    /**
+     * @phan-suppress PhanUndeclaredClassAttribute
+     */
+    #[\ReturnTypeWillChange]
+    public function offsetUnset($offset)
+    {
+        $this->builder->offsetUnset($offset);
+    }
+}
diff --git a/src/SDK/Common/Attribute/FilteredAttributesFactory.php b/src/SDK/Common/Attribute/FilteredAttributesFactory.php
new file mode 100644
index 00000000..74026ad2
--- /dev/null
+++ b/src/SDK/Common/Attribute/FilteredAttributesFactory.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Attribute;
+
+/**
+ * @internal
+ */
+final class FilteredAttributesFactory implements AttributesFactoryInterface
+{
+    private AttributesFactoryInterface $factory;
+    private array $rejectedKeys;
+
+    /**
+     * @param list<string> $rejectedKeys
+     */
+    public function __construct(AttributesFactoryInterface $factory, array $rejectedKeys)
+    {
+        $this->factory = $factory;
+        $this->rejectedKeys = $rejectedKeys;
+    }
+
+    public function builder(iterable $attributes = []): AttributesBuilderInterface
+    {
+        $builder = new FilteredAttributesBuilder($this->factory->builder(), $this->rejectedKeys);
+        foreach ($attributes as $attribute => $value) {
+            $builder[$attribute] = $value;
+        }
+
+        return $builder;
+    }
+}
diff --git a/src/SDK/Trace/SpanLimitsBuilder.php b/src/SDK/Trace/SpanLimitsBuilder.php
index cc3efa73..9fb85c8c 100644
--- a/src/SDK/Trace/SpanLimitsBuilder.php
+++ b/src/SDK/Trace/SpanLimitsBuilder.php
@@ -5,8 +5,10 @@ declare(strict_types=1);
 namespace OpenTelemetry\SDK\Trace;
 
 use OpenTelemetry\SDK\Common\Attribute\Attributes;
+use OpenTelemetry\SDK\Common\Attribute\FilteredAttributesFactory;
 use OpenTelemetry\SDK\Common\Environment\EnvironmentVariablesTrait;
 use OpenTelemetry\SDK\Common\Environment\Variables as Env;
+use OpenTelemetry\SemConv\TraceAttributes;
 use const PHP_INT_MAX;
 
 class SpanLimitsBuilder
@@ -31,6 +33,8 @@ class SpanLimitsBuilder
     /** @var ?int Maximum allowed attribute per span link count */
     private ?int $attributePerLinkCountLimit = null;
 
+    private bool $retainGeneralIdentityAttributes = false;
+
     /**
      * @param int $attributeCountLimit Maximum allowed attribute count per record
      */
@@ -91,6 +95,18 @@ class SpanLimitsBuilder
         return $this;
     }
 
+    /**
+     * @param bool $retain whether general identity attributes should be retained
+     *
+     * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/span-general.md#general-identity-attributes
+     */
+    public function retainGeneralIdentityAttributes(bool $retain = true): SpanLimitsBuilder
+    {
+        $this->retainGeneralIdentityAttributes = $retain;
+
+        return $this;
+    }
+
     /**
      * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md#span-limits-
      */
@@ -113,8 +129,18 @@ class SpanLimitsBuilder
             $attributeValueLengthLimit = null;
         }
 
+        $spanAttributesFactory = Attributes::factory($attributeCountLimit, $attributeValueLengthLimit);
+
+        if (!$this->retainGeneralIdentityAttributes) {
+            $spanAttributesFactory = new FilteredAttributesFactory($spanAttributesFactory, [
+                TraceAttributes::ENDUSER_ID,
+                TraceAttributes::ENDUSER_ROLE,
+                TraceAttributes::ENDUSER_SCOPE,
+            ]);
+        }
+
         return new SpanLimits(
-            Attributes::factory($attributeCountLimit, $attributeValueLengthLimit),
+            $spanAttributesFactory,
             Attributes::factory($attributePerEventCountLimit, $attributeValueLengthLimit),
             Attributes::factory($attributePerLinkCountLimit, $attributeValueLengthLimit),
             $eventCountLimit,


Test patch:
diff --git a/tests/Integration/SDK/SpanLimitsBuilderTest.php b/tests/Integration/SDK/SpanLimitsBuilderTest.php
index f5ba94b1..418349ee 100644
--- a/tests/Integration/SDK/SpanLimitsBuilderTest.php
+++ b/tests/Integration/SDK/SpanLimitsBuilderTest.php
@@ -28,6 +28,7 @@ class SpanLimitsBuilderTest extends TestCase
     {
         $this->setEnvironmentVariable('OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT', 9);
         $builder = new SpanLimitsBuilder();
+        $builder->retainGeneralIdentityAttributes();
         $spanLimits = $builder->build();
         $this->assertEquals(Attributes::factory(128, 9), $spanLimits->getAttributesFactory());
     }
@@ -39,6 +40,7 @@ class SpanLimitsBuilderTest extends TestCase
     {
         $this->setEnvironmentVariable('OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT', 9);
         $builder = new SpanLimitsBuilder();
+        $builder->retainGeneralIdentityAttributes();
         $builder->setAttributeValueLengthLimit(201);
         $spanLimits = $builder->build();
         $this->assertEquals(Attributes::factory(128, 201), $spanLimits->getAttributesFactory());
diff --git a/tests/Integration/SDK/TracerTest.php b/tests/Integration/SDK/TracerTest.php
index 672ad300..39b1e2da 100644
--- a/tests/Integration/SDK/TracerTest.php
+++ b/tests/Integration/SDK/TracerTest.php
@@ -15,8 +15,12 @@ use OpenTelemetry\SDK\Trace\SamplerInterface;
 use OpenTelemetry\SDK\Trace\SamplingResult;
 use OpenTelemetry\SDK\Trace\Span;
 use OpenTelemetry\SDK\Trace\SpanBuilder;
+use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter;
+use OpenTelemetry\SDK\Trace\SpanLimitsBuilder;
+use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor;
 use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
 use OpenTelemetry\SDK\Trace\TracerProvider;
+use OpenTelemetry\SemConv\TraceAttributes;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -110,4 +114,45 @@ class TracerTest extends TestCase
         $tracer = $tracerProvider->getTracer('foo');
         $this->assertInstanceOf(API\NoopTracer::class, $tracer);
     }
+
+    public function test_general_identity_attributes_are_dropped_by_default(): void
+    {
+        $exporter = new InMemoryExporter();
+        $tracerProvider = new TracerProvider(new SimpleSpanProcessor($exporter));
+        $tracer = $tracerProvider->getTracer('test');
+        $tracer->spanBuilder('test')
+            ->setAttribute(TraceAttributes::ENDUSER_ID, 'username')
+            ->setAttribute(TraceAttributes::ENDUSER_ROLE, 'admin')
+            ->setAttribute(TraceAttributes::ENDUSER_SCOPE, 'read:message, write:files')
+            ->startSpan()
+            ->end();
+
+        $tracerProvider->shutdown();
+
+        $attributes = $exporter->getSpans()[0]->getAttributes();
+        $this->assertCount(0, $attributes);
+        $this->assertSame(3, $attributes->getDroppedAttributesCount());
+    }
+
+    public function test_general_identity_attributes_are_retained_if_enabled(): void
+    {
+        $exporter = new InMemoryExporter();
+        $spanLimits = (new SpanLimitsBuilder())
+            ->retainGeneralIdentityAttributes()
+            ->build();
+        $tracerProvider = new TracerProvider(new SimpleSpanProcessor($exporter), null, null, $spanLimits);
+        $tracer = $tracerProvider->getTracer('test');
+        $tracer->spanBuilder('test')
+            ->setAttribute(TraceAttributes::ENDUSER_ID, 'username')
+            ->setAttribute(TraceAttributes::ENDUSER_ROLE, 'admin')
+            ->setAttribute(TraceAttributes::ENDUSER_SCOPE, 'read:message, write:files')
+            ->startSpan()
+            ->end();
+
+        $tracerProvider->shutdown();
+
+        $attributes = $exporter->getSpans()[0]->getAttributes();
+        $this->assertCount(3, $attributes);
+        $this->assertSame(0, $attributes->getDroppedAttributesCount());
+    }
 }
diff --git a/tests/Unit/SDK/Common/Attribute/FilteredAttributesTest.php b/tests/Unit/SDK/Common/Attribute/FilteredAttributesTest.php
new file mode 100644
index 00000000..4500c8ed
--- /dev/null
+++ b/tests/Unit/SDK/Common/Attribute/FilteredAttributesTest.php
@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Tests\Unit\SDK\Common\Attribute;
+
+use OpenTelemetry\SDK\Common\Attribute\Attributes;
+use OpenTelemetry\SDK\Common\Attribute\FilteredAttributesFactory;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \OpenTelemetry\SDK\Common\Attribute\FilteredAttributesBuilder
+ * @covers \OpenTelemetry\SDK\Common\Attribute\FilteredAttributesFactory
+ */
+class FilteredAttributesTest extends TestCase
+{
+    public function test_rejected_keys_are_dropped(): void
+    {
+        $attributesFactory = new FilteredAttributesFactory(Attributes::factory(), ['bar']);
+        $attributesBuilder = $attributesFactory->builder([
+            'foo' => 'foo',
+            'bar' => 'bar',
+            'baz' => 'baz',
+        ]);
+
+        $attributes = $attributesBuilder->build();
+        $this->assertCount(2, $attributes);
+        $this->assertSame(1, $attributes->getDroppedAttributesCount());
+        $this->assertTrue($attributes->has('foo'));
+        $this->assertFalse($attributes->has('bar'));
+        $this->assertTrue($attributes->has('baz'));
+    }
+
+    public function test_rejected_key_null_value_does_not_increase_dropped_count(): void
+    {
+        $attributesFactory = new FilteredAttributesFactory(Attributes::factory(), ['bar']);
+        $attributesBuilder = $attributesFactory->builder([
+            'bar' => null,
+        ]);
+
+        $attributes = $attributesBuilder->build();
+        $this->assertCount(0, $attributes);
+        $this->assertSame(0, $attributes->getDroppedAttributesCount());
+    }
+}
diff --git a/tests/Unit/SDK/Trace/SpanLimitsBuilderTest.php b/tests/Unit/SDK/Trace/SpanLimitsBuilderTest.php
index b377e823..21eb9b02 100644
--- a/tests/Unit/SDK/Trace/SpanLimitsBuilderTest.php
+++ b/tests/Unit/SDK/Trace/SpanLimitsBuilderTest.php
@@ -11,7 +11,7 @@ use OpenTelemetry\SDK\Trace\SpanLimitsBuilder;
 use PHPUnit\Framework\TestCase;
 
 /**
- * @covers OpenTelemetry\SDK\Trace\SpanLimitsBuilder
+ * @covers \OpenTelemetry\SDK\Trace\SpanLimitsBuilder
  */
 class SpanLimitsBuilderTest extends TestCase
 {
@@ -20,6 +20,7 @@ class SpanLimitsBuilderTest extends TestCase
     public function test_span_limits_builder_uses_default_values(): void
     {
         $builder = new SpanLimitsBuilder();
+        $builder->retainGeneralIdentityAttributes();
         $spanLimits = $builder->build();
         $this->assertEquals(Attributes::factory(128), $spanLimits->getAttributesFactory());
     }
@@ -31,6 +32,7 @@ class SpanLimitsBuilderTest extends TestCase
     {
         $this->setEnvironmentVariable('OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 111);
         $builder = new SpanLimitsBuilder();
+        $builder->retainGeneralIdentityAttributes();
         $spanLimits = $builder->build();
         $this->assertEquals(Attributes::factory(111), $spanLimits->getAttributesFactory());
     }
@@ -42,6 +44,7 @@ class SpanLimitsBuilderTest extends TestCase
     {
         $this->setEnvironmentVariable('OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 111);
         $builder = new SpanLimitsBuilder();
+        $builder->retainGeneralIdentityAttributes();
         $builder->setAttributeCountLimit(222);
         $spanLimits = $builder->build();
         $this->assertEquals(Attributes::factory(222), $spanLimits->getAttributesFactory());