From bc08d15853d3ba267e8cd9abbefcb8834400e532 Mon Sep 17 00:00:00 2001 From: John Sorial Date: Thu, 19 Sep 2024 14:14:49 +0200 Subject: [PATCH 1/4] Explicitly convert query to utf-8 --- composer.json | 3 ++- src/Instrumentation/PDO/src/PDOInstrumentation.php | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 7ac30bc1f..38cc232b7 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,8 @@ "prefer-stable": true, "require": { "php": "^7.4 || ^8.0", - "ext-json": "*" + "ext-json": "*", + "ext-mbstring": "*" }, "require-dev": { "composer/xdebug-handler": "^2.0", diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 16ae59ee7..51e7e4334 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -70,7 +70,7 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_STATEMENT, $params[0] ?? 'undefined'); + $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0], 'UTF-8') ?? 'undefined'); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -93,7 +93,7 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_STATEMENT, $params[0] ?? 'undefined'); + $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0], 'UTF-8') ?? 'undefined'); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -116,7 +116,7 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_STATEMENT, $params[0] ?? 'undefined'); + $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0], 'UTF-8') ?? 'undefined'); } $parent = Context::getCurrent(); $span = $builder->startSpan(); From ea65da810a8b893d040ab5812d4ee7ab3e9e7c9d Mon Sep 17 00:00:00 2001 From: John Sorial Date: Thu, 26 Sep 2024 10:08:53 +0200 Subject: [PATCH 2/4] Fix left side is not undefined --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 51e7e4334..312c3104f 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -70,7 +70,7 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0], 'UTF-8') ?? 'undefined'); + $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -93,7 +93,7 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0], 'UTF-8') ?? 'undefined'); + $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -116,7 +116,7 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0], 'UTF-8') ?? 'undefined'); + $builder->setAttribute(TraceAttributes::DB_STATEMENT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); } $parent = Context::getCurrent(); $span = $builder->startSpan(); From b07db4dedfef0656327fdbcf701827cd2381767b Mon Sep 17 00:00:00 2001 From: John Sorial Date: Fri, 27 Sep 2024 12:29:08 +0200 Subject: [PATCH 3/4] Add a polyfill and suggest a ext-mbstring --- composer.json | 3 +-- src/Instrumentation/PDO/composer.json | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 38cc232b7..7ac30bc1f 100644 --- a/composer.json +++ b/composer.json @@ -7,8 +7,7 @@ "prefer-stable": true, "require": { "php": "^7.4 || ^8.0", - "ext-json": "*", - "ext-mbstring": "*" + "ext-json": "*" }, "require-dev": { "composer/xdebug-handler": "^2.0", diff --git a/src/Instrumentation/PDO/composer.json b/src/Instrumentation/PDO/composer.json index 36025ebbc..501947f2b 100644 --- a/src/Instrumentation/PDO/composer.json +++ b/src/Instrumentation/PDO/composer.json @@ -13,7 +13,11 @@ "ext-pdo": "*", "ext-opentelemetry": "*", "open-telemetry/api": "^1.0", - "open-telemetry/sem-conv": "^1.24" + "open-telemetry/sem-conv": "^1.24", + "symfony/polyfill-mbstring": "^1.31" + }, + "suggest": { + "ext-mbstring": "For better performance than symfony/polyfill-mbstring" }, "require-dev": { "friendsofphp/php-cs-fixer": "^3", From 7b52281d387bd055d88e49edc37de2020ab2393a Mon Sep 17 00:00:00 2001 From: John Sorial Date: Fri, 27 Sep 2024 14:15:11 +0200 Subject: [PATCH 4/4] Add tests that check for utf-8 encoding --- .../Integration/PDOInstrumentationTest.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index 2fb65d95e..3fcd1b3c4 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -187,4 +187,31 @@ public function test_execute_and_fetch_all_spans_linked_to_prepared_statement_sp $this->assertCount(1, $fetchAllSpan->getLinks()); $this->assertEquals($prepareSpan->getContext(), $fetchAllSpan->getLinks()[0]->getSpanContext()); } + + /** + * Tests that the db statement is encoded as UTF-8, this is relevant for grpc exporter which expects UTF-8 encoding. + */ + public function test_encode_db_statement_as_utf8(): void + { + //setup + $db = self::createDB(); + $db->exec($this->fillDB()); + + $non_utf8_id = mb_convert_encoding('rückwärts', 'ISO-8859-1', 'UTF-8'); + + $db->prepare("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); + $span_db_prepare = $this->storage->offsetGet(2); + $this->assertTrue(mb_check_encoding($span_db_prepare->getAttributes()->get(TraceAttributes::DB_STATEMENT), 'UTF-8')); + $this->assertCount(3, $this->storage); + + $db->query("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); + $span_db_query = $this->storage->offsetGet(3); + $this->assertTrue(mb_check_encoding($span_db_query->getAttributes()->get(TraceAttributes::DB_STATEMENT), 'UTF-8')); + $this->assertCount(4, $this->storage); + + $db->exec("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); + $span_db_exec = $this->storage->offsetGet(4); + $this->assertTrue(mb_check_encoding($span_db_exec->getAttributes()->get(TraceAttributes::DB_STATEMENT), 'UTF-8')); + $this->assertCount(5, $this->storage); + } }