Added first test, fixed style and static analysis bugs

open-telemetry · Dec 5, 2024 · ca8d1dc · ca8d1dc
1 parent f223b7f
commit ca8d1dc
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 7 deletions.
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
@@ -139,7 +139,7 @@ jobs:
       with:
         php-version: ${{ matrix.php-version }}
         coverage: xdebug
-        extensions: ast, amqp, grpc, opentelemetry, rdkafka
+        extensions: ast, amqp, grpc, opentelemetry, rdkafka, mysqli
 
     - name: Validate composer.json and composer.lock
       run: composer validate
@@ -199,6 +199,11 @@ jobs:
       run: |
         KAFKA_ADVERTISED_LISTENERS=PLAINTEXT://localhost:29092,PLAINTEXT_HOST://localhost:9092 docker compose up  kafka -d --wait
 
+    - name: Start Mysql
+      if: ${{ matrix.project == 'Instrumentation/MySqli' }}
+      run: |
+        docker compose up  mysql -d --wait
+
     - name: Run PHPUnit
       working-directory: src/${{ matrix.project }}
       run: vendor/bin/phpunit

diff --git a/docker-compose.yaml b/docker-compose.yaml
@@ -14,6 +14,8 @@ services:
       PHP_IDE_CONFIG: ${PHP_IDE_CONFIG:-''}
       RABBIT_HOST: ${RABBIT_HOST:-rabbitmq}
       KAFKA_HOST: ${KAFKA_HOST:-kafka}
+      MYSQL_HOST: ${MYSQL_HOST:-mysql}
+
 
   zipkin:
     image: openzipkin/zipkin-slim
@@ -61,4 +63,20 @@ services:
     volumes:
       - ./docker/kafka/update_run.sh:/tmp/update_run.sh
 
-
+  mysql:
+    image: mysql:8.0
+    hostname: mysql
+    ports:
+      - "3306:3306/tcp"
+    environment:
+      MYSQL_ROOT_PASSWORD: root_password
+      MYSQL_DATABASE: otel_db
+      MYSQL_USER: otel_user
+      MYSQL_PASSWORD: otel_passwd
+    healthcheck:
+      test: mysql -uotel_user -potel_passwd -e "USE otel_db;"
+      interval: 30s
+      timeout: 30s
+      retries: 3
+    volumes:
+      - ./docker/mysql/init.sql:/docker-entrypoint-initdb.d/init.sql
diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -7,6 +7,7 @@ RUN install-php-extensions \
         opentelemetry \
         mongodb \
         amqp \
-        rdkafka
+        rdkafka \
+        mysqli
 
 USER php
diff --git a/src/Instrumentation/MySqli/src/MySqliInstrumentation.php b/src/Instrumentation/MySqli/src/MySqliInstrumentation.php
@@ -338,8 +338,10 @@ public static function register(): void
         //TODO test to https://www.php.net/manual/en/mysqli.begin-transaction.php
     }
 
+    /** @param non-empty-string $spanName */
     private static function constructPreHook(string $spanName, int $paramsOffset, CachedInstrumentation $instrumentation, MySqliTracker $tracker, $obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno): void
     {
+        $attributes = [];
         $attributes[TraceAttributes::SERVER_ADDRESS] = $params[$paramsOffset + 0] ?? get_cfg_var('mysqli.default_host');
         $attributes[TraceAttributes::SERVER_PORT] = $params[$paramsOffset + 4] ?? get_cfg_var('mysqli.default_port');
         $attributes[TraceAttributes::DB_USER] = $params[$paramsOffset + 1] ?? get_cfg_var('mysqli.default_user');
@@ -370,6 +372,7 @@ private static function constructPostHook(int $paramsOffset, CachedInstrumentati
 
     }
 
+    /** @param non-empty-string $spanName */
     private static function queryPreHook(string $spanName, CachedInstrumentation $instrumentation, MySqliTracker $tracker, $obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno): void
     {
         self::startSpan($spanName, $instrumentation, $class, $function, $filename, $lineno, []);
@@ -421,6 +424,7 @@ private static function multiQueryPostHook(CachedInstrumentation $instrumentatio
 
     }
 
+    /** @param non-empty-string $spanName */
     private static function nextResultPreHook(string $spanName, CachedInstrumentation $instrumentation, MySqliTracker $tracker, $obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno): void
     {
         $span = self::startSpan($spanName, $instrumentation, $class, $function, $filename, $lineno, []);
@@ -545,6 +549,7 @@ private static function stmtConstructPostHook(CachedInstrumentation $instrumenta
         }
     }
 
+    /** @param non-empty-string $spanName */
     private static function stmtExecutePreHook(string $spanName, CachedInstrumentation $instrumentation, MySqliTracker $tracker, $obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno): void
     {
         self::startSpan($spanName, $instrumentation, $class, $function, $filename, $lineno, []);
@@ -568,6 +573,7 @@ private static function stmtExecutePostHook(CachedInstrumentation $instrumentati
 
     }
 
+    /** @param non-empty-string $spanName */
     private static function stmtNextResultPreHook(string $spanName, CachedInstrumentation $instrumentation, MySqliTracker $tracker, $obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno): void
     {
         $span = self::startSpan($spanName, $instrumentation, $class, $function, $filename, $lineno, []);
@@ -603,6 +609,7 @@ private static function stmtNextResultPostHook(CachedInstrumentation $instrument
         self::endSpan($attributes, $exception, $errorStatus);
     }
 
+    /** @param non-empty-string $spanName */
     private static function startSpan(string $spanName, CachedInstrumentation $instrumentation, ?string $class, ?string $function, ?string $filename, ?int $lineno, iterable $attributes) : SpanInterface
     {
         $parent = Context::getCurrent();
@@ -624,7 +631,7 @@ private static function startSpan(string $spanName, CachedInstrumentation $instr
         return $span;
     }
 
-    private static function endSpan(array $attributes, ?\Throwable $exception, ?string $errorStatus)
+    private static function endSpan(iterable $attributes, ?\Throwable $exception, ?string $errorStatus)
     {
         $scope = Context::storage()->scope();
         if (!$scope) {
@@ -661,6 +668,7 @@ private static function dropSpan()
     private static function extractQueryCommand($query) : ?string
     {
         $query = preg_replace("/\r\n|\n\r|\r/", "\n", $query);
+        /** @psalm-suppress PossiblyInvalidArgument */
         if (preg_match('/^\s*(?:--[^\n]*\n|\/\*[\s\S]*?\*\/\s*)*([a-zA-Z_][a-zA-Z0-9_]*)/i', $query, $matches)) {
             return strtoupper($matches[1]);
         }

diff --git a/src/Instrumentation/MySqli/src/MySqliTracker.php b/src/Instrumentation/MySqli/src/MySqliTracker.php
@@ -48,6 +48,7 @@ public function getNextMySqliMultiQuery(mysqli $mysqli) : ?string
 
     public function storeMySqliAttributes(mysqli $mysqli, ?string $hostname = null, ?string $username = null, ?string $database = null, ?int $port = null, ?string $socket = null)
     {
+        $attributes = [];
         $attributes[TraceAttributes::DB_SYSTEM] = 'mysql';
         $attributes[TraceAttributes::SERVER_ADDRESS] = $hostname ?? get_cfg_var('mysqli.default_host');
         $attributes[TraceAttributes::SERVER_PORT] = $port ?? get_cfg_var('mysqli.default_port');
@@ -66,7 +67,7 @@ public function addMySqliAttribute($mysqli, string $attribute, bool|int|float|st
         $this->mySqliToAttributes[$mysqli][$attribute] = $value;
     }
 
-    public function getMySqliAttributes(mysqli $mysqli) : iterable
+    public function getMySqliAttributes(mysqli $mysqli) : array
     {
         return $this->mySqliToAttributes[$mysqli] ?? [];
     }
@@ -76,7 +77,7 @@ public function trackMySqliFromStatement(mysqli $mysqli, mysqli_stmt $mysqli_stm
         $this->statementToMySqli[$mysqli_stmt] = WeakReference::create($mysqli);
     }
 
-    public function getMySqliAttributesFromStatement(mysqli_stmt $stmt) : iterable
+    public function getMySqliAttributesFromStatement(mysqli_stmt $stmt) : array
     {
         $mysqli = ($this->statementToMySqli[$stmt] ?? null)?->get();
         if (!$mysqli) {
@@ -94,7 +95,7 @@ public function addStatementAttribute(mysqli_stmt $stmt, string $attribute, bool
         $this->statementAttributes[$stmt][$attribute] = $value;
     }
 
-    public function getStatementAttributes(mysqli_stmt $stmt) : iterable
+    public function getStatementAttributes(mysqli_stmt $stmt) : array
     {
         if (!$this->statementAttributes->offsetExists($stmt)) {
             return [];

diff --git a/src/Instrumentation/MySqli/tests/Integration/MySqliInstrumentationTest.php b/src/Instrumentation/MySqli/tests/Integration/MySqliInstrumentationTest.php
@@ -5,13 +5,15 @@
 namespace OpenTelemetry\Tests\Instrumentation\MySqli\Integration;
 
 use ArrayObject;
+use mysqli;
 use OpenTelemetry\API\Instrumentation\Configurator;
 use OpenTelemetry\API\Trace\Propagation\TraceContextPropagator;
 use OpenTelemetry\Context\ScopeInterface;
 use OpenTelemetry\SDK\Trace\ImmutableSpan;
 use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter;
 use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor;
 use OpenTelemetry\SDK\Trace\TracerProvider;
+use OpenTelemetry\SemConv\TraceAttributes;
 use PHPUnit\Framework\TestCase;
 
 class MySqliInstrumentationTest extends TestCase
@@ -20,6 +22,12 @@ class MySqliInstrumentationTest extends TestCase
     /** @var ArrayObject<int, ImmutableSpan> */
     private ArrayObject $storage;
 
+    private string $mysqlHost;
+
+    private string $user;
+    private string $passwd;
+    private string $database;
+
     public function setUp(): void
     {
         $this->storage = new ArrayObject();
@@ -33,6 +41,12 @@ public function setUp(): void
             ->withTracerProvider($tracerProvider)
             ->withPropagator(TraceContextPropagator::getInstance())
             ->activate();
+
+        $this->mysqlHost = getenv('MYSQL_HOST') ?: 'localhost';
+
+        $this->user = 'otel_user';
+        $this->passwd = 'otel_passwd';
+        $this->database = 'otel_db';
     }
 
     public function tearDown(): void
@@ -42,6 +56,37 @@ public function tearDown(): void
 
     public function test_mysqli_connect(): void
     {
+        $mysqli = new mysqli($this->mysqlHost, $this->user, $this->passwd, $this->database);
+        $mysqli->connect($this->mysqlHost, $this->user, $this->passwd, $this->database);
+        mysqli_connect($this->mysqlHost, $this->user, $this->passwd, $this->database);
+
+        $mysqli->real_connect($this->mysqlHost, $this->user, $this->passwd, $this->database);
+        mysqli_real_connect($mysqli, $this->mysqlHost, $this->user, $this->passwd, $this->database);
+
+        $this->assertCount(5, $this->storage);
+
+        $span = $this->storage->offsetGet(0);
+        $this->assertSame('mysqli::__construct', $span->getName());
+
+        $span = $this->storage->offsetGet(1);
+        $this->assertSame('mysqli::connect', $span->getName());
+
+        $span = $this->storage->offsetGet(2);
+        $this->assertSame('mysqli_connect', $span->getName());
+
+        $span = $this->storage->offsetGet(3);
+        $this->assertSame('mysqli::real_connect', $span->getName());
+
+        $span = $this->storage->offsetGet(4);
+        $this->assertSame('mysqli_real_connect', $span->getName());
+
+        for ($i = 0; $i < 5; $i++) {
+            $span = $this->storage->offsetGet($i);
+            $this->assertEquals($this->mysqlHost, $span->getAttributes()->get(TraceAttributes::SERVER_ADDRESS));
+            $this->assertEquals($this->user, $span->getAttributes()->get(TraceAttributes::DB_USER));
+            $this->assertEquals($this->database, $span->getAttributes()->get(TraceAttributes::DB_NAMESPACE));
+            $this->assertEquals('mysql', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM));
+        }
     }
 
     // to be continued