From d077d3b502cbedc8a155cbe3801721dfc7441208 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 11 Jul 2025 13:35:15 -0700 Subject: [PATCH 01/40] Added ServiceNamePropagator --- .github/dependabot.yml | 1 + .github/workflows/php.yml | 1 + .gitsplit.yml | 2 + composer.json | 2 + src/Propagation/ServiceName/.gitattributes | 12 + src/Propagation/ServiceName/.gitignore | 1 + src/Propagation/ServiceName/.phan/config.php | 371 ++++++++++++++++++ src/Propagation/ServiceName/.php-cs-fixer.php | 43 ++ src/Propagation/ServiceName/README.md | 63 +++ src/Propagation/ServiceName/_register.php | 15 + src/Propagation/ServiceName/composer.json | 42 ++ src/Propagation/ServiceName/phpstan.neon.dist | 9 + src/Propagation/ServiceName/phpunit.xml.dist | 44 +++ src/Propagation/ServiceName/psalm.xml.dist | 15 + .../ServiceName/src/ServiceNamePropagator.php | 76 ++++ .../tests/Unit/ServiceNamePropagatorTest.php | 52 +++ 16 files changed, 749 insertions(+) create mode 100644 src/Propagation/ServiceName/.gitattributes create mode 100644 src/Propagation/ServiceName/.gitignore create mode 100644 src/Propagation/ServiceName/.phan/config.php create mode 100644 src/Propagation/ServiceName/.php-cs-fixer.php create mode 100644 src/Propagation/ServiceName/README.md create mode 100644 src/Propagation/ServiceName/_register.php create mode 100644 src/Propagation/ServiceName/composer.json create mode 100644 src/Propagation/ServiceName/phpstan.neon.dist create mode 100644 src/Propagation/ServiceName/phpunit.xml.dist create mode 100644 src/Propagation/ServiceName/psalm.xml.dist create mode 100644 src/Propagation/ServiceName/src/ServiceNamePropagator.php create mode 100644 src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 2c1b19f45..cc462bf47 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -95,6 +95,7 @@ updates: - "/src/Propagation/CloudTrace" - "/src/Propagation/Instana" - "/src/Propagation/ServerTiming" + - "/src/Propagation/ServiceName" - "/src/Propagation/TraceResponse" - "/src/ResourceDetectors/Azure" - "/src/ResourceDetectors/Container" diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index a84df999c..c4de82fe9 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -57,6 +57,7 @@ jobs: 'Propagation/CloudTrace', 'Propagation/Instana', 'Propagation/ServerTiming', + 'Propagation/ServiceName', 'Propagation/TraceResponse', 'ResourceDetectors/Azure', 'ResourceDetectors/Container', diff --git a/.gitsplit.yml b/.gitsplit.yml index ba61b1426..ed02a15b6 100644 --- a/.gitsplit.yml +++ b/.gitsplit.yml @@ -78,6 +78,8 @@ splits: target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-instana.git" - prefix: "src/Propagation/ServerTiming" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-server-timing.git" + - prefix: "src/Propagation/ServiceName" + target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-service-name.git" - prefix: "src/Propagation/TraceResponse" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-traceresponse.git" - prefix: "src/ResourceDetectors/Azure" diff --git a/composer.json b/composer.json index 3c64f2e40..1544ec097 100644 --- a/composer.json +++ b/composer.json @@ -48,6 +48,7 @@ "OpenTelemetry\\Contrib\\Propagation\\CloudTrace\\": "src/Propagation/CloudTrace/src", "OpenTelemetry\\Contrib\\Propagation\\Instana\\": "src/Propagation/Instana/src", "OpenTelemetry\\Contrib\\Propagation\\ServerTiming\\": "src/Propagation/ServerTiming/src", + "OpenTelemetry\\Contrib\\Propagation\\ServiceName\\": "src/Propagation/ServiceName/src", "OpenTelemetry\\Contrib\\Propagation\\TraceResponse\\": "src/Propagation/TraceResponse/src", "OpenTelemetry\\Contrib\\Resource\\Detector\\Azure\\": "src/ResourceDetectors/Azure/src", "OpenTelemetry\\Contrib\\Resource\\Detector\\Container\\": "src/ResourceDetectors/Container/src", @@ -166,6 +167,7 @@ "open-telemetry/opentelemetry-propagation-cloudtrace": "self.version", "open-telemetry/opentelemetry-propagation-instana": "self.version", "open-telemetry/opentelemetry-propagation-server-timing": "self.version", + "open-telemetry/opentelemetry-propagation-service-name": "self.version", "open-telemetry/opentelemetry-propagation-traceresponse": "self.version", "open-telemetry/opentracing-shim": "self.version", "open-telemetry/sampler-rule-based": "self.version", diff --git a/src/Propagation/ServiceName/.gitattributes b/src/Propagation/ServiceName/.gitattributes new file mode 100644 index 000000000..1676cf825 --- /dev/null +++ b/src/Propagation/ServiceName/.gitattributes @@ -0,0 +1,12 @@ +* text=auto + +*.md diff=markdown +*.php diff=php + +/.gitattributes export-ignore +/.gitignore export-ignore +/.php-cs-fixer.php export-ignore +/phpstan.neon.dist export-ignore +/phpunit.xml.dist export-ignore +/psalm.xml.dist export-ignore +/tests export-ignore diff --git a/src/Propagation/ServiceName/.gitignore b/src/Propagation/ServiceName/.gitignore new file mode 100644 index 000000000..57872d0f1 --- /dev/null +++ b/src/Propagation/ServiceName/.gitignore @@ -0,0 +1 @@ +/vendor/ diff --git a/src/Propagation/ServiceName/.phan/config.php b/src/Propagation/ServiceName/.phan/config.php new file mode 100644 index 000000000..da2ac2d99 --- /dev/null +++ b/src/Propagation/ServiceName/.phan/config.php @@ -0,0 +1,371 @@ + '8.0', + + // If enabled, missing properties will be created when + // they are first seen. If false, we'll report an + // error message if there is an attempt to write + // to a class property that wasn't explicitly + // defined. + 'allow_missing_properties' => false, + + // If enabled, null can be cast to any type and any + // type can be cast to null. Setting this to true + // will cut down on false positives. + 'null_casts_as_any_type' => false, + + // If enabled, allow null to be cast as any array-like type. + // + // This is an incremental step in migrating away from `null_casts_as_any_type`. + // If `null_casts_as_any_type` is true, this has no effect. + 'null_casts_as_array' => true, + + // If enabled, allow any array-like type to be cast to null. + // This is an incremental step in migrating away from `null_casts_as_any_type`. + // If `null_casts_as_any_type` is true, this has no effect. + 'array_casts_as_null' => true, + + // If enabled, scalars (int, float, bool, string, null) + // are treated as if they can cast to each other. + // This does not affect checks of array keys. See `scalar_array_key_cast`. + 'scalar_implicit_cast' => false, + + // If enabled, any scalar array keys (int, string) + // are treated as if they can cast to each other. + // E.g. `array` can cast to `array` and vice versa. + // Normally, a scalar type such as int could only cast to/from int and mixed. + 'scalar_array_key_cast' => true, + + // If this has entries, scalars (int, float, bool, string, null) + // are allowed to perform the casts listed. + // + // E.g. `['int' => ['float', 'string'], 'float' => ['int'], 'string' => ['int'], 'null' => ['string']]` + // allows casting null to a string, but not vice versa. + // (subset of `scalar_implicit_cast`) + 'scalar_implicit_partial' => [], + + // If enabled, Phan will warn if **any** type in a method invocation's object + // is definitely not an object, + // or if **any** type in an invoked expression is not a callable. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_method_checking' => false, + + // If enabled, Phan will warn if **any** type of the object expression for a property access + // does not contain that property. + 'strict_object_checking' => false, + + // If enabled, Phan will warn if **any** type in the argument's union type + // cannot be cast to a type in the parameter's expected union type. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_param_checking' => false, + + // If enabled, Phan will warn if **any** type in a property assignment's union type + // cannot be cast to a type in the property's declared union type. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_property_checking' => false, + + // If enabled, Phan will warn if **any** type in a returned value's union type + // cannot be cast to the declared return type. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_return_checking' => false, + + // If true, seemingly undeclared variables in the global + // scope will be ignored. + // + // This is useful for projects with complicated cross-file + // globals that you have no hope of fixing. + 'ignore_undeclared_variables_in_global_scope' => true, + + // Set this to false to emit `PhanUndeclaredFunction` issues for internal functions that Phan has signatures for, + // but aren't available in the codebase, or from Reflection. + // (may lead to false positives if an extension isn't loaded) + // + // If this is true(default), then Phan will not warn. + // + // Even when this is false, Phan will still infer return values and check parameters of internal functions + // if Phan has the signatures. + 'ignore_undeclared_functions_with_known_signatures' => true, + + // Backwards Compatibility Checking. This is slow + // and expensive, but you should consider running + // it before upgrading your version of PHP to a + // new version that has backward compatibility + // breaks. + // + // If you are migrating from PHP 5 to PHP 7, + // you should also look into using + // [php7cc (no longer maintained)](https://github.com/sstalle/php7cc) + // and [php7mar](https://github.com/Alexia/php7mar), + // which have different backwards compatibility checks. + 'backward_compatibility_checks' => false, + + // If true, check to make sure the return type declared + // in the doc-block (if any) matches the return type + // declared in the method signature. + 'check_docblock_signature_return_type_match' => false, + + // If true, make narrowed types from phpdoc params override + // the real types from the signature, when real types exist. + // (E.g. allows specifying desired lists of subclasses, + // or to indicate a preference for non-nullable types over nullable types) + // + // Affects analysis of the body of the method and the param types passed in by callers. + // + // (*Requires `check_docblock_signature_param_type_match` to be true*) + 'prefer_narrowed_phpdoc_param_type' => true, + + // (*Requires `check_docblock_signature_return_type_match` to be true*) + // + // If true, make narrowed types from phpdoc returns override + // the real types from the signature, when real types exist. + // + // (E.g. allows specifying desired lists of subclasses, + // or to indicate a preference for non-nullable types over nullable types) + // + // This setting affects the analysis of return statements in the body of the method and the return types passed in by callers. + 'prefer_narrowed_phpdoc_return_type' => true, + + // If enabled, check all methods that override a + // parent method to make sure its signature is + // compatible with the parent's. + // + // This check can add quite a bit of time to the analysis. + // + // This will also check if final methods are overridden, etc. + 'analyze_signature_compatibility' => true, + + // This setting maps case-insensitive strings to union types. + // + // This is useful if a project uses phpdoc that differs from the phpdoc2 standard. + // + // If the corresponding value is the empty string, + // then Phan will ignore that union type (E.g. can ignore 'the' in `@return the value`) + // + // If the corresponding value is not empty, + // then Phan will act as though it saw the corresponding UnionTypes(s) + // when the keys show up in a UnionType of `@param`, `@return`, `@var`, `@property`, etc. + // + // This matches the **entire string**, not parts of the string. + // (E.g. `@return the|null` will still look for a class with the name `the`, but `@return the` will be ignored with the below setting) + // + // (These are not aliases, this setting is ignored outside of doc comments). + // (Phan does not check if classes with these names exist) + // + // Example setting: `['unknown' => '', 'number' => 'int|float', 'char' => 'string', 'long' => 'int', 'the' => '']` + 'phpdoc_type_mapping' => [], + + // Set to true in order to attempt to detect dead + // (unreferenced) code. Keep in mind that the + // results will only be a guess given that classes, + // properties, constants and methods can be referenced + // as variables (like `$class->$property` or + // `$class->$method()`) in ways that we're unable + // to make sense of. + 'dead_code_detection' => false, + + // Set to true in order to attempt to detect unused variables. + // `dead_code_detection` will also enable unused variable detection. + // + // This has a few known false positives, e.g. for loops or branches. + 'unused_variable_detection' => false, + + // Set to true in order to attempt to detect redundant and impossible conditions. + // + // This has some false positives involving loops, + // variables set in branches of loops, and global variables. + 'redundant_condition_detection' => false, + + // If enabled, Phan will act as though it's certain of real return types of a subset of internal functions, + // even if those return types aren't available in reflection (real types were taken from php 7.3 or 8.0-dev, depending on target_php_version). + // + // Note that with php 7 and earlier, php would return null or false for many internal functions if the argument types or counts were incorrect. + // As a result, enabling this setting with target_php_version 8.0 may result in false positives for `--redundant-condition-detection` when codebases also support php 7.x. + 'assume_real_types_for_internal_functions' => false, + + // If true, this runs a quick version of checks that takes less + // time at the cost of not running as thorough + // of an analysis. You should consider setting this + // to true only when you wish you had more **undiagnosed** issues + // to fix in your code base. + // + // In quick-mode the scanner doesn't rescan a function + // or a method's code block every time a call is seen. + // This means that the problem here won't be detected: + // + // ```php + // false, + + // Enable or disable support for generic templated + // class types. + 'generic_types_enabled' => true, + + // Override to hardcode existence and types of (non-builtin) globals in the global scope. + // Class names should be prefixed with `\`. + // + // (E.g. `['_FOO' => '\FooClass', 'page' => '\PageClass', 'userId' => 'int']`) + 'globals_type_map' => [], + + // The minimum severity level to report on. This can be + // set to `Issue::SEVERITY_LOW`, `Issue::SEVERITY_NORMAL` or + // `Issue::SEVERITY_CRITICAL`. Setting it to only + // critical issues is a good place to start on a big + // sloppy mature code base. + 'minimum_severity' => Issue::SEVERITY_LOW, + + // Add any issue types (such as `'PhanUndeclaredMethod'`) + // to this deny-list to inhibit them from being reported. + 'suppress_issue_types' => [], + + // A regular expression to match files to be excluded + // from parsing and analysis and will not be read at all. + // + // This is useful for excluding groups of test or example + // directories/files, unanalyzable files, or files that + // can't be removed for whatever reason. + // (e.g. `'@Test\.php$@'`, or `'@vendor/.*/(tests|Tests)/@'`) + 'exclude_file_regex' => '@^vendor/.*/(tests?|Tests?)/@', + + // A list of files that will be excluded from parsing and analysis + // and will not be read at all. + // + // This is useful for excluding hopelessly unanalyzable + // files that can't be removed for whatever reason. + 'exclude_file_list' => [ + 'vendor/composer/composer/src/Composer/InstalledVersions.php' + ], + + // A directory list that defines files that will be excluded + // from static analysis, but whose class and method + // information should be included. + // + // Generally, you'll want to include the directories for + // third-party code (such as "vendor/") in this list. + // + // n.b.: If you'd like to parse but not analyze 3rd + // party code, directories containing that code + // should be added to the `directory_list` as well as + // to `exclude_analysis_directory_list`. + 'exclude_analysis_directory_list' => [ + 'vendor/', + 'proto/', + 'thrift/' + ], + + // Enable this to enable checks of require/include statements referring to valid paths. + 'enable_include_path_checks' => true, + + // The number of processes to fork off during the analysis + // phase. + 'processes' => 1, + + // List of case-insensitive file extensions supported by Phan. + // (e.g. `['php', 'html', 'htm']`) + 'analyzed_file_extensions' => [ + 'php', + ], + + // You can put paths to stubs of internal extensions in this config option. + // If the corresponding extension is **not** loaded, then Phan will use the stubs instead. + // Phan will continue using its detailed type annotations, + // but load the constants, classes, functions, and classes (and their Reflection types) + // from these stub files (doubling as valid php files). + // Use a different extension from php to avoid accidentally loading these. + // The `tools/make_stubs` script can be used to generate your own stubs (compatible with php 7.0+ right now) + // + // (e.g. `['xdebug' => '.phan/internal_stubs/xdebug.phan_php']`) + 'autoload_internal_extension_signatures' => [], + + // A list of plugin files to execute. + // + // Plugins which are bundled with Phan can be added here by providing their name (e.g. `'AlwaysReturnPlugin'`) + // + // Documentation about available bundled plugins can be found [here](https://github.com/phan/phan/tree/master/.phan/plugins). + // + // Alternately, you can pass in the full path to a PHP file with the plugin's implementation (e.g. `'vendor/phan/phan/.phan/plugins/AlwaysReturnPlugin.php'`) + 'plugins' => [ + 'AlwaysReturnPlugin', + 'PregRegexCheckerPlugin', + 'UnreachableCodePlugin', + ], + + // A list of directories that should be parsed for class and + // method information. After excluding the directories + // defined in `exclude_analysis_directory_list`, the remaining + // files will be statically analyzed for errors. + // + // Thus, both first-party and third-party code being used by + // your application should be included in this list. + 'directory_list' => [ + 'src', + 'vendor' + ], + + // A list of individual files to include in analysis + // with a path relative to the root directory of the + // project. + 'file_list' => [], +]; diff --git a/src/Propagation/ServiceName/.php-cs-fixer.php b/src/Propagation/ServiceName/.php-cs-fixer.php new file mode 100644 index 000000000..e35fa078c --- /dev/null +++ b/src/Propagation/ServiceName/.php-cs-fixer.php @@ -0,0 +1,43 @@ +exclude('vendor') + ->exclude('var/cache') + ->in(__DIR__); + +$config = new PhpCsFixer\Config(); +return $config->setRules([ + 'concat_space' => ['spacing' => 'one'], + 'declare_equal_normalize' => ['space' => 'none'], + 'is_null' => true, + 'modernize_types_casting' => true, + 'ordered_imports' => true, + 'php_unit_construct' => true, + 'single_line_comment_style' => true, + 'yoda_style' => false, + '@PSR2' => true, + 'array_syntax' => ['syntax' => 'short'], + 'blank_line_after_opening_tag' => true, + 'blank_line_before_statement' => true, + 'cast_spaces' => true, + 'declare_strict_types' => true, + 'type_declaration_spaces' => true, + 'include' => true, + 'lowercase_cast' => true, + 'new_with_parentheses' => true, + 'no_extra_blank_lines' => true, + 'no_leading_import_slash' => true, + 'echo_tag_syntax' => true, + 'no_unused_imports' => true, + 'no_useless_else' => true, + 'no_useless_return' => true, + 'phpdoc_order' => true, + 'phpdoc_scalar' => true, + 'phpdoc_types' => true, + 'short_scalar_cast' => true, + 'blank_lines_before_namespace' => true, + 'single_quote' => true, + 'trailing_comma_in_multiline' => true, + ]) + ->setRiskyAllowed(true) + ->setFinder($finder); + diff --git a/src/Propagation/ServiceName/README.md b/src/Propagation/ServiceName/README.md new file mode 100644 index 000000000..eda45e9f5 --- /dev/null +++ b/src/Propagation/ServiceName/README.md @@ -0,0 +1,63 @@ +[![Releases](https://img.shields.io/badge/releases-purple)](https://github.com/opentelemetry-php/contrib-propagator-service-name/releases) +[![Issues](https://img.shields.io/badge/issues-pink)](https://github.com/open-telemetry/opentelemetry-php/issues) +[![Source](https://img.shields.io/badge/source-contrib-green)](https://github.com/open-telemetry/opentelemetry-php-contrib/tree/main/src/Propagation/ServiceName) +[![Mirror](https://img.shields.io/badge/mirror-opentelemetry--php--contrib-blue)](https://github.com/opentelemetry-php/contrib-propagator-service-name) +[![Latest Version](http://poser.pugx.org/open-telemetry/opentelemetry-propagation-service-name/v/unstable)](https://packagist.org/packages/open-telemetry/opentelemetry-propagation-service-name/) +[![Stable](http://poser.pugx.org/open-telemetry/opentelemetry-propagation-service-name/v/stable)](https://packagist.org/packages/open-telemetry/opentelemetry-propagation-service-name/) + +This is a read-only subtree split of https://github.com/open-telemetry/opentelemetry-php-contrib. + +# OpenTelemetry Service Name Propagator + +This package provides a [service.name](https://opentelemetry.io/docs/specs/semconv/resource/#service) +propagator to inject the current `service.name` into Response datastructures. + +The main goal is to allow client-side technology (Real User Monitoring, HTTP Clients) to record +the server side context in order to allow referencing it. + +## Requirements + +* OpenTelemetry SDK and exporters (required to actually export traces) + +Optional: +* OpenTelemetry extension (Some instrumentations can automatically use the `TraceResponsePropagator`) + +## Usage + +Assuming there is an active `SpanContext`, you can inject it into your response as follows: + +```php +// your framework probably provides a datastructure to model HTTP responses +// and allows you to hook into the end of a request / listen to a matching event. +$response = new Response(); + +// get the current scope, bail out if none +$scope = Context::storage()->scope(); +if (null === $scope) { + return; +} + +// create a PropagationSetterInterface that knows how to inject response headers +$propagationSetter = new class implements OpenTelemetry\Context\Propagation\PropagationSetterInterface { + public function set(&$carrier, string $key, string $value) : void { + $carrier->headers->set($key, $value); + } +}; +$propagator = new ServiceNamePropagator(); +$propagator->inject($response, $propagationSetter, $scope->context()); +``` + +## Installation via composer + +```bash +$ composer require open-telemetry/opentelemetry-propagation-service-name +``` + +## Installing dependencies and executing tests + +From ServiceName subdirectory: + +```bash +$ composer install +$ ./vendor/bin/phpunit tests +``` diff --git a/src/Propagation/ServiceName/_register.php b/src/Propagation/ServiceName/_register.php new file mode 100644 index 000000000..b7842a677 --- /dev/null +++ b/src/Propagation/ServiceName/_register.php @@ -0,0 +1,15 @@ + + + + + + + src + + + + + + + + + + + + + tests/Unit + + + + diff --git a/src/Propagation/ServiceName/psalm.xml.dist b/src/Propagation/ServiceName/psalm.xml.dist new file mode 100644 index 000000000..155711712 --- /dev/null +++ b/src/Propagation/ServiceName/psalm.xml.dist @@ -0,0 +1,15 @@ + + + + + + + + + + diff --git a/src/Propagation/ServiceName/src/ServiceNamePropagator.php b/src/Propagation/ServiceName/src/ServiceNamePropagator.php new file mode 100644 index 000000000..a8c4f96c7 --- /dev/null +++ b/src/Propagation/ServiceName/src/ServiceNamePropagator.php @@ -0,0 +1,76 @@ +getResource(); + + if ($resource->getAttributes()->has(ResourceAttributes::SERVICE_NAME)) { + $setter->set($carrier, self::SERVICE_NAME, $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME)); + } + } + + /** + * @suppress PhanUndeclaredClassAttribute + */ + #[\Override] + public function extract($carrier, ?PropagationGetterInterface $getter = null, ?ContextInterface $context = null): ContextInterface + { + $context ??= Context::getCurrent(); + + return $context; + } +} diff --git a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php new file mode 100644 index 000000000..4406d2039 --- /dev/null +++ b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php @@ -0,0 +1,52 @@ +serviceNamePropagator = ServiceNamePropagator::getInstance(); + } + + public function test_fields(): void + { + $this->assertSame( + ['service.name'], + $this->serviceNamePropagator->fields() + ); + } + + public function test_inject_empty(): void + { + $carrier = []; + $this->serviceNamePropagator->inject($carrier); + $this->assertEmpty($carrier); + } + + public function test_inject(): void + { + putenv('OTEL_SERVICE_NAME=foo-service'); + $carrier = []; + $this->serviceNamePropagator->inject($carrier); + $this->assertEquals($carrier, ['service.name'=>'foo-service']); + putenv('OTEL_SERVICE_NAME'); + } + + public function test_extract_empty(): void + { + $carrier = []; + $context = $this->serviceNamePropagator->extract($carrier); + $this->assertSame(Context::getCurrent(), $context); + } +} From 4859d6b2e73187900ee06720cc8dd9708d196c33 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 11 Jul 2025 19:13:36 -0700 Subject: [PATCH 02/40] Updated PDO --- src/Instrumentation/PDO/src/Opentelemetry.php | 47 ++++++++++++++++++ .../PDO/src/PDOInstrumentation.php | 45 +++++++++++++++-- src/Instrumentation/PDO/src/Utils.php | 49 +++++++++++++++++++ .../PDO/tests/Unit/UtilsTest.php | 48 ++++++++++++++++++ 4 files changed, 186 insertions(+), 3 deletions(-) create mode 100644 src/Instrumentation/PDO/src/Opentelemetry.php create mode 100644 src/Instrumentation/PDO/src/Utils.php create mode 100644 src/Instrumentation/PDO/tests/Unit/UtilsTest.php diff --git a/src/Instrumentation/PDO/src/Opentelemetry.php b/src/Instrumentation/PDO/src/Opentelemetry.php new file mode 100644 index 000000000..ac2a71f30 --- /dev/null +++ b/src/Instrumentation/PDO/src/Opentelemetry.php @@ -0,0 +1,47 @@ +inject($carrier); + + return $carrier; + } + + public static function getServiceNameValues() + { + $carrier = []; + + if (class_exists('OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator')) { + /** @phan-suppress-next-line PhanUndeclaredClassMethod */ + $trace = new \OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator(); + /** @phan-suppress-next-line PhanAccessMethodInternal,PhanUndeclaredClassMethod */ + $trace->inject($carrier); + } + + return $carrier; + } +} diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index a739d9e38..079f1d259 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -112,8 +112,9 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -122,6 +123,18 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { + $sqlStatement = self::appendSqlComments($sqlStatement); + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + + return [ + 0 => $sqlStatement, + ]; + } + + return []; }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { self::end($exception); @@ -135,8 +148,9 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -145,6 +159,18 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { + $sqlStatement = self::appendSqlComments($sqlStatement); + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + + return [ + 0 => $sqlStatement, + ]; + } + + return []; }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { self::end($exception); @@ -158,8 +184,9 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -168,6 +195,18 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { + $sqlStatement = self::appendSqlComments($sqlStatement, false); + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + + return [ + 0 => $sqlStatement, + ]; + } + + return []; }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { if ($statement instanceof PDOStatement) { diff --git a/src/Instrumentation/PDO/src/Utils.php b/src/Instrumentation/PDO/src/Utils.php new file mode 100644 index 000000000..f0f869999 --- /dev/null +++ b/src/Instrumentation/PDO/src/Utils.php @@ -0,0 +1,49 @@ + Utils::customUrlEncode($key) . "='" . Utils::customUrlEncode($value) . "'", + $comments, + array_keys($comments) + ), + ) . '*/'; + } + + private static function customUrlEncode(string $input): string + { + $encodedString = urlencode($input); + + // Since SQL uses '%' as a keyword, '%' is a by-product of url quoting + // e.g. foo,bar --> foo%2Cbar + // thus in our quoting, we need to escape it too to finally give + // foo,bar --> foo%%2Cbar + + return str_replace('%', '%%', $encodedString); + } +} diff --git a/src/Instrumentation/PDO/tests/Unit/UtilsTest.php b/src/Instrumentation/PDO/tests/Unit/UtilsTest.php new file mode 100644 index 000000000..2c2be2161 --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/UtilsTest.php @@ -0,0 +1,48 @@ +assertEquals("/*key1='value1',key2='value2'*/", Utils::formatComments(['key1' => 'value1', 'key2' => 'value2'])); + } + + public function testFormatCommentsWithoutKeys(): void + { + $this->assertEquals('', Utils::formatComments([])); + } + + public function testFormatCommentsWithSpecialCharKeys(): void + { + $this->assertEquals("/*key1='value1%%40',key2='value2'*/", Utils::formatComments(['key1' => 'value1@', 'key2' => 'value2'])); + } + + public function testFormatCommentsWithPlaceholder(): void + { + $this->assertEquals("/*key1='value1%%3F',key2='value2'*/", Utils::formatComments(['key1' => 'value1?', 'key2' => 'value2'])); + } + + public function testFormatCommentsWithNamedPlaceholder(): void + { + $this->assertEquals("/*key1='%%3Anamed',key2='value2'*/", Utils::formatComments(['key1' => ':named', 'key2' => 'value2'])); + } +} From 6145de216f9e6d106e33c389ef416bf76a78997a Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 11 Jul 2025 19:40:43 -0700 Subject: [PATCH 03/40] Updated --- .../PDO/src/PDOInstrumentation.php | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 079f1d259..fa83ebae4 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -124,7 +124,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { - $sqlStatement = self::appendSqlComments($sqlStatement); + $sqlStatement = self::appendSqlComments($sqlStatement, true); $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, ]); @@ -160,7 +160,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { - $sqlStatement = self::appendSqlComments($sqlStatement); + $sqlStatement = self::appendSqlComments($sqlStatement, true); $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, ]); @@ -369,4 +369,29 @@ private static function isDistributeStatementToLinkedSpansEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.distribute_statement_to_linked_spans'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } + + private static function isSqlCommenterEnabled(): bool + { + if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { + return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER', false); + } + + return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; + } + + private static function appendSqlComments(string $query, bool $withTraceContext): string + { + $comments = []; + if ($withTraceContext) { + $comments = Opentelemetry::getTraceContextValues(); + } + foreach (Opentelemetry::getServiceNameValues() as $key => $value) { + $comments[$key] = $value; + } + $query = trim($query); + $hasSemicolon = $query[-1] === ';'; + $query = rtrim($query, ';'); + + return $query . Utils::formatComments(array_filter($comments)) . ($hasSemicolon ? ';' : ''); + } } From 62f8ebd05f53f85074da63262c07dd17958b0e40 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 11 Jul 2025 19:55:16 -0700 Subject: [PATCH 04/40] Updated --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index fa83ebae4..c4143c4e7 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -389,7 +389,7 @@ private static function appendSqlComments(string $query, bool $withTraceContext) $comments[$key] = $value; } $query = trim($query); - $hasSemicolon = $query[-1] === ';'; + $hasSemicolon = $query !== '' && $query[strlen($query) - 1] === ';'; $query = rtrim($query, ';'); return $query . Utils::formatComments(array_filter($comments)) . ($hasSemicolon ? ';' : ''); From a33724599ce66b511e6cc2dbd90ebf6c83b32ce0 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 11 Jul 2025 20:26:07 -0700 Subject: [PATCH 05/40] Fix --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index c4143c4e7..792d48eae 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -113,6 +113,9 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + if (!is_string($sqlStatement)) { + $sqlStatement = 'undefined'; + } if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); } @@ -149,6 +152,9 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + if (!is_string($sqlStatement)) { + $sqlStatement = 'undefined'; + } if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); } @@ -185,6 +191,9 @@ public static function register(): void $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + if (!is_string($sqlStatement)) { + $sqlStatement = 'undefined'; + } if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); } From f29504c01485128f7002b690fecb6affcce58cf5 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Mon, 14 Jul 2025 09:36:44 -0700 Subject: [PATCH 06/40] Added tests --- .../ServiceName/tests/Unit/ServiceNamePropagatorTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php index 4406d2039..bbf84ae0e 100644 --- a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php +++ b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php @@ -49,4 +49,11 @@ public function test_extract_empty(): void $context = $this->serviceNamePropagator->extract($carrier); $this->assertSame(Context::getCurrent(), $context); } + + public function test_no_extract(): void + { + $carrier = ['service.name' => 'foo-service']; + $context = $this->serviceNamePropagator->extract($carrier); + $this->assertSame(Context::getCurrent(), $context); + } } From 0c5446583b6b10d1cbfee95a23ddbbdc61b367ec Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Mon, 14 Jul 2025 14:40:56 -0700 Subject: [PATCH 07/40] Added opentelemetry test --- .../PDO/tests/Unit/OpentelemetryTest.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php diff --git a/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php b/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php new file mode 100644 index 000000000..a7b10f276 --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php @@ -0,0 +1,30 @@ +assertIsArray($result); + } + + public function testGetServiceNameValuesReturnsArray() + { + $result = Opentelemetry::getServiceNameValues(); + $this->assertIsArray($result); + } + + public function testGetServiceNameValuesHandlesMissingClass() + { + // Temporarily ensure the class does not exist + if (class_exists('OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator')) { + $this->markTestSkipped('ServiceNamePropagator class exists, cannot test missing class branch.'); + } + $result = Opentelemetry::getServiceNameValues(); + $this->assertIsArray($result); + $this->assertEmpty($result); + } +} From bb12ed3ba8e07a9ef59551a8b7a4bb3234dcb295 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Tue, 15 Jul 2025 14:22:16 -0700 Subject: [PATCH 08/40] Update src/Instrumentation/PDO/src/Utils.php Co-authored-by: Brett McBride --- src/Instrumentation/PDO/src/Utils.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Instrumentation/PDO/src/Utils.php b/src/Instrumentation/PDO/src/Utils.php index f0f869999..850b34777 100644 --- a/src/Instrumentation/PDO/src/Utils.php +++ b/src/Instrumentation/PDO/src/Utils.php @@ -1,18 +1,5 @@ Date: Tue, 15 Jul 2025 14:22:59 -0700 Subject: [PATCH 09/40] Update src/Instrumentation/PDO/src/Opentelemetry.php Co-authored-by: Brett McBride --- src/Instrumentation/PDO/src/Opentelemetry.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Instrumentation/PDO/src/Opentelemetry.php b/src/Instrumentation/PDO/src/Opentelemetry.php index ac2a71f30..e4cf34a8f 100644 --- a/src/Instrumentation/PDO/src/Opentelemetry.php +++ b/src/Instrumentation/PDO/src/Opentelemetry.php @@ -1,18 +1,5 @@ Date: Tue, 15 Jul 2025 14:29:02 -0700 Subject: [PATCH 10/40] Addressed service.name issue --- src/Propagation/ServiceName/src/ServiceNamePropagator.php | 6 ++---- .../ServiceName/tests/Unit/ServiceNamePropagatorTest.php | 7 ++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Propagation/ServiceName/src/ServiceNamePropagator.php b/src/Propagation/ServiceName/src/ServiceNamePropagator.php index a8c4f96c7..d52ab9511 100644 --- a/src/Propagation/ServiceName/src/ServiceNamePropagator.php +++ b/src/Propagation/ServiceName/src/ServiceNamePropagator.php @@ -20,10 +20,8 @@ */ final class ServiceNamePropagator implements TextMapPropagatorInterface { - private const SERVICE_NAME = 'service.name'; - private const FIELDS = [ - self::SERVICE_NAME, + ResourceAttributes::SERVICE_NAME, ]; private static ?self $instance = null; @@ -59,7 +57,7 @@ public function inject(&$carrier, ?PropagationSetterInterface $setter = null, ?C $resource = $detector->getResource(); if ($resource->getAttributes()->has(ResourceAttributes::SERVICE_NAME)) { - $setter->set($carrier, self::SERVICE_NAME, $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME)); + $setter->set($carrier, ResourceAttributes::SERVICE_NAME, $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME)); } } diff --git a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php index bbf84ae0e..99c5c4a2d 100644 --- a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php +++ b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php @@ -6,6 +6,7 @@ use OpenTelemetry\Context\Context; use OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator; +use OpenTelemetry\SemConv\ResourceAttributes; use Override; use PHPUnit\Framework\TestCase; @@ -22,7 +23,7 @@ protected function setUp(): void public function test_fields(): void { $this->assertSame( - ['service.name'], + [ResourceAttributes::SERVICE_NAME], $this->serviceNamePropagator->fields() ); } @@ -39,7 +40,7 @@ public function test_inject(): void putenv('OTEL_SERVICE_NAME=foo-service'); $carrier = []; $this->serviceNamePropagator->inject($carrier); - $this->assertEquals($carrier, ['service.name'=>'foo-service']); + $this->assertEquals($carrier, [ResourceAttributes::SERVICE_NAME=>'foo-service']); putenv('OTEL_SERVICE_NAME'); } @@ -52,7 +53,7 @@ public function test_extract_empty(): void public function test_no_extract(): void { - $carrier = ['service.name' => 'foo-service']; + $carrier = [ResourceAttributes::SERVICE_NAME => 'foo-service']; $context = $this->serviceNamePropagator->extract($carrier); $this->assertSame(Context::getCurrent(), $context); } From 586df3fead818e67521962f5f8d2c885bd288880 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Tue, 15 Jul 2025 15:01:12 -0700 Subject: [PATCH 11/40] Update src/Instrumentation/PDO/tests/Unit/UtilsTest.php Co-authored-by: Brett McBride --- src/Instrumentation/PDO/tests/Unit/UtilsTest.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/Instrumentation/PDO/tests/Unit/UtilsTest.php b/src/Instrumentation/PDO/tests/Unit/UtilsTest.php index 2c2be2161..dc1e8ba80 100644 --- a/src/Instrumentation/PDO/tests/Unit/UtilsTest.php +++ b/src/Instrumentation/PDO/tests/Unit/UtilsTest.php @@ -1,17 +1,5 @@ Date: Tue, 15 Jul 2025 21:30:42 -0700 Subject: [PATCH 12/40] fixed style --- src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php b/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php index a7b10f276..30ca7fee7 100644 --- a/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php +++ b/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php @@ -1,7 +1,9 @@ Date: Wed, 16 Jul 2025 09:36:23 -0700 Subject: [PATCH 13/40] Making undefined as constant --- .../PDO/src/PDOInstrumentation.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 792d48eae..825626d39 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -22,6 +22,7 @@ class PDOInstrumentation { public const NAME = 'pdo'; + private const UNDEFINED = 'undefined'; public static function register(): void { @@ -112,9 +113,9 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + $sqlStatement = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); if (!is_string($sqlStatement)) { - $sqlStatement = 'undefined'; + $sqlStatement = self::UNDEFINED; } if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); @@ -126,7 +127,7 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { + if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { $sqlStatement = self::appendSqlComments($sqlStatement, true); $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -151,9 +152,9 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + $sqlStatement = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); if (!is_string($sqlStatement)) { - $sqlStatement = 'undefined'; + $sqlStatement = self::UNDEFINED; } if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); @@ -165,7 +166,7 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { + if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { $sqlStatement = self::appendSqlComments($sqlStatement, true); $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -190,9 +191,9 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $sqlStatement = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + $sqlStatement = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); if (!is_string($sqlStatement)) { - $sqlStatement = 'undefined'; + $sqlStatement = self::UNDEFINED; } if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); @@ -204,7 +205,7 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (self::isSqlCommenterEnabled() && $sqlStatement !== 'undefined') { + if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { $sqlStatement = self::appendSqlComments($sqlStatement, false); $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, From 1c2c49b9df908e52293df06bc152e290cb56f031 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 16 Jul 2025 10:56:14 -0700 Subject: [PATCH 14/40] Added opt in for sql commenter attribute and update READMD.md --- src/Instrumentation/PDO/README.md | 20 ++++++++++- .../PDO/src/PDOInstrumentation.php | 33 ++++++++++++++----- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index 7a56fbda4..e23f35754 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -33,4 +33,22 @@ otel.instrumentation.pdo.distribute_statement_to_linked_spans = true or environment variable: ```shell OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS=true -``` \ No newline at end of file +``` + +The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive: +``` +otel.instrumentation.pdo.sql_commenter = true +``` +or environment variable: +```shell +OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER=true +``` + +and its attribute can be configured using the following configuration directive: +``` +otel.instrumentation.pdo.sql_commenter_attribute = true +``` +or environment variable: +```shell +OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_ATTRIBUTE=true +``` diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 825626d39..4c9dd204b 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -129,9 +129,11 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { $sqlStatement = self::appendSqlComments($sqlStatement, true); - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (self::isSqlCommenterAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } return [ 0 => $sqlStatement, @@ -168,9 +170,11 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { $sqlStatement = self::appendSqlComments($sqlStatement, true); - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (self::isSqlCommenterAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } return [ 0 => $sqlStatement, @@ -207,9 +211,11 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { $sqlStatement = self::appendSqlComments($sqlStatement, false); - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (self::isSqlCommenterAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } return [ 0 => $sqlStatement, @@ -389,6 +395,15 @@ private static function isSqlCommenterEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } + private static function isSqlCommenterAttributeEnabled(): bool + { + if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { + return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_ATTRIBUTE', false); + } + + return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter_attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; + } + private static function appendSqlComments(string $query, bool $withTraceContext): string { $comments = []; From 3b2871be46d6fd5a1d9199de254e172c555880bf Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 16 Jul 2025 13:24:39 -0700 Subject: [PATCH 15/40] Added prepend vs append opt-in --- src/Instrumentation/PDO/README.md | 15 +++++++++--- .../PDO/src/PDOInstrumentation.php | 23 +++++++++++++++---- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index e23f35754..98f3c6283 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -35,6 +35,7 @@ or environment variable: OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS=true ``` +### SQL Commenter feature The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive: ``` otel.instrumentation.pdo.sql_commenter = true @@ -43,10 +44,18 @@ or environment variable: ```shell OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER=true ``` - -and its attribute can be configured using the following configuration directive: +This feature by default will append a SQL comment to the query statement with the information about the code that executed the query. +The SQL comment can be configured to prepend to the query statement using the following configuration directive: +``` +otel.instrumentation.pdo.sql_commenter.prepend = true +``` +or environment variable: +```shell +OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND=true +``` +The modified query statement by default will not update `TraceAttributes::DB_QUERY_TEXT` due to high cardinality risk, but it can be configured using the following configuration directive: ``` -otel.instrumentation.pdo.sql_commenter_attribute = true +otel.instrumentation.pdo.sql_commenter.attribute = true ``` or environment variable: ```shell diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 4c9dd204b..6742d995a 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -128,7 +128,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - $sqlStatement = self::appendSqlComments($sqlStatement, true); + $sqlStatement = self::addSqlComments($sqlStatement, true); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -169,7 +169,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - $sqlStatement = self::appendSqlComments($sqlStatement, true); + $sqlStatement = self::addSqlComments($sqlStatement, true); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -210,7 +210,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - $sqlStatement = self::appendSqlComments($sqlStatement, false); + $sqlStatement = self::addSqlComments($sqlStatement, false); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -395,16 +395,25 @@ private static function isSqlCommenterEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } + private static function isSqlCommenterPrepend(): bool + { + if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { + return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND', false); + } + + return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.prepend'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; + } + private static function isSqlCommenterAttributeEnabled(): bool { if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_ATTRIBUTE', false); } - return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter_attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; + return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } - private static function appendSqlComments(string $query, bool $withTraceContext): string + private static function addSqlComments(string $query, bool $withTraceContext): string { $comments = []; if ($withTraceContext) { @@ -414,9 +423,13 @@ private static function appendSqlComments(string $query, bool $withTraceContext) $comments[$key] = $value; } $query = trim($query); + if (self::isSqlCommenterPrepend()) { + return Utils::formatComments(array_filter($comments)) . $query; + } $hasSemicolon = $query !== '' && $query[strlen($query) - 1] === ';'; $query = rtrim($query, ';'); return $query . Utils::formatComments(array_filter($comments)) . ($hasSemicolon ? ';' : ''); + } } From b434b42144b7fdb2d004879ec7f5a3126fcc5312 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 16 Jul 2025 14:15:02 -0700 Subject: [PATCH 16/40] Removed Opentelemetry class --- src/Instrumentation/PDO/src/Opentelemetry.php | 34 ------------------- .../PDO/src/PDOInstrumentation.php | 11 ++++-- .../PDO/tests/Unit/OpentelemetryTest.php | 32 ----------------- 3 files changed, 8 insertions(+), 69 deletions(-) delete mode 100644 src/Instrumentation/PDO/src/Opentelemetry.php delete mode 100644 src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php diff --git a/src/Instrumentation/PDO/src/Opentelemetry.php b/src/Instrumentation/PDO/src/Opentelemetry.php deleted file mode 100644 index e4cf34a8f..000000000 --- a/src/Instrumentation/PDO/src/Opentelemetry.php +++ /dev/null @@ -1,34 +0,0 @@ -inject($carrier); - - return $carrier; - } - - public static function getServiceNameValues() - { - $carrier = []; - - if (class_exists('OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator')) { - /** @phan-suppress-next-line PhanUndeclaredClassMethod */ - $trace = new \OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator(); - /** @phan-suppress-next-line PhanAccessMethodInternal,PhanUndeclaredClassMethod */ - $trace->inject($carrier); - } - - return $carrier; - } -} diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 6742d995a..f0546f979 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; use OpenTelemetry\API\Instrumentation\CachedInstrumentation; +use OpenTelemetry\API\Trace\Propagation\TraceContextPropagator; use OpenTelemetry\API\Trace\Span; use OpenTelemetry\API\Trace\SpanBuilderInterface; use OpenTelemetry\API\Trace\SpanKind; @@ -417,10 +418,14 @@ private static function addSqlComments(string $query, bool $withTraceContext): s { $comments = []; if ($withTraceContext) { - $comments = Opentelemetry::getTraceContextValues(); + $prop = TraceContextPropagator::getInstance(); + $prop->inject($comments); } - foreach (Opentelemetry::getServiceNameValues() as $key => $value) { - $comments[$key] = $value; + if (class_exists('OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator')) { + /** @phan-suppress-next-line PhanUndeclaredClassMethod */ + $prop = new \OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator(); + /** @phan-suppress-next-line PhanAccessMethodInternal,PhanUndeclaredClassMethod */ + $prop->inject($comments); } $query = trim($query); if (self::isSqlCommenterPrepend()) { diff --git a/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php b/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php deleted file mode 100644 index 30ca7fee7..000000000 --- a/src/Instrumentation/PDO/tests/Unit/OpentelemetryTest.php +++ /dev/null @@ -1,32 +0,0 @@ -assertIsArray($result); - } - - public function testGetServiceNameValuesReturnsArray() - { - $result = Opentelemetry::getServiceNameValues(); - $this->assertIsArray($result); - } - - public function testGetServiceNameValuesHandlesMissingClass() - { - // Temporarily ensure the class does not exist - if (class_exists('OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator')) { - $this->markTestSkipped('ServiceNamePropagator class exists, cannot test missing class branch.'); - } - $result = Opentelemetry::getServiceNameValues(); - $this->assertIsArray($result); - $this->assertEmpty($result); - } -} From 8e63332890c067042dfd189a18273f14b0c526f3 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 16 Jul 2025 17:19:16 -0700 Subject: [PATCH 17/40] Added database opt-in configuration --- src/Instrumentation/PDO/README.md | 13 ++++ .../PDO/src/PDOInstrumentation.php | 78 ++++++++++++------- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index 98f3c6283..c3c30d741 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -44,6 +44,19 @@ or environment variable: ```shell OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER=true ``` +To select which database(s) to opt-in, specify the database(s) using configuration directive: + +Valid values are `postgresql`, `mysql`, `sqlite`, `mssql`, `oracle`, `db2`, `other_sql` and `all` (default). + +``` +otel.instrumentation.pdo.sql_commenter.database[]=postgresql +otel.instrumentation.pdo.sql_commenter.database[]=mysql +``` +or environment variable: +```shell +OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_DATABASE=postgresql,mysql +``` + This feature by default will append a SQL comment to the query statement with the information about the code that executed the query. The SQL comment can be configured to prepend to the query statement using the following configuration directive: ``` diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index f0546f979..81e082447 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -25,6 +25,8 @@ class PDOInstrumentation public const NAME = 'pdo'; private const UNDEFINED = 'undefined'; + private const ALL = 'all'; + public static function register(): void { $instrumentation = new CachedInstrumentation( @@ -129,16 +131,18 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - $sqlStatement = self::addSqlComments($sqlStatement, true); - if (self::isSqlCommenterAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterSupportedDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + $sqlStatement = self::addSqlComments($sqlStatement, true); + if (self::isSqlCommenterAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } + + return [ + 0 => $sqlStatement, + ]; } - - return [ - 0 => $sqlStatement, - ]; } return []; @@ -170,16 +174,18 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - $sqlStatement = self::addSqlComments($sqlStatement, true); - if (self::isSqlCommenterAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterSupportedDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + $sqlStatement = self::addSqlComments($sqlStatement, true); + if (self::isSqlCommenterAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } + + return [ + 0 => $sqlStatement, + ]; } - - return [ - 0 => $sqlStatement, - ]; } return []; @@ -211,16 +217,18 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - $sqlStatement = self::addSqlComments($sqlStatement, false); - if (self::isSqlCommenterAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterSupportedDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + $sqlStatement = self::addSqlComments($sqlStatement, false); + if (self::isSqlCommenterAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } + + return [ + 0 => $sqlStatement, + ]; } - - return [ - 0 => $sqlStatement, - ]; } return []; @@ -414,6 +422,15 @@ private static function isSqlCommenterAttributeEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } + private static function getSqlCommenterDatabase(): array + { + if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { + return Configuration::getList('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_DATABASE', [self::ALL]); + } + + return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.database'), FILTER_REQUIRE_ARRAY, FILTER_NULL_ON_FAILURE) ?? [self::ALL]; + } + private static function addSqlComments(string $query, bool $withTraceContext): string { $comments = []; @@ -437,4 +454,11 @@ private static function addSqlComments(string $query, bool $withTraceContext): s return $query . Utils::formatComments(array_filter($comments)) . ($hasSemicolon ? ';' : ''); } + + private static function isSQLCommenterSupportedDatabase(string $db) : bool + { + $supported = self::getSqlCommenterDatabase(); + + return in_array(strtolower($db), $supported) || in_array(self::ALL, $supported); + } } From db6a91580b6761f69bd076f869162f0ff8cedc0f Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 16 Jul 2025 17:32:55 -0700 Subject: [PATCH 18/40] nits --- .../PDO/src/PDOInstrumentation.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 81e082447..999346807 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -131,7 +131,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterSupportedDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { $sqlStatement = self::addSqlComments($sqlStatement, true); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ @@ -174,7 +174,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterSupportedDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { $sqlStatement = self::addSqlComments($sqlStatement, true); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ @@ -217,7 +217,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterSupportedDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { $sqlStatement = self::addSqlComments($sqlStatement, false); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ @@ -424,11 +424,11 @@ private static function isSqlCommenterAttributeEnabled(): bool private static function getSqlCommenterDatabase(): array { - if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { - return Configuration::getList('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_DATABASE', [self::ALL]); + if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration') && count($values = Configuration::getList('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_DATABASE', [])) > 0) { + return $values; } - return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.database'), FILTER_REQUIRE_ARRAY, FILTER_NULL_ON_FAILURE) ?? [self::ALL]; + return (array) (get_cfg_var('otel.instrumentation.pdo.sql_commenter.database') ?: [self::ALL]); } private static function addSqlComments(string $query, bool $withTraceContext): string @@ -455,10 +455,10 @@ private static function addSqlComments(string $query, bool $withTraceContext): s } - private static function isSQLCommenterSupportedDatabase(string $db) : bool + private static function isSQLCommenterOptInDatabase(string $db) : bool { - $supported = self::getSqlCommenterDatabase(); + $optInList = self::getSqlCommenterDatabase(); - return in_array(strtolower($db), $supported) || in_array(self::ALL, $supported); + return in_array(strtolower($db), $optInList) || in_array(self::ALL, $optInList); } } From 569e24ded021d1f6f346bbed0cf3e229007c34b3 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 16 Jul 2025 17:40:18 -0700 Subject: [PATCH 19/40] Added suppress --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 999346807..40f36a8fa 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -131,6 +131,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { + /** @psalm-suppress PossiblyInvalidCast */ if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { $sqlStatement = self::addSqlComments($sqlStatement, true); if (self::isSqlCommenterAttributeEnabled()) { @@ -174,6 +175,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { + /** @psalm-suppress PossiblyInvalidCast */ if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { $sqlStatement = self::addSqlComments($sqlStatement, true); if (self::isSqlCommenterAttributeEnabled()) { @@ -217,6 +219,7 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { + /** @psalm-suppress PossiblyInvalidCast */ if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { $sqlStatement = self::addSqlComments($sqlStatement, false); if (self::isSqlCommenterAttributeEnabled()) { From 11bab35fbed296500da08e15a73a1ce558d0c61f Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Thu, 17 Jul 2025 09:53:23 -0700 Subject: [PATCH 20/40] Removed 'all' by default --- src/Instrumentation/PDO/README.md | 2 +- src/Instrumentation/PDO/src/PDOInstrumentation.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index c3c30d741..7f9ba52de 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -46,7 +46,7 @@ OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER=true ``` To select which database(s) to opt-in, specify the database(s) using configuration directive: -Valid values are `postgresql`, `mysql`, `sqlite`, `mssql`, `oracle`, `db2`, `other_sql` and `all` (default). +Valid values are `postgresql`, `mysql`, `sqlite`, `mssql`, `oracle`, `db2`, `other_sql` and `all`. ``` otel.instrumentation.pdo.sql_commenter.database[]=postgresql diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 40f36a8fa..431b8ea00 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -431,7 +431,7 @@ private static function getSqlCommenterDatabase(): array return $values; } - return (array) (get_cfg_var('otel.instrumentation.pdo.sql_commenter.database') ?: [self::ALL]); + return (array) (get_cfg_var('otel.instrumentation.pdo.sql_commenter.database') ?: []); } private static function addSqlComments(string $query, bool $withTraceContext): string From d280ff895a51cf154c0d7eafd5a9df6659eb8b4a Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 18 Jul 2025 11:58:05 -0700 Subject: [PATCH 21/40] Revert --- .github/dependabot.yml | 1 - .github/workflows/php.yml | 1 - .gitsplit.yml | 2 - composer.json | 2 - .../PDO/src/PDOInstrumentation.php | 41 +- src/Propagation/ServiceName/.gitattributes | 12 - src/Propagation/ServiceName/.gitignore | 1 - src/Propagation/ServiceName/.phan/config.php | 371 ------------------ src/Propagation/ServiceName/.php-cs-fixer.php | 43 -- src/Propagation/ServiceName/README.md | 63 --- src/Propagation/ServiceName/_register.php | 15 - src/Propagation/ServiceName/composer.json | 42 -- src/Propagation/ServiceName/phpstan.neon.dist | 9 - src/Propagation/ServiceName/phpunit.xml.dist | 44 --- src/Propagation/ServiceName/psalm.xml.dist | 15 - .../ServiceName/src/ServiceNamePropagator.php | 74 ---- .../tests/Unit/ServiceNamePropagatorTest.php | 60 --- 17 files changed, 6 insertions(+), 790 deletions(-) delete mode 100644 src/Propagation/ServiceName/.gitattributes delete mode 100644 src/Propagation/ServiceName/.gitignore delete mode 100644 src/Propagation/ServiceName/.phan/config.php delete mode 100644 src/Propagation/ServiceName/.php-cs-fixer.php delete mode 100644 src/Propagation/ServiceName/README.md delete mode 100644 src/Propagation/ServiceName/_register.php delete mode 100644 src/Propagation/ServiceName/composer.json delete mode 100644 src/Propagation/ServiceName/phpstan.neon.dist delete mode 100644 src/Propagation/ServiceName/phpunit.xml.dist delete mode 100644 src/Propagation/ServiceName/psalm.xml.dist delete mode 100644 src/Propagation/ServiceName/src/ServiceNamePropagator.php delete mode 100644 src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php diff --git a/.github/dependabot.yml b/.github/dependabot.yml index cc462bf47..2c1b19f45 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -95,7 +95,6 @@ updates: - "/src/Propagation/CloudTrace" - "/src/Propagation/Instana" - "/src/Propagation/ServerTiming" - - "/src/Propagation/ServiceName" - "/src/Propagation/TraceResponse" - "/src/ResourceDetectors/Azure" - "/src/ResourceDetectors/Container" diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index c4de82fe9..a84df999c 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -57,7 +57,6 @@ jobs: 'Propagation/CloudTrace', 'Propagation/Instana', 'Propagation/ServerTiming', - 'Propagation/ServiceName', 'Propagation/TraceResponse', 'ResourceDetectors/Azure', 'ResourceDetectors/Container', diff --git a/.gitsplit.yml b/.gitsplit.yml index ed02a15b6..ba61b1426 100644 --- a/.gitsplit.yml +++ b/.gitsplit.yml @@ -78,8 +78,6 @@ splits: target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-instana.git" - prefix: "src/Propagation/ServerTiming" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-server-timing.git" - - prefix: "src/Propagation/ServiceName" - target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-service-name.git" - prefix: "src/Propagation/TraceResponse" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-propagator-traceresponse.git" - prefix: "src/ResourceDetectors/Azure" diff --git a/composer.json b/composer.json index 1544ec097..3c64f2e40 100644 --- a/composer.json +++ b/composer.json @@ -48,7 +48,6 @@ "OpenTelemetry\\Contrib\\Propagation\\CloudTrace\\": "src/Propagation/CloudTrace/src", "OpenTelemetry\\Contrib\\Propagation\\Instana\\": "src/Propagation/Instana/src", "OpenTelemetry\\Contrib\\Propagation\\ServerTiming\\": "src/Propagation/ServerTiming/src", - "OpenTelemetry\\Contrib\\Propagation\\ServiceName\\": "src/Propagation/ServiceName/src", "OpenTelemetry\\Contrib\\Propagation\\TraceResponse\\": "src/Propagation/TraceResponse/src", "OpenTelemetry\\Contrib\\Resource\\Detector\\Azure\\": "src/ResourceDetectors/Azure/src", "OpenTelemetry\\Contrib\\Resource\\Detector\\Container\\": "src/ResourceDetectors/Container/src", @@ -167,7 +166,6 @@ "open-telemetry/opentelemetry-propagation-cloudtrace": "self.version", "open-telemetry/opentelemetry-propagation-instana": "self.version", "open-telemetry/opentelemetry-propagation-server-timing": "self.version", - "open-telemetry/opentelemetry-propagation-service-name": "self.version", "open-telemetry/opentelemetry-propagation-traceresponse": "self.version", "open-telemetry/opentracing-shim": "self.version", "open-telemetry/sampler-rule-based": "self.version", diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 431b8ea00..2ebc3f1b4 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -133,7 +133,7 @@ public static function register(): void if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { /** @psalm-suppress PossiblyInvalidCast */ if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $sqlStatement = self::addSqlComments($sqlStatement, true); + $sqlStatement = self::addSqlComments($sqlStatement); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -177,7 +177,7 @@ public static function register(): void if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { /** @psalm-suppress PossiblyInvalidCast */ if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $sqlStatement = self::addSqlComments($sqlStatement, true); + $sqlStatement = self::addSqlComments($sqlStatement); if (self::isSqlCommenterAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -204,12 +204,8 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $sqlStatement = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); - if (!is_string($sqlStatement)) { - $sqlStatement = self::UNDEFINED; - } if ($class === PDO::class) { - $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $sqlStatement); + $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -218,23 +214,6 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { - /** @psalm-suppress PossiblyInvalidCast */ - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $sqlStatement = self::addSqlComments($sqlStatement, false); - if (self::isSqlCommenterAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); - } - - return [ - 0 => $sqlStatement, - ]; - } - } - - return []; }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { if ($statement instanceof PDOStatement) { @@ -434,19 +413,11 @@ private static function getSqlCommenterDatabase(): array return (array) (get_cfg_var('otel.instrumentation.pdo.sql_commenter.database') ?: []); } - private static function addSqlComments(string $query, bool $withTraceContext): string + private static function addSqlComments(string $query): string { $comments = []; - if ($withTraceContext) { - $prop = TraceContextPropagator::getInstance(); - $prop->inject($comments); - } - if (class_exists('OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator')) { - /** @phan-suppress-next-line PhanUndeclaredClassMethod */ - $prop = new \OpenTelemetry\Contrib\Propagation\ServiceName\ServiceNamePropagator(); - /** @phan-suppress-next-line PhanAccessMethodInternal,PhanUndeclaredClassMethod */ - $prop->inject($comments); - } + $prop = TraceContextPropagator::getInstance(); + $prop->inject($comments); $query = trim($query); if (self::isSqlCommenterPrepend()) { return Utils::formatComments(array_filter($comments)) . $query; diff --git a/src/Propagation/ServiceName/.gitattributes b/src/Propagation/ServiceName/.gitattributes deleted file mode 100644 index 1676cf825..000000000 --- a/src/Propagation/ServiceName/.gitattributes +++ /dev/null @@ -1,12 +0,0 @@ -* text=auto - -*.md diff=markdown -*.php diff=php - -/.gitattributes export-ignore -/.gitignore export-ignore -/.php-cs-fixer.php export-ignore -/phpstan.neon.dist export-ignore -/phpunit.xml.dist export-ignore -/psalm.xml.dist export-ignore -/tests export-ignore diff --git a/src/Propagation/ServiceName/.gitignore b/src/Propagation/ServiceName/.gitignore deleted file mode 100644 index 57872d0f1..000000000 --- a/src/Propagation/ServiceName/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/vendor/ diff --git a/src/Propagation/ServiceName/.phan/config.php b/src/Propagation/ServiceName/.phan/config.php deleted file mode 100644 index da2ac2d99..000000000 --- a/src/Propagation/ServiceName/.phan/config.php +++ /dev/null @@ -1,371 +0,0 @@ - '8.0', - - // If enabled, missing properties will be created when - // they are first seen. If false, we'll report an - // error message if there is an attempt to write - // to a class property that wasn't explicitly - // defined. - 'allow_missing_properties' => false, - - // If enabled, null can be cast to any type and any - // type can be cast to null. Setting this to true - // will cut down on false positives. - 'null_casts_as_any_type' => false, - - // If enabled, allow null to be cast as any array-like type. - // - // This is an incremental step in migrating away from `null_casts_as_any_type`. - // If `null_casts_as_any_type` is true, this has no effect. - 'null_casts_as_array' => true, - - // If enabled, allow any array-like type to be cast to null. - // This is an incremental step in migrating away from `null_casts_as_any_type`. - // If `null_casts_as_any_type` is true, this has no effect. - 'array_casts_as_null' => true, - - // If enabled, scalars (int, float, bool, string, null) - // are treated as if they can cast to each other. - // This does not affect checks of array keys. See `scalar_array_key_cast`. - 'scalar_implicit_cast' => false, - - // If enabled, any scalar array keys (int, string) - // are treated as if they can cast to each other. - // E.g. `array` can cast to `array` and vice versa. - // Normally, a scalar type such as int could only cast to/from int and mixed. - 'scalar_array_key_cast' => true, - - // If this has entries, scalars (int, float, bool, string, null) - // are allowed to perform the casts listed. - // - // E.g. `['int' => ['float', 'string'], 'float' => ['int'], 'string' => ['int'], 'null' => ['string']]` - // allows casting null to a string, but not vice versa. - // (subset of `scalar_implicit_cast`) - 'scalar_implicit_partial' => [], - - // If enabled, Phan will warn if **any** type in a method invocation's object - // is definitely not an object, - // or if **any** type in an invoked expression is not a callable. - // Setting this to true will introduce numerous false positives - // (and reveal some bugs). - 'strict_method_checking' => false, - - // If enabled, Phan will warn if **any** type of the object expression for a property access - // does not contain that property. - 'strict_object_checking' => false, - - // If enabled, Phan will warn if **any** type in the argument's union type - // cannot be cast to a type in the parameter's expected union type. - // Setting this to true will introduce numerous false positives - // (and reveal some bugs). - 'strict_param_checking' => false, - - // If enabled, Phan will warn if **any** type in a property assignment's union type - // cannot be cast to a type in the property's declared union type. - // Setting this to true will introduce numerous false positives - // (and reveal some bugs). - 'strict_property_checking' => false, - - // If enabled, Phan will warn if **any** type in a returned value's union type - // cannot be cast to the declared return type. - // Setting this to true will introduce numerous false positives - // (and reveal some bugs). - 'strict_return_checking' => false, - - // If true, seemingly undeclared variables in the global - // scope will be ignored. - // - // This is useful for projects with complicated cross-file - // globals that you have no hope of fixing. - 'ignore_undeclared_variables_in_global_scope' => true, - - // Set this to false to emit `PhanUndeclaredFunction` issues for internal functions that Phan has signatures for, - // but aren't available in the codebase, or from Reflection. - // (may lead to false positives if an extension isn't loaded) - // - // If this is true(default), then Phan will not warn. - // - // Even when this is false, Phan will still infer return values and check parameters of internal functions - // if Phan has the signatures. - 'ignore_undeclared_functions_with_known_signatures' => true, - - // Backwards Compatibility Checking. This is slow - // and expensive, but you should consider running - // it before upgrading your version of PHP to a - // new version that has backward compatibility - // breaks. - // - // If you are migrating from PHP 5 to PHP 7, - // you should also look into using - // [php7cc (no longer maintained)](https://github.com/sstalle/php7cc) - // and [php7mar](https://github.com/Alexia/php7mar), - // which have different backwards compatibility checks. - 'backward_compatibility_checks' => false, - - // If true, check to make sure the return type declared - // in the doc-block (if any) matches the return type - // declared in the method signature. - 'check_docblock_signature_return_type_match' => false, - - // If true, make narrowed types from phpdoc params override - // the real types from the signature, when real types exist. - // (E.g. allows specifying desired lists of subclasses, - // or to indicate a preference for non-nullable types over nullable types) - // - // Affects analysis of the body of the method and the param types passed in by callers. - // - // (*Requires `check_docblock_signature_param_type_match` to be true*) - 'prefer_narrowed_phpdoc_param_type' => true, - - // (*Requires `check_docblock_signature_return_type_match` to be true*) - // - // If true, make narrowed types from phpdoc returns override - // the real types from the signature, when real types exist. - // - // (E.g. allows specifying desired lists of subclasses, - // or to indicate a preference for non-nullable types over nullable types) - // - // This setting affects the analysis of return statements in the body of the method and the return types passed in by callers. - 'prefer_narrowed_phpdoc_return_type' => true, - - // If enabled, check all methods that override a - // parent method to make sure its signature is - // compatible with the parent's. - // - // This check can add quite a bit of time to the analysis. - // - // This will also check if final methods are overridden, etc. - 'analyze_signature_compatibility' => true, - - // This setting maps case-insensitive strings to union types. - // - // This is useful if a project uses phpdoc that differs from the phpdoc2 standard. - // - // If the corresponding value is the empty string, - // then Phan will ignore that union type (E.g. can ignore 'the' in `@return the value`) - // - // If the corresponding value is not empty, - // then Phan will act as though it saw the corresponding UnionTypes(s) - // when the keys show up in a UnionType of `@param`, `@return`, `@var`, `@property`, etc. - // - // This matches the **entire string**, not parts of the string. - // (E.g. `@return the|null` will still look for a class with the name `the`, but `@return the` will be ignored with the below setting) - // - // (These are not aliases, this setting is ignored outside of doc comments). - // (Phan does not check if classes with these names exist) - // - // Example setting: `['unknown' => '', 'number' => 'int|float', 'char' => 'string', 'long' => 'int', 'the' => '']` - 'phpdoc_type_mapping' => [], - - // Set to true in order to attempt to detect dead - // (unreferenced) code. Keep in mind that the - // results will only be a guess given that classes, - // properties, constants and methods can be referenced - // as variables (like `$class->$property` or - // `$class->$method()`) in ways that we're unable - // to make sense of. - 'dead_code_detection' => false, - - // Set to true in order to attempt to detect unused variables. - // `dead_code_detection` will also enable unused variable detection. - // - // This has a few known false positives, e.g. for loops or branches. - 'unused_variable_detection' => false, - - // Set to true in order to attempt to detect redundant and impossible conditions. - // - // This has some false positives involving loops, - // variables set in branches of loops, and global variables. - 'redundant_condition_detection' => false, - - // If enabled, Phan will act as though it's certain of real return types of a subset of internal functions, - // even if those return types aren't available in reflection (real types were taken from php 7.3 or 8.0-dev, depending on target_php_version). - // - // Note that with php 7 and earlier, php would return null or false for many internal functions if the argument types or counts were incorrect. - // As a result, enabling this setting with target_php_version 8.0 may result in false positives for `--redundant-condition-detection` when codebases also support php 7.x. - 'assume_real_types_for_internal_functions' => false, - - // If true, this runs a quick version of checks that takes less - // time at the cost of not running as thorough - // of an analysis. You should consider setting this - // to true only when you wish you had more **undiagnosed** issues - // to fix in your code base. - // - // In quick-mode the scanner doesn't rescan a function - // or a method's code block every time a call is seen. - // This means that the problem here won't be detected: - // - // ```php - // false, - - // Enable or disable support for generic templated - // class types. - 'generic_types_enabled' => true, - - // Override to hardcode existence and types of (non-builtin) globals in the global scope. - // Class names should be prefixed with `\`. - // - // (E.g. `['_FOO' => '\FooClass', 'page' => '\PageClass', 'userId' => 'int']`) - 'globals_type_map' => [], - - // The minimum severity level to report on. This can be - // set to `Issue::SEVERITY_LOW`, `Issue::SEVERITY_NORMAL` or - // `Issue::SEVERITY_CRITICAL`. Setting it to only - // critical issues is a good place to start on a big - // sloppy mature code base. - 'minimum_severity' => Issue::SEVERITY_LOW, - - // Add any issue types (such as `'PhanUndeclaredMethod'`) - // to this deny-list to inhibit them from being reported. - 'suppress_issue_types' => [], - - // A regular expression to match files to be excluded - // from parsing and analysis and will not be read at all. - // - // This is useful for excluding groups of test or example - // directories/files, unanalyzable files, or files that - // can't be removed for whatever reason. - // (e.g. `'@Test\.php$@'`, or `'@vendor/.*/(tests|Tests)/@'`) - 'exclude_file_regex' => '@^vendor/.*/(tests?|Tests?)/@', - - // A list of files that will be excluded from parsing and analysis - // and will not be read at all. - // - // This is useful for excluding hopelessly unanalyzable - // files that can't be removed for whatever reason. - 'exclude_file_list' => [ - 'vendor/composer/composer/src/Composer/InstalledVersions.php' - ], - - // A directory list that defines files that will be excluded - // from static analysis, but whose class and method - // information should be included. - // - // Generally, you'll want to include the directories for - // third-party code (such as "vendor/") in this list. - // - // n.b.: If you'd like to parse but not analyze 3rd - // party code, directories containing that code - // should be added to the `directory_list` as well as - // to `exclude_analysis_directory_list`. - 'exclude_analysis_directory_list' => [ - 'vendor/', - 'proto/', - 'thrift/' - ], - - // Enable this to enable checks of require/include statements referring to valid paths. - 'enable_include_path_checks' => true, - - // The number of processes to fork off during the analysis - // phase. - 'processes' => 1, - - // List of case-insensitive file extensions supported by Phan. - // (e.g. `['php', 'html', 'htm']`) - 'analyzed_file_extensions' => [ - 'php', - ], - - // You can put paths to stubs of internal extensions in this config option. - // If the corresponding extension is **not** loaded, then Phan will use the stubs instead. - // Phan will continue using its detailed type annotations, - // but load the constants, classes, functions, and classes (and their Reflection types) - // from these stub files (doubling as valid php files). - // Use a different extension from php to avoid accidentally loading these. - // The `tools/make_stubs` script can be used to generate your own stubs (compatible with php 7.0+ right now) - // - // (e.g. `['xdebug' => '.phan/internal_stubs/xdebug.phan_php']`) - 'autoload_internal_extension_signatures' => [], - - // A list of plugin files to execute. - // - // Plugins which are bundled with Phan can be added here by providing their name (e.g. `'AlwaysReturnPlugin'`) - // - // Documentation about available bundled plugins can be found [here](https://github.com/phan/phan/tree/master/.phan/plugins). - // - // Alternately, you can pass in the full path to a PHP file with the plugin's implementation (e.g. `'vendor/phan/phan/.phan/plugins/AlwaysReturnPlugin.php'`) - 'plugins' => [ - 'AlwaysReturnPlugin', - 'PregRegexCheckerPlugin', - 'UnreachableCodePlugin', - ], - - // A list of directories that should be parsed for class and - // method information. After excluding the directories - // defined in `exclude_analysis_directory_list`, the remaining - // files will be statically analyzed for errors. - // - // Thus, both first-party and third-party code being used by - // your application should be included in this list. - 'directory_list' => [ - 'src', - 'vendor' - ], - - // A list of individual files to include in analysis - // with a path relative to the root directory of the - // project. - 'file_list' => [], -]; diff --git a/src/Propagation/ServiceName/.php-cs-fixer.php b/src/Propagation/ServiceName/.php-cs-fixer.php deleted file mode 100644 index e35fa078c..000000000 --- a/src/Propagation/ServiceName/.php-cs-fixer.php +++ /dev/null @@ -1,43 +0,0 @@ -exclude('vendor') - ->exclude('var/cache') - ->in(__DIR__); - -$config = new PhpCsFixer\Config(); -return $config->setRules([ - 'concat_space' => ['spacing' => 'one'], - 'declare_equal_normalize' => ['space' => 'none'], - 'is_null' => true, - 'modernize_types_casting' => true, - 'ordered_imports' => true, - 'php_unit_construct' => true, - 'single_line_comment_style' => true, - 'yoda_style' => false, - '@PSR2' => true, - 'array_syntax' => ['syntax' => 'short'], - 'blank_line_after_opening_tag' => true, - 'blank_line_before_statement' => true, - 'cast_spaces' => true, - 'declare_strict_types' => true, - 'type_declaration_spaces' => true, - 'include' => true, - 'lowercase_cast' => true, - 'new_with_parentheses' => true, - 'no_extra_blank_lines' => true, - 'no_leading_import_slash' => true, - 'echo_tag_syntax' => true, - 'no_unused_imports' => true, - 'no_useless_else' => true, - 'no_useless_return' => true, - 'phpdoc_order' => true, - 'phpdoc_scalar' => true, - 'phpdoc_types' => true, - 'short_scalar_cast' => true, - 'blank_lines_before_namespace' => true, - 'single_quote' => true, - 'trailing_comma_in_multiline' => true, - ]) - ->setRiskyAllowed(true) - ->setFinder($finder); - diff --git a/src/Propagation/ServiceName/README.md b/src/Propagation/ServiceName/README.md deleted file mode 100644 index eda45e9f5..000000000 --- a/src/Propagation/ServiceName/README.md +++ /dev/null @@ -1,63 +0,0 @@ -[![Releases](https://img.shields.io/badge/releases-purple)](https://github.com/opentelemetry-php/contrib-propagator-service-name/releases) -[![Issues](https://img.shields.io/badge/issues-pink)](https://github.com/open-telemetry/opentelemetry-php/issues) -[![Source](https://img.shields.io/badge/source-contrib-green)](https://github.com/open-telemetry/opentelemetry-php-contrib/tree/main/src/Propagation/ServiceName) -[![Mirror](https://img.shields.io/badge/mirror-opentelemetry--php--contrib-blue)](https://github.com/opentelemetry-php/contrib-propagator-service-name) -[![Latest Version](http://poser.pugx.org/open-telemetry/opentelemetry-propagation-service-name/v/unstable)](https://packagist.org/packages/open-telemetry/opentelemetry-propagation-service-name/) -[![Stable](http://poser.pugx.org/open-telemetry/opentelemetry-propagation-service-name/v/stable)](https://packagist.org/packages/open-telemetry/opentelemetry-propagation-service-name/) - -This is a read-only subtree split of https://github.com/open-telemetry/opentelemetry-php-contrib. - -# OpenTelemetry Service Name Propagator - -This package provides a [service.name](https://opentelemetry.io/docs/specs/semconv/resource/#service) -propagator to inject the current `service.name` into Response datastructures. - -The main goal is to allow client-side technology (Real User Monitoring, HTTP Clients) to record -the server side context in order to allow referencing it. - -## Requirements - -* OpenTelemetry SDK and exporters (required to actually export traces) - -Optional: -* OpenTelemetry extension (Some instrumentations can automatically use the `TraceResponsePropagator`) - -## Usage - -Assuming there is an active `SpanContext`, you can inject it into your response as follows: - -```php -// your framework probably provides a datastructure to model HTTP responses -// and allows you to hook into the end of a request / listen to a matching event. -$response = new Response(); - -// get the current scope, bail out if none -$scope = Context::storage()->scope(); -if (null === $scope) { - return; -} - -// create a PropagationSetterInterface that knows how to inject response headers -$propagationSetter = new class implements OpenTelemetry\Context\Propagation\PropagationSetterInterface { - public function set(&$carrier, string $key, string $value) : void { - $carrier->headers->set($key, $value); - } -}; -$propagator = new ServiceNamePropagator(); -$propagator->inject($response, $propagationSetter, $scope->context()); -``` - -## Installation via composer - -```bash -$ composer require open-telemetry/opentelemetry-propagation-service-name -``` - -## Installing dependencies and executing tests - -From ServiceName subdirectory: - -```bash -$ composer install -$ ./vendor/bin/phpunit tests -``` diff --git a/src/Propagation/ServiceName/_register.php b/src/Propagation/ServiceName/_register.php deleted file mode 100644 index b7842a677..000000000 --- a/src/Propagation/ServiceName/_register.php +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - src - - - - - - - - - - - - - tests/Unit - - - - diff --git a/src/Propagation/ServiceName/psalm.xml.dist b/src/Propagation/ServiceName/psalm.xml.dist deleted file mode 100644 index 155711712..000000000 --- a/src/Propagation/ServiceName/psalm.xml.dist +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - - - - diff --git a/src/Propagation/ServiceName/src/ServiceNamePropagator.php b/src/Propagation/ServiceName/src/ServiceNamePropagator.php deleted file mode 100644 index d52ab9511..000000000 --- a/src/Propagation/ServiceName/src/ServiceNamePropagator.php +++ /dev/null @@ -1,74 +0,0 @@ -getResource(); - - if ($resource->getAttributes()->has(ResourceAttributes::SERVICE_NAME)) { - $setter->set($carrier, ResourceAttributes::SERVICE_NAME, $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME)); - } - } - - /** - * @suppress PhanUndeclaredClassAttribute - */ - #[\Override] - public function extract($carrier, ?PropagationGetterInterface $getter = null, ?ContextInterface $context = null): ContextInterface - { - $context ??= Context::getCurrent(); - - return $context; - } -} diff --git a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php b/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php deleted file mode 100644 index 99c5c4a2d..000000000 --- a/src/Propagation/ServiceName/tests/Unit/ServiceNamePropagatorTest.php +++ /dev/null @@ -1,60 +0,0 @@ -serviceNamePropagator = ServiceNamePropagator::getInstance(); - } - - public function test_fields(): void - { - $this->assertSame( - [ResourceAttributes::SERVICE_NAME], - $this->serviceNamePropagator->fields() - ); - } - - public function test_inject_empty(): void - { - $carrier = []; - $this->serviceNamePropagator->inject($carrier); - $this->assertEmpty($carrier); - } - - public function test_inject(): void - { - putenv('OTEL_SERVICE_NAME=foo-service'); - $carrier = []; - $this->serviceNamePropagator->inject($carrier); - $this->assertEquals($carrier, [ResourceAttributes::SERVICE_NAME=>'foo-service']); - putenv('OTEL_SERVICE_NAME'); - } - - public function test_extract_empty(): void - { - $carrier = []; - $context = $this->serviceNamePropagator->extract($carrier); - $this->assertSame(Context::getCurrent(), $context); - } - - public function test_no_extract(): void - { - $carrier = [ResourceAttributes::SERVICE_NAME => 'foo-service']; - $context = $this->serviceNamePropagator->extract($carrier); - $this->assertSame(Context::getCurrent(), $context); - } -} From 574e7fc5add12c40ca73735ac19e3691ceb2e261 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 18 Jul 2025 12:20:13 -0700 Subject: [PATCH 22/40] nits --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 2ebc3f1b4..eb4e42a7e 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -24,7 +24,6 @@ class PDOInstrumentation { public const NAME = 'pdo'; private const UNDEFINED = 'undefined'; - private const ALL = 'all'; public static function register(): void From 0a61fe2c338f9dbae3b224948a1f8fefa88e019d Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Mon, 21 Jul 2025 10:17:53 -0700 Subject: [PATCH 23/40] Used Globals --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index eb4e42a7e..aa21e39bb 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -4,8 +4,8 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\CachedInstrumentation; -use OpenTelemetry\API\Trace\Propagation\TraceContextPropagator; use OpenTelemetry\API\Trace\Span; use OpenTelemetry\API\Trace\SpanBuilderInterface; use OpenTelemetry\API\Trace\SpanKind; @@ -415,8 +415,7 @@ private static function getSqlCommenterDatabase(): array private static function addSqlComments(string $query): string { $comments = []; - $prop = TraceContextPropagator::getInstance(); - $prop->inject($comments); + Globals::propagator()->inject($comments); $query = trim($query); if (self::isSqlCommenterPrepend()) { return Utils::formatComments(array_filter($comments)) . $query; From 91556b58e8e526c3b72058ebfdf23fea81533a6c Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Mon, 21 Jul 2025 11:35:22 -0700 Subject: [PATCH 24/40] limited the opt-in to postgresql & mysql only --- src/Instrumentation/PDO/README.md | 14 +------------- src/Instrumentation/PDO/src/PDOInstrumentation.php | 14 +------------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index 7f9ba52de..75d8d8b00 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -36,7 +36,7 @@ OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS=true ``` ### SQL Commenter feature -The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive: +The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive, currently it can be used with `postgresql` and `mysql` drivers.: ``` otel.instrumentation.pdo.sql_commenter = true ``` @@ -44,18 +44,6 @@ or environment variable: ```shell OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER=true ``` -To select which database(s) to opt-in, specify the database(s) using configuration directive: - -Valid values are `postgresql`, `mysql`, `sqlite`, `mssql`, `oracle`, `db2`, `other_sql` and `all`. - -``` -otel.instrumentation.pdo.sql_commenter.database[]=postgresql -otel.instrumentation.pdo.sql_commenter.database[]=mysql -``` -or environment variable: -```shell -OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_DATABASE=postgresql,mysql -``` This feature by default will append a SQL comment to the query statement with the information about the code that executed the query. The SQL comment can be configured to prepend to the query statement using the following configuration directive: diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index aa21e39bb..2db46e83f 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -24,7 +24,6 @@ class PDOInstrumentation { public const NAME = 'pdo'; private const UNDEFINED = 'undefined'; - private const ALL = 'all'; public static function register(): void { @@ -403,15 +402,6 @@ private static function isSqlCommenterAttributeEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } - private static function getSqlCommenterDatabase(): array - { - if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration') && count($values = Configuration::getList('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_DATABASE', [])) > 0) { - return $values; - } - - return (array) (get_cfg_var('otel.instrumentation.pdo.sql_commenter.database') ?: []); - } - private static function addSqlComments(string $query): string { $comments = []; @@ -429,8 +419,6 @@ private static function addSqlComments(string $query): string private static function isSQLCommenterOptInDatabase(string $db) : bool { - $optInList = self::getSqlCommenterDatabase(); - - return in_array(strtolower($db), $optInList) || in_array(self::ALL, $optInList); + return $db == "postgresql" || $db == "mysql"; } } From 44a74c324a98bfbbbc2332c193953a8969a361d5 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Mon, 21 Jul 2025 11:37:31 -0700 Subject: [PATCH 25/40] nits --- src/Instrumentation/PDO/README.md | 2 +- src/Instrumentation/PDO/src/PDOInstrumentation.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index 75d8d8b00..5e9623de0 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -36,7 +36,7 @@ OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS=true ``` ### SQL Commenter feature -The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive, currently it can be used with `postgresql` and `mysql` drivers.: +The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive, currently it can be used with `postgresql` and `mysql` drivers only. ``` otel.instrumentation.pdo.sql_commenter = true ``` diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 2db46e83f..b1b72b7ed 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -419,6 +419,6 @@ private static function addSqlComments(string $query): string private static function isSQLCommenterOptInDatabase(string $db) : bool { - return $db == "postgresql" || $db == "mysql"; + return $db == 'postgresql' || $db == 'mysql'; } } From 7ad124a64ef15c7122f99d45ec3d544ae8a0f443 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 23 Jul 2025 09:48:38 -0700 Subject: [PATCH 26/40] Moved to another class and added unit tests --- .../PDO/src/ContextPropagation.php | 55 +++++++++++ .../PDO/src/PDOInstrumentation.php | 67 +++---------- .../PDO/tests/Unit/ContextPropagationTest.php | 97 +++++++++++++++++++ 3 files changed, 164 insertions(+), 55 deletions(-) create mode 100644 src/Instrumentation/PDO/src/ContextPropagation.php create mode 100644 src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php diff --git a/src/Instrumentation/PDO/src/ContextPropagation.php b/src/Instrumentation/PDO/src/ContextPropagation.php new file mode 100644 index 000000000..acfc984c8 --- /dev/null +++ b/src/Instrumentation/PDO/src/ContextPropagation.php @@ -0,0 +1,55 @@ +setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { + if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { /** @psalm-suppress PossiblyInvalidCast */ - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $sqlStatement = self::addSqlComments($sqlStatement); - if (self::isSqlCommenterAttributeEnabled()) { + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && ContextPropagation::isOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + $comments = []; + Globals::propagator()->inject($comments); + $sqlStatement = ContextPropagation::addSqlComments($sqlStatement, $comments); + if (ContextPropagation::isAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, ]); @@ -172,11 +174,13 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (self::isSqlCommenterEnabled() && $sqlStatement !== self::UNDEFINED) { + if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { /** @psalm-suppress PossiblyInvalidCast */ - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && self::isSQLCommenterOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $sqlStatement = self::addSqlComments($sqlStatement); - if (self::isSqlCommenterAttributeEnabled()) { + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && ContextPropagation::isOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { + $comments = []; + Globals::propagator()->inject($comments); + $sqlStatement = ContextPropagation::addSqlComments($sqlStatement, $comments); + if (ContextPropagation::isAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, ]); @@ -374,51 +378,4 @@ private static function isDistributeStatementToLinkedSpansEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.distribute_statement_to_linked_spans'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } - - private static function isSqlCommenterEnabled(): bool - { - if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { - return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER', false); - } - - return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; - } - - private static function isSqlCommenterPrepend(): bool - { - if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { - return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND', false); - } - - return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.prepend'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; - } - - private static function isSqlCommenterAttributeEnabled(): bool - { - if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { - return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_ATTRIBUTE', false); - } - - return filter_var(get_cfg_var('otel.instrumentation.pdo.sql_commenter.attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; - } - - private static function addSqlComments(string $query): string - { - $comments = []; - Globals::propagator()->inject($comments); - $query = trim($query); - if (self::isSqlCommenterPrepend()) { - return Utils::formatComments(array_filter($comments)) . $query; - } - $hasSemicolon = $query !== '' && $query[strlen($query) - 1] === ';'; - $query = rtrim($query, ';'); - - return $query . Utils::formatComments(array_filter($comments)) . ($hasSemicolon ? ';' : ''); - - } - - private static function isSQLCommenterOptInDatabase(string $db) : bool - { - return $db == 'postgresql' || $db == 'mysql'; - } } diff --git a/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php b/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php new file mode 100644 index 000000000..5cab84037 --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php @@ -0,0 +1,97 @@ +assertTrue($result); + } + + #[BackupGlobals(true)] + public function testIsEnabledReturnsFalse() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION'] = false; + $result = ContextPropagation::isEnabled(); + $this->assertFalse($result); + } + + #[BackupGlobals(true)] + public function testIsPrependReturnsTrue() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; + $result = ContextPropagation::isPrepend(); + $this->assertTrue($result); + } + + #[BackupGlobals(true)] + public function testIsPrependReturnsFalse() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; + $result = ContextPropagation::isPrepend(); + $this->assertFalse($result); + } + + #[BackupGlobals(true)] + public function testIsAttributeEnabledReturnsTrue() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE'] = true; + $result = ContextPropagation::isAttributeEnabled(); + $this->assertTrue($result); + } + + #[BackupGlobals(true)] + public function testIsAttributeEnabledReturnsFalse() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE'] = false; + $result = ContextPropagation::isAttributeEnabled(); + $this->assertFalse($result); + } + + #[BackupGlobals(true)] + public function testAddSqlCommentsPrepend() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; + $comments = [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + $query = 'SELECT 1;'; + $result = ContextPropagation::addSqlComments($query, $comments); + $this->assertEquals("/*key1='value1',key2='value2',key3='value3'*/SELECT 1;", $result); + } + + #[BackupGlobals(true)] + public function testAddSqlCommentsAppend() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; + $comments = [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + $query = 'SELECT 1;'; + $result = ContextPropagation::addSqlComments($query, $comments); + $this->assertEquals("SELECT 1/*key1='value1',key2='value2',key3='value3'*/;", $result); + } + + public function testIsOptInDatabase() + { + $this->assertTrue(ContextPropagation::isOptInDatabase('postgresql')); + $this->assertTrue(ContextPropagation::isOptInDatabase('mysql')); + $this->assertFalse(ContextPropagation::isOptInDatabase('sqlite')); + } +} From 2ee35906ea29425bfa8e374c8e42ecf9b300ccea Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Wed, 23 Jul 2025 09:56:23 -0700 Subject: [PATCH 27/40] Fixed psalm complaints --- .../PDO/tests/Unit/ContextPropagationTest.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php b/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php index 5cab84037..de4ee65e8 100644 --- a/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php +++ b/src/Instrumentation/PDO/tests/Unit/ContextPropagationTest.php @@ -5,14 +5,10 @@ namespace OpenTelemetry\Tests\Instrumentation\PDO\tests\Unit; use OpenTelemetry\Contrib\Instrumentation\PDO\ContextPropagation; -use PHPUnit\Framework\Attributes\BackupGlobals; -use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; -#[CoversClass(ContextPropagation::class)] class ContextPropagationTest extends TestCase { - #[BackupGlobals(true)] public function testIsEnabledReturnsTrue() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION'] = true; @@ -20,7 +16,6 @@ public function testIsEnabledReturnsTrue() $this->assertTrue($result); } - #[BackupGlobals(true)] public function testIsEnabledReturnsFalse() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION'] = false; @@ -28,7 +23,6 @@ public function testIsEnabledReturnsFalse() $this->assertFalse($result); } - #[BackupGlobals(true)] public function testIsPrependReturnsTrue() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; @@ -36,7 +30,6 @@ public function testIsPrependReturnsTrue() $this->assertTrue($result); } - #[BackupGlobals(true)] public function testIsPrependReturnsFalse() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; @@ -44,7 +37,6 @@ public function testIsPrependReturnsFalse() $this->assertFalse($result); } - #[BackupGlobals(true)] public function testIsAttributeEnabledReturnsTrue() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE'] = true; @@ -52,7 +44,6 @@ public function testIsAttributeEnabledReturnsTrue() $this->assertTrue($result); } - #[BackupGlobals(true)] public function testIsAttributeEnabledReturnsFalse() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE'] = false; @@ -60,7 +51,6 @@ public function testIsAttributeEnabledReturnsFalse() $this->assertFalse($result); } - #[BackupGlobals(true)] public function testAddSqlCommentsPrepend() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; @@ -74,7 +64,6 @@ public function testAddSqlCommentsPrepend() $this->assertEquals("/*key1='value1',key2='value2',key3='value3'*/SELECT 1;", $result); } - #[BackupGlobals(true)] public function testAddSqlCommentsAppend() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; From 0ce7b7340c257221994157caeec730d3ef619376 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 25 Jul 2025 13:26:27 -0700 Subject: [PATCH 28/40] Updated --- src/Instrumentation/PDO/README.md | 21 +++--- .../PDO/src/ContextPropagation.php | 27 -------- .../PDO/src/PDOInstrumentation.php | 66 +++++++++++-------- .../PDO/src/SqlCommentPropagator.php | 31 +++++++++ .../PDO/src/SqlPropagatorInterface.php | 10 +++ .../PDO/src/SqlServerPropagator.php | 14 ++++ .../PDO/tests/Unit/ContextPropagationTest.php | 47 ------------- .../tests/Unit/SqlCommentPropagatorTest.php | 51 ++++++++++++++ .../tests/Unit/SqlServerPropagatorTest.php | 24 +++++++ 9 files changed, 181 insertions(+), 110 deletions(-) create mode 100644 src/Instrumentation/PDO/src/SqlCommentPropagator.php create mode 100644 src/Instrumentation/PDO/src/SqlPropagatorInterface.php create mode 100644 src/Instrumentation/PDO/src/SqlServerPropagator.php create mode 100644 src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php create mode 100644 src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index 5e9623de0..d0982203b 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -38,27 +38,28 @@ OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS=true ### SQL Commenter feature The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive, currently it can be used with `postgresql` and `mysql` drivers only. ``` -otel.instrumentation.pdo.sql_commenter = true +otel.instrumentation.pdo.context_propagation = true ``` or environment variable: ```shell -OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER=true +OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION=true ``` -This feature by default will append a SQL comment to the query statement with the information about the code that executed the query. -The SQL comment can be configured to prepend to the query statement using the following configuration directive: +The modified query statement by default will not update `TraceAttributes::DB_QUERY_TEXT` due to high cardinality risk, but it can be configured using the following configuration directive: ``` -otel.instrumentation.pdo.sql_commenter.prepend = true +otel.instrumentation.pdo.context_propagation.attribute = true ``` or environment variable: ```shell -OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND=true +OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE=true ``` -The modified query statement by default will not update `TraceAttributes::DB_QUERY_TEXT` due to high cardinality risk, but it can be configured using the following configuration directive: + +This feature by default will append a SQL comment to the query statement with the information about the code that executed the query. +The SQL comment can be configured to prepend to the query statement using the following configuration directive: ``` -otel.instrumentation.pdo.sql_commenter.attribute = true +otel.instrumentation.pdo.sql_commenter.prepend = true ``` or environment variable: ```shell -OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_ATTRIBUTE=true -``` +OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND=true +``` \ No newline at end of file diff --git a/src/Instrumentation/PDO/src/ContextPropagation.php b/src/Instrumentation/PDO/src/ContextPropagation.php index acfc984c8..ef48b9a7e 100644 --- a/src/Instrumentation/PDO/src/ContextPropagation.php +++ b/src/Instrumentation/PDO/src/ContextPropagation.php @@ -17,15 +17,6 @@ public static function isEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.context_propagation'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } - public static function isPrepend(): bool - { - if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration') && Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND', false)) { - return true; - } - - return filter_var(get_cfg_var('otel.instrumentation.pdo.context_propagation.prepend'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; - } - public static function isAttributeEnabled(): bool { if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration') && Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE', false)) { @@ -34,22 +25,4 @@ public static function isAttributeEnabled(): bool return filter_var(get_cfg_var('otel.instrumentation.pdo.context_propagation.attribute'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; } - - public static function addSqlComments(string $query, array $comments) : string - { - $query = trim($query); - if (self::isPrepend()) { - return Utils::formatComments(array_filter($comments)) . $query; - } - $hasSemicolon = $query !== '' && $query[strlen($query) - 1] === ';'; - $query = rtrim($query, ';'); - - return $query . Utils::formatComments(array_filter($comments)) . ($hasSemicolon ? ';' : ''); - - } - - public static function isOptInDatabase(string $db) : bool - { - return $db == 'postgresql' || $db == 'mysql'; - } } diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 29ce94dd8..26848de17 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -129,20 +129,27 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { - /** @psalm-suppress PossiblyInvalidCast */ - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && ContextPropagation::isOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $comments = []; - Globals::propagator()->inject($comments); - $sqlStatement = ContextPropagation::addSqlComments($sqlStatement, $comments); - if (ContextPropagation::isAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes)) { + /** @psalm-suppress PossiblyInvalidCast */ + switch ((string) $attributes[TraceAttributes::DB_SYSTEM_NAME]) { + case 'postgresql': + case 'mysql': + $comments = []; + Globals::propagator()->inject($comments); + $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); + if (ContextPropagation::isAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } + + return [ + 0 => $sqlStatement, + ]; + default: + // Do nothing, not a database we want to propagate + break; } - - return [ - 0 => $sqlStatement, - ]; } } @@ -175,20 +182,27 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { - /** @psalm-suppress PossiblyInvalidCast */ - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes) && ContextPropagation::isOptInDatabase((string) ($attributes[TraceAttributes::DB_SYSTEM_NAME]))) { - $comments = []; - Globals::propagator()->inject($comments); - $sqlStatement = ContextPropagation::addSqlComments($sqlStatement, $comments); - if (ContextPropagation::isAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); + if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes)) { + /** @psalm-suppress PossiblyInvalidCast */ + switch ((string) $attributes[TraceAttributes::DB_SYSTEM_NAME]) { + case 'postgresql': + case 'mysql': + $comments = []; + Globals::propagator()->inject($comments); + $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); + if (ContextPropagation::isAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } + + return [ + 0 => $sqlStatement, + ]; + default: + // Do nothing, not a database we want to propagate + break; } - - return [ - 0 => $sqlStatement, - ]; } } diff --git a/src/Instrumentation/PDO/src/SqlCommentPropagator.php b/src/Instrumentation/PDO/src/SqlCommentPropagator.php new file mode 100644 index 000000000..622f40a73 --- /dev/null +++ b/src/Instrumentation/PDO/src/SqlCommentPropagator.php @@ -0,0 +1,31 @@ +assertFalse($result); } - public function testIsPrependReturnsTrue() - { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; - $result = ContextPropagation::isPrepend(); - $this->assertTrue($result); - } - - public function testIsPrependReturnsFalse() - { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; - $result = ContextPropagation::isPrepend(); - $this->assertFalse($result); - } - public function testIsAttributeEnabledReturnsTrue() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_ATTRIBUTE'] = true; @@ -50,37 +36,4 @@ public function testIsAttributeEnabledReturnsFalse() $result = ContextPropagation::isAttributeEnabled(); $this->assertFalse($result); } - - public function testAddSqlCommentsPrepend() - { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; - $comments = [ - 'key1' => 'value1', - 'key2' => 'value2', - 'key3' => 'value3', - ]; - $query = 'SELECT 1;'; - $result = ContextPropagation::addSqlComments($query, $comments); - $this->assertEquals("/*key1='value1',key2='value2',key3='value3'*/SELECT 1;", $result); - } - - public function testAddSqlCommentsAppend() - { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; - $comments = [ - 'key1' => 'value1', - 'key2' => 'value2', - 'key3' => 'value3', - ]; - $query = 'SELECT 1;'; - $result = ContextPropagation::addSqlComments($query, $comments); - $this->assertEquals("SELECT 1/*key1='value1',key2='value2',key3='value3'*/;", $result); - } - - public function testIsOptInDatabase() - { - $this->assertTrue(ContextPropagation::isOptInDatabase('postgresql')); - $this->assertTrue(ContextPropagation::isOptInDatabase('mysql')); - $this->assertFalse(ContextPropagation::isOptInDatabase('sqlite')); - } } diff --git a/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php b/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php new file mode 100644 index 000000000..7aae8a80d --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php @@ -0,0 +1,51 @@ +assertTrue($result); + } + + public function testIsPrependReturnsFalse() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; + $result = SqlCommentPropagator::isPrepend(); + $this->assertFalse($result); + } + + public function testInjectPrepend() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; + $comments = [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + $query = 'SELECT 1;'; + $result = SqlCommentPropagator::inject($query, $comments); + $this->assertEquals("/*key1='value1',key2='value2',key3='value3'*/SELECT 1;", $result); + } + + public function testInjectAppend() + { + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; + $comments = [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + $query = 'SELECT 1;'; + $result = SqlCommentPropagator::inject($query, $comments); + $this->assertEquals("SELECT 1/*key1='value1',key2='value2',key3='value3'*/;", $result); + } +} diff --git a/src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php b/src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php new file mode 100644 index 000000000..adcbb34d5 --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php @@ -0,0 +1,24 @@ + 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + $query = 'SELECT 1;'; + $result = SqlServerPropagator::inject($query, $comments); + $this->assertEquals('SELECT 1;', $result); + } + +} From bb2d5fd38c9d4613f172a6e8e4852aa0142c0b1b Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 25 Jul 2025 13:40:17 -0700 Subject: [PATCH 29/40] Fixed tests --- .../PDO/tests/Unit/SqlCommentPropagatorTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php b/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php index 7aae8a80d..27ee25b6e 100644 --- a/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php +++ b/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php @@ -11,21 +11,21 @@ class SqlCommentPropagatorTest extends TestCase { public function testIsPrependReturnsTrue() { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = true; $result = SqlCommentPropagator::isPrepend(); $this->assertTrue($result); } public function testIsPrependReturnsFalse() { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = false; $result = SqlCommentPropagator::isPrepend(); $this->assertFalse($result); } public function testInjectPrepend() { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = true; + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = true; $comments = [ 'key1' => 'value1', 'key2' => 'value2', @@ -38,7 +38,7 @@ public function testInjectPrepend() public function testInjectAppend() { - $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION_PREPEND'] = false; + $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = false; $comments = [ 'key1' => 'value1', 'key2' => 'value2', From 7aee9ec28f3984b585c8e03a09f1c9caffe06020 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 29 Aug 2025 11:46:02 -0700 Subject: [PATCH 30/40] Address comments --- .../{SqlServerPropagator.php => ContextInfoPropagator.php} | 2 +- ...rverPropagatorTest.php => ContextInfoPropagatorTest.php} | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/Instrumentation/PDO/src/{SqlServerPropagator.php => ContextInfoPropagator.php} (83%) rename src/Instrumentation/PDO/tests/Unit/{SqlServerPropagatorTest.php => ContextInfoPropagatorTest.php} (67%) diff --git a/src/Instrumentation/PDO/src/SqlServerPropagator.php b/src/Instrumentation/PDO/src/ContextInfoPropagator.php similarity index 83% rename from src/Instrumentation/PDO/src/SqlServerPropagator.php rename to src/Instrumentation/PDO/src/ContextInfoPropagator.php index b499eafd8..8dc832eed 100644 --- a/src/Instrumentation/PDO/src/SqlServerPropagator.php +++ b/src/Instrumentation/PDO/src/ContextInfoPropagator.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; -class SqlServerPropagator implements SqlPropagatorInterface +class ContextInfoPropagator implements SqlPropagatorInterface { public static function inject(string $query, array $comments): string { diff --git a/src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php b/src/Instrumentation/PDO/tests/Unit/ContextInfoPropagatorTest.php similarity index 67% rename from src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php rename to src/Instrumentation/PDO/tests/Unit/ContextInfoPropagatorTest.php index adcbb34d5..d7a91a760 100644 --- a/src/Instrumentation/PDO/tests/Unit/SqlServerPropagatorTest.php +++ b/src/Instrumentation/PDO/tests/Unit/ContextInfoPropagatorTest.php @@ -4,10 +4,10 @@ namespace OpenTelemetry\Tests\Instrumentation\PDO\tests\Unit; -use OpenTelemetry\Contrib\Instrumentation\PDO\SqlServerPropagator; +use OpenTelemetry\Contrib\Instrumentation\PDO\ContextInfoPropagator; use PHPUnit\Framework\TestCase; -class SqlServerPropagatorTest extends TestCase +class ContextInfoPropagatorTest extends TestCase { public function testInject() { @@ -17,7 +17,7 @@ public function testInject() 'key3' => 'value3', ]; $query = 'SELECT 1;'; - $result = SqlServerPropagator::inject($query, $comments); + $result = ContextInfoPropagator::inject($query, $comments); $this->assertEquals('SELECT 1;', $result); } From ee41b2a94ed4221e81a09659f0341b272738afc6 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 29 Aug 2025 12:51:26 -0700 Subject: [PATCH 31/40] Fixed --- src/Instrumentation/PDO/README.md | 2 +- .../PDO/src/PDOInstrumentation.php | 8 ++-- .../Integration/PDOInstrumentationTest.php | 42 +++++++++---------- .../tests/Unit/PDOAttributeTrackerTest.php | 12 +++--- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index d0982203b..3a98152d5 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -45,7 +45,7 @@ or environment variable: OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION=true ``` -The modified query statement by default will not update `TraceAttributes::DB_QUERY_TEXT` due to high cardinality risk, but it can be configured using the following configuration directive: +The modified query statement by default will not update `DbAttributes::DB_QUERY_TEXT` due to high cardinality risk, but it can be configured using the following configuration directive: ``` otel.instrumentation.pdo.context_propagation.attribute = true ``` diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 26848de17..eb434285d 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -129,9 +129,9 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes)) { + if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { /** @psalm-suppress PossiblyInvalidCast */ - switch ((string) $attributes[TraceAttributes::DB_SYSTEM_NAME]) { + switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { case 'postgresql': case 'mysql': $comments = []; @@ -182,9 +182,9 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { - if (array_key_exists(TraceAttributes::DB_SYSTEM_NAME, $attributes)) { + if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { /** @psalm-suppress PossiblyInvalidCast */ - switch ((string) $attributes[TraceAttributes::DB_SYSTEM_NAME]) { + switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { case 'postgresql': case 'mysql': $comments = []; diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index 9398c8701..3f408c013 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -14,7 +14,7 @@ use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter; use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor; use OpenTelemetry\SDK\Trace\TracerProvider; -use OpenTelemetry\SemConv\TraceAttributes; +use OpenTelemetry\SemConv\Attributes\DbAttributes; use OpenTelemetry\TestUtils\TraceStructureAssertionTrait; use PDO; use PHPUnit\Framework\TestCase; @@ -85,7 +85,7 @@ public function test_pdo_construct(): void $this->assertCount(1, $this->storage); $span = $this->storage->offsetGet(0); $this->assertSame('PDO::__construct', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); } /** @@ -112,7 +112,7 @@ public function test_pdo_sqlite_subclass(): void $this->assertCount(1, $this->storage); $span = $this->storage->offsetGet(0); $this->assertSame('PDO::connect', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); // Test that the subclass-specific methods work $db->createFunction('test_function', static fn ($value) => strtoupper($value)); @@ -121,7 +121,7 @@ public function test_pdo_sqlite_subclass(): void $db->exec($this->fillDB()); $span = $this->storage->offsetGet(1); $this->assertSame('PDO::exec', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(2, $this->storage); // Test that the custom function works @@ -129,7 +129,7 @@ public function test_pdo_sqlite_subclass(): void $this->assertEquals('HELLO', $result); $span = $this->storage->offsetGet(2); $this->assertSame('PDO::query', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(3, $this->storage); } @@ -148,32 +148,32 @@ public function test_statement_execution(): void $db->exec($statement); $span = $this->storage->offsetGet(1); $this->assertSame('PDO::exec', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertFalse($db->inTransaction()); $this->assertCount(2, $this->storage); $sth = $db->prepare('SELECT * FROM `technology`'); $span = $this->storage->offsetGet(2); $this->assertSame('PDO::prepare', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(3, $this->storage); $sth->execute(); $span = $this->storage->offsetGet(3); $this->assertSame('PDOStatement::execute', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(4, $this->storage); $sth->fetchAll(); $span = $this->storage->offsetGet(4); $this->assertSame('PDOStatement::fetchAll', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(5, $this->storage); $db->query('SELECT * FROM `technology`'); $span = $this->storage->offsetGet(5); $this->assertSame('PDO::query', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(6, $this->storage); } @@ -183,7 +183,7 @@ public function test_transaction(): void $result = $db->beginTransaction(); $span = $this->storage->offsetGet(1); $this->assertSame('PDO::beginTransaction', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(2, $this->storage); $this->assertSame($result, true); @@ -192,7 +192,7 @@ public function test_transaction(): void $result = $db->commit(); $span = $this->storage->offsetGet(3); $this->assertSame('PDO::commit', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(4, $this->storage); $this->assertTrue($result); @@ -204,7 +204,7 @@ public function test_transaction(): void $result = $db->rollback(); $span = $this->storage->offsetGet(6); $this->assertSame('PDO::rollBack', $span->getName()); - $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertEquals('sqlite', $span->getAttributes()->get(DbAttributes::DB_SYSTEM_NAME)); $this->assertCount(7, $this->storage); $this->assertTrue($result); $this->assertFalse($db->inTransaction()); @@ -262,17 +262,17 @@ public function test_encode_db_statement_as_utf8(): void $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_QUERY_TEXT), 'UTF-8')); + $this->assertTrue(mb_check_encoding($span_db_prepare->getAttributes()->get(DbAttributes::DB_QUERY_TEXT), '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_QUERY_TEXT), 'UTF-8')); + $this->assertTrue(mb_check_encoding($span_db_query->getAttributes()->get(DbAttributes::DB_QUERY_TEXT), '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_QUERY_TEXT), 'UTF-8')); + $this->assertTrue(mb_check_encoding($span_db_exec->getAttributes()->get(DbAttributes::DB_QUERY_TEXT), 'UTF-8')); $this->assertCount(5, $this->storage); } @@ -364,35 +364,35 @@ public function test_span_hierarchy_with_pdo_operations(): void 'name' => 'PDO::__construct', 'kind' => SpanKind::KIND_CLIENT, 'attributes' => [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', ], ], [ 'name' => 'PDO::exec', 'kind' => SpanKind::KIND_CLIENT, 'attributes' => [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', ], ], [ 'name' => 'PDO::prepare', 'kind' => SpanKind::KIND_CLIENT, 'attributes' => [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', ], ], [ 'name' => 'PDOStatement::execute', 'kind' => SpanKind::KIND_CLIENT, 'attributes' => [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', ], ], [ 'name' => 'PDOStatement::fetchAll', 'kind' => SpanKind::KIND_CLIENT, 'attributes' => [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', ], ], ], diff --git a/src/Instrumentation/PDO/tests/Unit/PDOAttributeTrackerTest.php b/src/Instrumentation/PDO/tests/Unit/PDOAttributeTrackerTest.php index 44937122e..9516a0af4 100644 --- a/src/Instrumentation/PDO/tests/Unit/PDOAttributeTrackerTest.php +++ b/src/Instrumentation/PDO/tests/Unit/PDOAttributeTrackerTest.php @@ -6,7 +6,7 @@ use OpenTelemetry\API\Trace\Span; use OpenTelemetry\Contrib\Instrumentation\PDO\PDOTracker; -use OpenTelemetry\SemConv\TraceAttributes; +use OpenTelemetry\SemConv\Attributes\DbAttributes; use PHPUnit\Framework\TestCase; class PDOAttributeTrackerTest extends TestCase @@ -22,20 +22,20 @@ public function testPdoCanBeTracked(): void $span = Span::getInvalid(); /** @psalm-suppress InvalidArgument */ - $this->assertContains(TraceAttributes::DB_SYSTEM_NAME, array_keys($attributes)); + $this->assertContains(DbAttributes::DB_SYSTEM_NAME, array_keys($attributes)); /** @psalm-suppress InvalidArgument */ - $this->assertContains(TraceAttributes::DB_NAMESPACE, array_keys($attributes)); + $this->assertContains(DbAttributes::DB_NAMESPACE, array_keys($attributes)); /** @psalm-suppress InvalidArrayAccess */ - $this->assertSame('memory', $attributes[TraceAttributes::DB_NAMESPACE]); + $this->assertSame('memory', $attributes[DbAttributes::DB_NAMESPACE]); $stmt = $pdo->prepare('SELECT NULL LIMIT 0;'); $objectMap->trackStatement($stmt, $pdo, $span->getContext()); $attributes = $objectMap->trackedAttributesForStatement($stmt); /** @psalm-suppress InvalidArgument */ - $this->assertContains(TraceAttributes::DB_SYSTEM_NAME, array_keys($attributes)); + $this->assertContains(DbAttributes::DB_SYSTEM_NAME, array_keys($attributes)); /** @psalm-suppress InvalidArrayAccess */ - $this->assertEquals('sqlite', $attributes[TraceAttributes::DB_SYSTEM_NAME]); + $this->assertEquals('sqlite', $attributes[DbAttributes::DB_SYSTEM_NAME]); $this->assertSame($span->getContext(), $objectMap->getSpanForPreparedStatement($stmt)); } } From df60ca851cbd4c4c8f0e5397336590000d629de8 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 29 Aug 2025 13:22:52 -0700 Subject: [PATCH 32/40] Updated attributes --- src/Instrumentation/PDO/src/PDOTracker.php | 37 +++++----- .../PDO/tests/Unit/PDOTrackerTest.php | 73 ++++++++++--------- 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOTracker.php b/src/Instrumentation/PDO/src/PDOTracker.php index 4753a0baa..d458238ec 100644 --- a/src/Instrumentation/PDO/src/PDOTracker.php +++ b/src/Instrumentation/PDO/src/PDOTracker.php @@ -5,7 +5,8 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; use OpenTelemetry\API\Trace\SpanContextInterface; -use OpenTelemetry\SemConv\TraceAttributes; +use OpenTelemetry\SemConv\Attributes\DbAttributes; +use OpenTelemetry\SemConv\Attributes\ServerAttributes; use PDO; use PDOStatement; use WeakMap; @@ -74,11 +75,11 @@ public function trackPdoAttributes(PDO $pdo, string $dsn): array /** @var string $dbSystem */ $dbSystem = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); /** @psalm-suppress InvalidArrayAssignment */ - $attributes[TraceAttributes::DB_SYSTEM_NAME] = self::mapDriverNameToAttribute($dbSystem); + $attributes[DbAttributes::DB_SYSTEM_NAME] = self::mapDriverNameToAttribute($dbSystem); } catch (\Error) { // if we caught an exception, the driver is likely not supporting the operation, default to "other" /** @psalm-suppress PossiblyInvalidArrayAssignment */ - $attributes[TraceAttributes::DB_SYSTEM_NAME] = 'other_sql'; + $attributes[DbAttributes::DB_SYSTEM_NAME] = 'other_sql'; } $this->pdoToAttributesMap[$pdo] = $attributes; @@ -135,18 +136,18 @@ private static function extractAttributesFromDSN(string $dsn): array { $attributes = []; if (str_starts_with($dsn, 'sqlite::memory:')) { - $attributes[TraceAttributes::DB_SYSTEM_NAME] = 'sqlite'; - $attributes[TraceAttributes::DB_NAMESPACE] = 'memory'; + $attributes[DbAttributes::DB_SYSTEM_NAME] = 'sqlite'; + $attributes[DbAttributes::DB_NAMESPACE] = 'memory'; return $attributes; } elseif (str_starts_with($dsn, 'sqlite:')) { - $attributes[TraceAttributes::DB_SYSTEM_NAME] = 'sqlite'; - $attributes[TraceAttributes::DB_NAMESPACE] = substr($dsn, 7); + $attributes[DbAttributes::DB_SYSTEM_NAME] = 'sqlite'; + $attributes[DbAttributes::DB_NAMESPACE] = substr($dsn, 7); return $attributes; } elseif (str_starts_with($dsn, 'sqlite')) { - $attributes[TraceAttributes::DB_SYSTEM_NAME] = 'sqlite'; - $attributes[TraceAttributes::DB_NAMESPACE] = $dsn; + $attributes[DbAttributes::DB_SYSTEM_NAME] = 'sqlite'; + $attributes[DbAttributes::DB_NAMESPACE] = $dsn; return $attributes; } @@ -156,18 +157,18 @@ private static function extractAttributesFromDSN(string $dsn): array if (preg_match('/Server=([^,;]+)(?:,([0-9]+))?/', $dsn, $serverMatches)) { $server = $serverMatches[1]; if ($server !== '') { - $attributes[TraceAttributes::SERVER_ADDRESS] = $server; + $attributes[ServerAttributes::SERVER_ADDRESS] = $server; } if (isset($serverMatches[2]) && $serverMatches[2] !== '') { - $attributes[TraceAttributes::SERVER_PORT] = (int) $serverMatches[2]; + $attributes[ServerAttributes::SERVER_PORT] = (int) $serverMatches[2]; } } if (preg_match('/Database=([^;]*)/', $dsn, $dbMatches)) { $dbname = $dbMatches[1]; if ($dbname !== '') { - $attributes[TraceAttributes::DB_NAMESPACE] = $dbname; + $attributes[DbAttributes::DB_NAMESPACE] = $dbname; } } @@ -186,33 +187,33 @@ private static function extractAttributesFromDSN(string $dsn): array if (preg_match('/host=([^;]*)/', $dsn, $matches)) { $host = $matches[1]; if ($host !== '') { - $attributes[TraceAttributes::SERVER_ADDRESS] = $host; + $attributes[ServerAttributes::SERVER_ADDRESS] = $host; } } elseif (preg_match('/mysql:([^;:]+)/', $dsn, $hostMatches)) { $host = $hostMatches[1]; if ($host !== '' && $host !== 'dbname') { - $attributes[TraceAttributes::SERVER_ADDRESS] = $host; + $attributes[ServerAttributes::SERVER_ADDRESS] = $host; } } // Extract port information if (preg_match('/port=([0-9]+)/', $dsn, $portMatches)) { $port = (int) $portMatches[1]; - $attributes[TraceAttributes::SERVER_PORT] = $port; + $attributes[ServerAttributes::SERVER_PORT] = $port; } elseif (preg_match('/[.0-9]+:([0-9]+)/', $dsn, $portMatches)) { // This pattern matches IP:PORT format like 127.0.0.1:3308 $port = (int) $portMatches[1]; - $attributes[TraceAttributes::SERVER_PORT] = $port; + $attributes[ServerAttributes::SERVER_PORT] = $port; } elseif (preg_match('/:([0-9]+)/', $dsn, $portMatches)) { $port = (int) $portMatches[1]; - $attributes[TraceAttributes::SERVER_PORT] = $port; + $attributes[ServerAttributes::SERVER_PORT] = $port; } // Extract database name if (preg_match('/dbname=([^;]*)/', $dsn, $matches)) { $dbname = $matches[1]; if ($dbname !== '') { - $attributes[TraceAttributes::DB_NAMESPACE] = $dbname; + $attributes[DbAttributes::DB_NAMESPACE] = $dbname; } } diff --git a/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php b/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php index 5049f2039..3407bf5b8 100644 --- a/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php +++ b/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php @@ -5,7 +5,8 @@ namespace OpenTelemetry\Tests\Instrumentation\PDO\tests\Unit; use OpenTelemetry\Contrib\Instrumentation\PDO\PDOTracker; -use OpenTelemetry\SemConv\TraceAttributes; +use OpenTelemetry\SemConv\Attributes\DbAttributes; +use OpenTelemetry\SemConv\Attributes\ServerAttributes; use PHPUnit\Framework\TestCase; use ReflectionMethod; @@ -39,106 +40,106 @@ public function dsnProvider(): array 'standard format with host and port' => [ 'mysql:host=localhost;port=3306;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::SERVER_PORT => 3306, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + ServerAttributes::SERVER_PORT => 3306, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'format with host but no port' => [ 'mysql:host=localhost;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + DbAttributes::DB_NAMESPACE => 'test', ], ], 'format with port but using alternative format' => [ 'mysql:localhost:3306;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::SERVER_PORT => 3306, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + ServerAttributes::SERVER_PORT => 3306, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'format with neither host parameter nor port parameter' => [ 'mysql:localhost;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + DbAttributes::DB_NAMESPACE => 'test', ], ], 'format with IP address as host' => [ 'mysql:host=127.0.0.1;port=3306;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => '127.0.0.1', - TraceAttributes::SERVER_PORT => 3306, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => '127.0.0.1', + ServerAttributes::SERVER_PORT => 3306, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'format with domain name as host' => [ 'mysql:host=example.com;port=3306;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'example.com', - TraceAttributes::SERVER_PORT => 3306, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'example.com', + ServerAttributes::SERVER_PORT => 3306, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'PostgreSQL format' => [ 'pgsql:host=localhost;port=5432;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::SERVER_PORT => 5432, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + ServerAttributes::SERVER_PORT => 5432, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'SQLite format' => [ 'sqlite:/path/to/database.sqlite', [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', - TraceAttributes::DB_NAMESPACE => '/path/to/database.sqlite', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_NAMESPACE => '/path/to/database.sqlite', ], ], 'SQLite in-memory format' => [ 'sqlite::memory:', [ - TraceAttributes::DB_SYSTEM_NAME => 'sqlite', - TraceAttributes::DB_NAMESPACE => 'memory', + DbAttributes::DB_SYSTEM_NAME => 'sqlite', + DbAttributes::DB_NAMESPACE => 'memory', ], ], 'Oracle format' => [ 'oci:host=localhost;port=1521;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::SERVER_PORT => 1521, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + ServerAttributes::SERVER_PORT => 1521, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'SQL Server format' => [ 'sqlsrv:Server=localhost,1433;Database=test', [ - TraceAttributes::SERVER_ADDRESS => 'localhost', - TraceAttributes::SERVER_PORT => 1433, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => 'localhost', + ServerAttributes::SERVER_PORT => 1433, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'MySQL format with host in DSN prefix' => [ 'mysql:dbname=test;charset=utf8', [ - TraceAttributes::DB_NAMESPACE => 'test', + DbAttributes::DB_NAMESPACE => 'test', ], ], 'MySQL format with host in DSN prefix and port' => [ 'mysql:dbname=test;port=3307;charset=utf8', [ - TraceAttributes::SERVER_PORT => 3307, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_PORT => 3307, + DbAttributes::DB_NAMESPACE => 'test', ], ], 'MySQL format with host in DSN prefix and colon port' => [ 'mysql:127.0.0.1:3308;dbname=test', [ - TraceAttributes::SERVER_ADDRESS => '127.0.0.1', - TraceAttributes::SERVER_PORT => 3308, - TraceAttributes::DB_NAMESPACE => 'test', + ServerAttributes::SERVER_ADDRESS => '127.0.0.1', + ServerAttributes::SERVER_PORT => 3308, + DbAttributes::DB_NAMESPACE => 'test', ], ], ]; From e3ad9ca5fee1f22943dff1dfc6474fb5dd860be6 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Tue, 2 Sep 2025 16:49:05 -0700 Subject: [PATCH 33/40] Corrected the reference in the README.md --- src/Instrumentation/PDO/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index 3a98152d5..fddbc894b 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -36,7 +36,7 @@ OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS=true ``` ### SQL Commenter feature -The [sqlcommenter](https://google.github.io/sqlcommenter/) feature can be enabled using configuration directive, currently it can be used with `postgresql` and `mysql` drivers only. +The [sqlcommenter](https://opentelemetry.io/docs/specs/semconv/database/database-spans/#sql-commenter) feature can be enabled using configuration directive, currently it can be used with `postgresql` and `mysql` drivers only. ``` otel.instrumentation.pdo.context_propagation = true ``` From b1686cdcde75b30ac94e4e3844c494701eca1b33 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Thu, 4 Sep 2025 14:29:40 -0700 Subject: [PATCH 34/40] Updated --- src/Instrumentation/PDO/README.md | 5 ++ .../PDO/src/ContextInfoPropagator.php | 14 ---- .../PDO/src/ContextPropagatorFactory.php | 65 +++++++++++++++++++ .../PDO/src/PDOInstrumentation.php | 65 +++++++++++-------- ...ropagator.php => QueryCommentInjector.php} | 2 +- ...terface.php => QueryInjectorInterface.php} | 2 +- src/Instrumentation/PDO/src/Variables.php | 10 +++ .../tests/Unit/ContextInfoPropagatorTest.php | 24 ------- ...torTest.php => SqlCommentInjectorTest.php} | 12 ++-- 9 files changed, 127 insertions(+), 72 deletions(-) delete mode 100644 src/Instrumentation/PDO/src/ContextInfoPropagator.php create mode 100644 src/Instrumentation/PDO/src/ContextPropagatorFactory.php rename src/Instrumentation/PDO/src/{SqlCommentPropagator.php => QueryCommentInjector.php} (94%) rename src/Instrumentation/PDO/src/{SqlPropagatorInterface.php => QueryInjectorInterface.php} (83%) create mode 100644 src/Instrumentation/PDO/src/Variables.php delete mode 100644 src/Instrumentation/PDO/tests/Unit/ContextInfoPropagatorTest.php rename src/Instrumentation/PDO/tests/Unit/{SqlCommentPropagatorTest.php => SqlCommentInjectorTest.php} (78%) diff --git a/src/Instrumentation/PDO/README.md b/src/Instrumentation/PDO/README.md index fddbc894b..a2d624e14 100644 --- a/src/Instrumentation/PDO/README.md +++ b/src/Instrumentation/PDO/README.md @@ -45,6 +45,11 @@ or environment variable: OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATION=true ``` +The context sources from global propagator by default, but it can be configured using the following environment variables: +```shell +OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS=tracecontext +``` + The modified query statement by default will not update `DbAttributes::DB_QUERY_TEXT` due to high cardinality risk, but it can be configured using the following configuration directive: ``` otel.instrumentation.pdo.context_propagation.attribute = true diff --git a/src/Instrumentation/PDO/src/ContextInfoPropagator.php b/src/Instrumentation/PDO/src/ContextInfoPropagator.php deleted file mode 100644 index 8dc832eed..000000000 --- a/src/Instrumentation/PDO/src/ContextInfoPropagator.php +++ /dev/null @@ -1,14 +0,0 @@ - null, + 1 => $this->buildPropagator($propagators[0]), + default => function() use ($propagators) { + $props = $this->buildPropagators($propagators); + if ($props) { + return new MultiTextMapPropagator($props); + } + return null; + }, + }; + } + + /** + * @return ?list + */ + private function buildPropagators(array $names): ?array + { + $propagators = []; + foreach ($names as $name) { + $propagator = $this->buildPropagator($name); + if ($propagator !== null) { + $propagators[] = $propagator; + } + } + if (count($propagators) === 0) { + return null; + } + return $propagators; + } + + private function buildPropagator(string $name): ?TextMapPropagatorInterface + { + try { + return Registry::textMapPropagator($name); + } catch (\RuntimeException $e) { + self::logWarning($e->getMessage()); + } + + return null; + } +} \ No newline at end of file diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index eb434285d..3f4790101 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -33,6 +33,7 @@ public static function register(): void Version::VERSION_1_36_0->url(), ); $pdoTracker = new PDOTracker(); + $contextPropagator = (new ContextPropagatorFactory())->create(); // Hook for the new PDO::connect static method if (method_exists(PDO::class, 'connect')) { @@ -110,7 +111,7 @@ public static function register(): void hook( PDO::class, 'query', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($contextPropagator, $pdoTracker, $instrumentation) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); @@ -135,8 +136,15 @@ public static function register(): void case 'postgresql': case 'mysql': $comments = []; - Globals::propagator()->inject($comments); - $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); + if ($contextPropagator) { + // Propagator passed by user + $contextPropagator->inject($comments); + } else { + // fallback to global propagator if user didn't pass one + Globals::propagator()->inject($comments); + } + // Inject comments into SQL statement + $sqlStatement = QueryCommentInjector::inject($sqlStatement, $comments); if (ContextPropagation::isAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -163,7 +171,7 @@ public static function register(): void hook( PDO::class, 'exec', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($contextPropagator, $pdoTracker, $instrumentation) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); @@ -182,28 +190,33 @@ public static function register(): void Context::storage()->attach($span->storeInContext($parent)); if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { - if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { - /** @psalm-suppress PossiblyInvalidCast */ - switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { - case 'postgresql': - case 'mysql': - $comments = []; - Globals::propagator()->inject($comments); - $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); - if (ContextPropagation::isAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => $sqlStatement, - ]); - } - - return [ - 0 => $sqlStatement, - ]; - default: - // Do nothing, not a database we want to propagate - break; - } - } +// if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { +// /** @psalm-suppress PossiblyInvalidCast */ +// switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { +// case 'postgresql': +// case 'mysql': +// $comments = []; +// if ($this->contextPropagator) { +// $contextPropagator->inject($comments); +// } +// +// +// Globals::propagator()->inject($comments); +// $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); +// if (ContextPropagation::isAttributeEnabled()) { +// $span->setAttributes([ +// DbAttributes::DB_QUERY_TEXT => $sqlStatement, +// ]); +// } +// +// return [ +// 0 => $sqlStatement, +// ]; +// default: +// // Do nothing, not a database we want to propagate +// break; +// } +// } } return []; diff --git a/src/Instrumentation/PDO/src/SqlCommentPropagator.php b/src/Instrumentation/PDO/src/QueryCommentInjector.php similarity index 94% rename from src/Instrumentation/PDO/src/SqlCommentPropagator.php rename to src/Instrumentation/PDO/src/QueryCommentInjector.php index 622f40a73..65ff75dee 100644 --- a/src/Instrumentation/PDO/src/SqlCommentPropagator.php +++ b/src/Instrumentation/PDO/src/QueryCommentInjector.php @@ -6,7 +6,7 @@ use OpenTelemetry\SDK\Common\Configuration\Configuration; -class SqlCommentPropagator implements SqlPropagatorInterface +class QueryCommentInjector implements QueryInjectorInterface { public static function isPrepend(): bool { diff --git a/src/Instrumentation/PDO/src/SqlPropagatorInterface.php b/src/Instrumentation/PDO/src/QueryInjectorInterface.php similarity index 83% rename from src/Instrumentation/PDO/src/SqlPropagatorInterface.php rename to src/Instrumentation/PDO/src/QueryInjectorInterface.php index 1ced81d69..937428b57 100644 --- a/src/Instrumentation/PDO/src/SqlPropagatorInterface.php +++ b/src/Instrumentation/PDO/src/QueryInjectorInterface.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; -interface SqlPropagatorInterface +interface QueryInjectorInterface { public static function inject(string $query, array $comments): string; } diff --git a/src/Instrumentation/PDO/src/Variables.php b/src/Instrumentation/PDO/src/Variables.php new file mode 100644 index 000000000..5c54d672d --- /dev/null +++ b/src/Instrumentation/PDO/src/Variables.php @@ -0,0 +1,10 @@ + 'value1', - 'key2' => 'value2', - 'key3' => 'value3', - ]; - $query = 'SELECT 1;'; - $result = ContextInfoPropagator::inject($query, $comments); - $this->assertEquals('SELECT 1;', $result); - } - -} diff --git a/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php b/src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php similarity index 78% rename from src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php rename to src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php index 27ee25b6e..2f46e2cc5 100644 --- a/src/Instrumentation/PDO/tests/Unit/SqlCommentPropagatorTest.php +++ b/src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php @@ -4,22 +4,22 @@ namespace OpenTelemetry\Tests\Instrumentation\PDO\tests\Unit; -use OpenTelemetry\Contrib\Instrumentation\PDO\SqlCommentPropagator; +use OpenTelemetry\Contrib\Instrumentation\PDO\QueryCommentInjector; use PHPUnit\Framework\TestCase; -class SqlCommentPropagatorTest extends TestCase +class SqlCommentInjectorTest extends TestCase { public function testIsPrependReturnsTrue() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = true; - $result = SqlCommentPropagator::isPrepend(); + $result = QueryCommentInjector::isPrepend(); $this->assertTrue($result); } public function testIsPrependReturnsFalse() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = false; - $result = SqlCommentPropagator::isPrepend(); + $result = QueryCommentInjector::isPrepend(); $this->assertFalse($result); } @@ -32,7 +32,7 @@ public function testInjectPrepend() 'key3' => 'value3', ]; $query = 'SELECT 1;'; - $result = SqlCommentPropagator::inject($query, $comments); + $result = QueryCommentInjector::inject($query, $comments); $this->assertEquals("/*key1='value1',key2='value2',key3='value3'*/SELECT 1;", $result); } @@ -45,7 +45,7 @@ public function testInjectAppend() 'key3' => 'value3', ]; $query = 'SELECT 1;'; - $result = SqlCommentPropagator::inject($query, $comments); + $result = QueryCommentInjector::inject($query, $comments); $this->assertEquals("SELECT 1/*key1='value1',key2='value2',key3='value3'*/;", $result); } } From 44bb5ed26724489e76657cc70906baf946f3fb49 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Thu, 4 Sep 2025 14:53:30 -0700 Subject: [PATCH 35/40] Fixed --- .../PDO/src/ContextPropagatorFactory.php | 22 +++---- .../PDO/src/PDOInstrumentation.php | 60 +++++++++---------- src/Instrumentation/PDO/src/Variables.php | 2 +- 3 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php index 8c6b27c6c..233e3e80d 100644 --- a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php +++ b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php @@ -14,23 +14,24 @@ class ContextPropagatorFactory { use LogsMessagesTrait; - public function create(): ?TextMapPropagatorInterface + public function create() { $propagators = []; if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { - $propagators = Configuration::getList(Variables::OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS); + $propagators = Configuration::getList(Variables::OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS, []); } return match (count($propagators)) { 0 => null, 1 => $this->buildPropagator($propagators[0]), - default => function() use ($propagators) { - $props = $this->buildPropagators($propagators); - if ($props) { - return new MultiTextMapPropagator($props); - } - return null; - }, + default => function () use ($propagators) { + $props = $this->buildPropagators($propagators); + if ($props) { + return new MultiTextMapPropagator($props); + } + + return null; + }, }; } @@ -49,6 +50,7 @@ private function buildPropagators(array $names): ?array if (count($propagators) === 0) { return null; } + return $propagators; } @@ -62,4 +64,4 @@ private function buildPropagator(string $name): ?TextMapPropagatorInterface return null; } -} \ No newline at end of file +} diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 3f4790101..6e91e7a06 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -171,7 +171,7 @@ public static function register(): void hook( PDO::class, 'exec', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($contextPropagator, $pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); @@ -189,35 +189,35 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { -// if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { -// /** @psalm-suppress PossiblyInvalidCast */ -// switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { -// case 'postgresql': -// case 'mysql': -// $comments = []; -// if ($this->contextPropagator) { -// $contextPropagator->inject($comments); -// } -// -// -// Globals::propagator()->inject($comments); -// $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); -// if (ContextPropagation::isAttributeEnabled()) { -// $span->setAttributes([ -// DbAttributes::DB_QUERY_TEXT => $sqlStatement, -// ]); -// } -// -// return [ -// 0 => $sqlStatement, -// ]; -// default: -// // Do nothing, not a database we want to propagate -// break; -// } -// } - } + // if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { + // if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { + // /** @psalm-suppress PossiblyInvalidCast */ + // switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { + // case 'postgresql': + // case 'mysql': + // $comments = []; + // if ($this->contextPropagator) { + // $contextPropagator->inject($comments); + // } + // + // + // Globals::propagator()->inject($comments); + // $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); + // if (ContextPropagation::isAttributeEnabled()) { + // $span->setAttributes([ + // DbAttributes::DB_QUERY_TEXT => $sqlStatement, + // ]); + // } + // + // return [ + // 0 => $sqlStatement, + // ]; + // default: + // // Do nothing, not a database we want to propagate + // break; + // } + // } + // } return []; }, diff --git a/src/Instrumentation/PDO/src/Variables.php b/src/Instrumentation/PDO/src/Variables.php index 5c54d672d..65abf0612 100644 --- a/src/Instrumentation/PDO/src/Variables.php +++ b/src/Instrumentation/PDO/src/Variables.php @@ -7,4 +7,4 @@ interface Variables { public const OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS = 'OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS'; -} \ No newline at end of file +} From 4c0d9cd8feee8e131ae2dee061fa85723ffecc86 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Thu, 4 Sep 2025 15:31:01 -0700 Subject: [PATCH 36/40] updated --- .../PDO/src/ContextPropagatorFactory.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php index 233e3e80d..cee5b524d 100644 --- a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php +++ b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php @@ -14,25 +14,26 @@ class ContextPropagatorFactory { use LogsMessagesTrait; - public function create() + public function create(): TextMapPropagatorInterface | null { $propagators = []; if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { $propagators = Configuration::getList(Variables::OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS, []); } - return match (count($propagators)) { - 0 => null, - 1 => $this->buildPropagator($propagators[0]), - default => function () use ($propagators) { + switch (count($propagators)) { + case 0: + return null; + case 1: + return $this->buildPropagator($propagators[0]); + default: $props = $this->buildPropagators($propagators); if ($props) { return new MultiTextMapPropagator($props); } return null; - }, - }; + } } /** From ef7c00f7f119d72254dff3c01f3bd1bdd07dab9d Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Thu, 4 Sep 2025 15:51:06 -0700 Subject: [PATCH 37/40] Updated --- .../PDO/src/ContextPropagatorFactory.php | 2 +- src/Instrumentation/PDO/src/Variables.php | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) delete mode 100644 src/Instrumentation/PDO/src/Variables.php diff --git a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php index cee5b524d..ffb7e6eea 100644 --- a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php +++ b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php @@ -18,7 +18,7 @@ public function create(): TextMapPropagatorInterface | null { $propagators = []; if (class_exists('OpenTelemetry\SDK\Common\Configuration\Configuration')) { - $propagators = Configuration::getList(Variables::OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS, []); + $propagators = Configuration::getList('OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS', []); } switch (count($propagators)) { diff --git a/src/Instrumentation/PDO/src/Variables.php b/src/Instrumentation/PDO/src/Variables.php deleted file mode 100644 index 65abf0612..000000000 --- a/src/Instrumentation/PDO/src/Variables.php +++ /dev/null @@ -1,10 +0,0 @@ - Date: Thu, 4 Sep 2025 16:52:06 -0700 Subject: [PATCH 38/40] Updated --- .../PDO/src/ContextPropagatorFactory.php | 10 +++- .../Unit/ContextPropagatorFactoryTest.php | 54 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php diff --git a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php index ffb7e6eea..4af56e444 100644 --- a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php +++ b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php @@ -6,6 +6,7 @@ use OpenTelemetry\API\Behavior\LogsMessagesTrait; use OpenTelemetry\Context\Propagation\MultiTextMapPropagator; +use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\SDK\Common\Configuration\Configuration; use OpenTelemetry\SDK\Registry; @@ -25,7 +26,12 @@ public function create(): TextMapPropagatorInterface | null case 0: return null; case 1: - return $this->buildPropagator($propagators[0]); + $propagator = $this->buildPropagator($propagators[0]); + if (is_a($propagator, NoopTextMapPropagator::class)) { + return null; + } + + return $propagator; default: $props = $this->buildPropagators($propagators); if ($props) { @@ -44,7 +50,7 @@ private function buildPropagators(array $names): ?array $propagators = []; foreach ($names as $name) { $propagator = $this->buildPropagator($name); - if ($propagator !== null) { + if ($propagator !== null && !is_a($propagator, NoopTextMapPropagator::class)) { $propagators[] = $propagator; } } diff --git a/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php b/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php new file mode 100644 index 000000000..98e48ebbf --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php @@ -0,0 +1,54 @@ +create(); + if ($expected === null) { + $this->assertNull($propagator); + } else { + $this->assertInstanceOf($expected, $propagator); + } + putenv('OTEL_PHP_INSTRUMENTATION_PDO_CONTEXT_PROPAGATORS'); + } + + public static function propagatorsProvider(): array + { + return [ + [KnownValues::VALUE_BAGGAGE, BaggagePropagator::class], + [KnownValues::VALUE_TRACECONTEXT, TraceContextPropagator::class], + [KnownValues::VALUE_NONE, null], + [sprintf('%s,%s', KnownValues::VALUE_TRACECONTEXT, KnownValues::VALUE_BAGGAGE), MultiTextMapPropagator::class], + ['', null], + ['invalid', null], + ]; + } +} From 97aff4e59da487f424dc969296ffb6a5caf6440a Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Thu, 4 Sep 2025 16:58:24 -0700 Subject: [PATCH 39/40] Updated --- .../PDO/src/ContextPropagatorFactory.php | 2 +- .../PDO/src/PDOInstrumentation.php | 66 ++++++++++--------- ...entInjector.php => SqlCommentInjector.php} | 2 +- ...Interface.php => SqlInjectorInterface.php} | 2 +- .../Unit/ContextPropagatorFactoryTest.php | 2 - .../PDO/tests/Unit/SqlCommentInjectorTest.php | 10 +-- 6 files changed, 42 insertions(+), 42 deletions(-) rename src/Instrumentation/PDO/src/{QueryCommentInjector.php => SqlCommentInjector.php} (94%) rename src/Instrumentation/PDO/src/{QueryInjectorInterface.php => SqlInjectorInterface.php} (83%) diff --git a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php index 4af56e444..2db4a7624 100644 --- a/src/Instrumentation/PDO/src/ContextPropagatorFactory.php +++ b/src/Instrumentation/PDO/src/ContextPropagatorFactory.php @@ -27,7 +27,7 @@ public function create(): TextMapPropagatorInterface | null return null; case 1: $propagator = $this->buildPropagator($propagators[0]); - if (is_a($propagator, NoopTextMapPropagator::class)) { + if ($propagator !== null && is_a($propagator, NoopTextMapPropagator::class)) { return null; } diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 6e91e7a06..e8c4fb18e 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -136,7 +136,7 @@ public static function register(): void case 'postgresql': case 'mysql': $comments = []; - if ($contextPropagator) { + if ($contextPropagator !== null) { // Propagator passed by user $contextPropagator->inject($comments); } else { @@ -144,7 +144,7 @@ public static function register(): void Globals::propagator()->inject($comments); } // Inject comments into SQL statement - $sqlStatement = QueryCommentInjector::inject($sqlStatement, $comments); + $sqlStatement = SqlCommentInjector::inject($sqlStatement, $comments); if (ContextPropagation::isAttributeEnabled()) { $span->setAttributes([ DbAttributes::DB_QUERY_TEXT => $sqlStatement, @@ -171,7 +171,7 @@ public static function register(): void hook( PDO::class, 'exec', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($contextPropagator, $pdoTracker, $instrumentation) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); @@ -189,35 +189,37 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - // if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { - // if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { - // /** @psalm-suppress PossiblyInvalidCast */ - // switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { - // case 'postgresql': - // case 'mysql': - // $comments = []; - // if ($this->contextPropagator) { - // $contextPropagator->inject($comments); - // } - // - // - // Globals::propagator()->inject($comments); - // $sqlStatement = SqlCommentPropagator::inject($sqlStatement, $comments); - // if (ContextPropagation::isAttributeEnabled()) { - // $span->setAttributes([ - // DbAttributes::DB_QUERY_TEXT => $sqlStatement, - // ]); - // } - // - // return [ - // 0 => $sqlStatement, - // ]; - // default: - // // Do nothing, not a database we want to propagate - // break; - // } - // } - // } + if (ContextPropagation::isEnabled() && $sqlStatement !== self::UNDEFINED) { + if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { + /** @psalm-suppress PossiblyInvalidCast */ + switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { + case 'postgresql': + case 'mysql': + $comments = []; + if ($contextPropagator !== null) { + // Propagator passed by user + $contextPropagator->inject($comments); + } else { + // fallback to global propagator if user didn't pass one + Globals::propagator()->inject($comments); + } + + $sqlStatement = SqlCommentInjector::inject($sqlStatement, $comments); + if (ContextPropagation::isAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => $sqlStatement, + ]); + } + + return [ + 0 => $sqlStatement, + ]; + default: + // Do nothing, not a database we want to propagate + break; + } + } + } return []; }, diff --git a/src/Instrumentation/PDO/src/QueryCommentInjector.php b/src/Instrumentation/PDO/src/SqlCommentInjector.php similarity index 94% rename from src/Instrumentation/PDO/src/QueryCommentInjector.php rename to src/Instrumentation/PDO/src/SqlCommentInjector.php index 65ff75dee..a26d2b2aa 100644 --- a/src/Instrumentation/PDO/src/QueryCommentInjector.php +++ b/src/Instrumentation/PDO/src/SqlCommentInjector.php @@ -6,7 +6,7 @@ use OpenTelemetry\SDK\Common\Configuration\Configuration; -class QueryCommentInjector implements QueryInjectorInterface +class SqlCommentInjector implements SqlInjectorInterface { public static function isPrepend(): bool { diff --git a/src/Instrumentation/PDO/src/QueryInjectorInterface.php b/src/Instrumentation/PDO/src/SqlInjectorInterface.php similarity index 83% rename from src/Instrumentation/PDO/src/QueryInjectorInterface.php rename to src/Instrumentation/PDO/src/SqlInjectorInterface.php index 937428b57..61ebbdde7 100644 --- a/src/Instrumentation/PDO/src/QueryInjectorInterface.php +++ b/src/Instrumentation/PDO/src/SqlInjectorInterface.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; -interface QueryInjectorInterface +interface SqlInjectorInterface { public static function inject(string $query, array $comments): string; } diff --git a/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php b/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php index 98e48ebbf..930436c4e 100644 --- a/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php +++ b/src/Instrumentation/PDO/tests/Unit/ContextPropagatorFactoryTest.php @@ -11,10 +11,8 @@ use OpenTelemetry\Context\Propagation\MultiTextMapPropagator; use OpenTelemetry\Contrib\Instrumentation\PDO\ContextPropagatorFactory; use OpenTelemetry\SDK\Common\Configuration\KnownValues; -use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; -#[CoversClass(ContextPropagatorFactory::class)] class ContextPropagatorFactoryTest extends TestCase { #[\Override] diff --git a/src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php b/src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php index 2f46e2cc5..dad020751 100644 --- a/src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php +++ b/src/Instrumentation/PDO/tests/Unit/SqlCommentInjectorTest.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Tests\Instrumentation\PDO\tests\Unit; -use OpenTelemetry\Contrib\Instrumentation\PDO\QueryCommentInjector; +use OpenTelemetry\Contrib\Instrumentation\PDO\SqlCommentInjector; use PHPUnit\Framework\TestCase; class SqlCommentInjectorTest extends TestCase @@ -12,14 +12,14 @@ class SqlCommentInjectorTest extends TestCase public function testIsPrependReturnsTrue() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = true; - $result = QueryCommentInjector::isPrepend(); + $result = SqlCommentInjector::isPrepend(); $this->assertTrue($result); } public function testIsPrependReturnsFalse() { $_SERVER['OTEL_PHP_INSTRUMENTATION_PDO_SQL_COMMENTER_PREPEND'] = false; - $result = QueryCommentInjector::isPrepend(); + $result = SqlCommentInjector::isPrepend(); $this->assertFalse($result); } @@ -32,7 +32,7 @@ public function testInjectPrepend() 'key3' => 'value3', ]; $query = 'SELECT 1;'; - $result = QueryCommentInjector::inject($query, $comments); + $result = SqlCommentInjector::inject($query, $comments); $this->assertEquals("/*key1='value1',key2='value2',key3='value3'*/SELECT 1;", $result); } @@ -45,7 +45,7 @@ public function testInjectAppend() 'key3' => 'value3', ]; $query = 'SELECT 1;'; - $result = QueryCommentInjector::inject($query, $comments); + $result = SqlCommentInjector::inject($query, $comments); $this->assertEquals("SELECT 1/*key1='value1',key2='value2',key3='value3'*/;", $result); } } From a98eb56d30c110e4f9adc9c57f3f38ab4acec518 Mon Sep 17 00:00:00 2001 From: Jerry Leung Date: Fri, 5 Sep 2025 08:59:26 -0700 Subject: [PATCH 40/40] nits --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index e8c4fb18e..a7d97f6ba 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -203,7 +203,7 @@ public static function register(): void // fallback to global propagator if user didn't pass one Globals::propagator()->inject($comments); } - + // Inject comments into SQL statement $sqlStatement = SqlCommentInjector::inject($sqlStatement, $comments); if (ContextPropagation::isAttributeEnabled()) { $span->setAttributes([