Add custom instrumentation for user-defined class/method hooks by kaz29 · Pull Request #6 · kaz29/cakephp-otel-plugin

kaz29 · 2026-04-09T21:35:52Z

Allow users to register arbitrary class methods for automatic span generation via Configure or CustomInstrumentation::register(). Supports custom span names, SpanKind, static attributes, and dynamic attribute callbacks.

Allow users to register arbitrary class methods for automatic span generation via Configure or CustomInstrumentation::register(). Supports custom span names, SpanKind, static attributes, and dynamic attribute callbacks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds support for user-defined method hooks to automatically generate OpenTelemetry spans, configurable via CakePHP Configure or via a CustomInstrumentation registration API.

Changes:

Introduces HookDefinition and CustomInstrumentation to register/apply \OpenTelemetry\Instrumentation\hook() hooks for arbitrary class methods.
Loads hook definitions from Configure::read('OtelInstrumentation.hooks') during plugin bootstrap and applies them.
Adds unit + integration tests and expands README (English/Japanese) with usage docs.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/TestCase/Unit/Instrumentation/HookDefinitionTest.php	Adds unit tests for `HookDefinition` defaults and span-name resolution.
tests/TestCase/Integration/CustomInstrumentationTest.php	Adds integration tests verifying default/custom span behavior and dynamic attributes.
src/Plugin.php	Reads custom hook config and applies custom instrumentation during bootstrap.
src/Instrumentation/HookDefinition.php	Defines hook configuration object (class/method/spanName/kind/attributes/callback).
src/Instrumentation/CustomInstrumentation.php	Implements registration, config loading, and hook application logic.
README.md	Documents custom instrumentation (Configure + static registration) and options.
README.ja.md	Japanese documentation updates mirroring the new custom instrumentation feature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T21:39:42Z

src/Instrumentation/CustomInstrumentation.php

+    public static function apply(): void
+    {
+        if (self::$applied) {
+            return;
+        }
+        self::$applied = true;
+


CustomInstrumentation::apply() becomes a one-shot toggle (self::$applied). Because Plugin::bootstrap() calls apply() unconditionally, any later CustomInstrumentation::register()/add()/loadFromConfig() calls will never take effect. Consider either (a) making register/add/loadFromConfig immediately hook definitions when already applied, or (b) tracking which definitions have been hooked and allowing apply() to be called multiple times to apply newly registered hooks.

Copilot · 2026-04-09T21:39:43Z

src/Instrumentation/CustomInstrumentation.php

+    /**
+     * Load hook definitions from Configure-style array format.
+     *
+     * @param array<array{class: class-string, method: string, spanName?: string, kind?: int, attributes?: array<string, mixed>}> $configs


The loadFromConfig() phpdoc array shape does not include the 'attributeCallback' key, but the implementation reads $config['attributeCallback']. Update the phpdoc to include attributeCallback?: \Closure (or callable) so static analysis and IDE hints match behavior.

Suggested change

* @param array<array{class: class-string, method: string, spanName?: string, kind?: int, attributes?: array<string, mixed>}> $configs

* @param array<array{class: class-string, method: string, spanName?: string, kind?: int, attributes?: array<string, mixed>, attributeCallback?: \Closure}> $configs

Copilot · 2026-04-09T21:39:43Z

src/Instrumentation/CustomInstrumentation.php

+        foreach ($configs as $config) {
+            self::$definitions[] = new HookDefinition(
+                class: $config['class'],
+                method: $config['method'],
+                spanName: $config['spanName'] ?? null,
+                kind: $config['kind'] ?? SpanKind::KIND_INTERNAL,
+                attributes: $config['attributes'] ?? [],
+                attributeCallback: $config['attributeCallback'] ?? null,
+            );


loadFromConfig() assumes each config entry has 'class' and 'method' keys with correct types; misconfiguration will currently trigger undefined index notices / TypeErrors. Consider validating required keys and types (and throwing a clear exception or skipping invalid entries) to make Configure-based setup safer to use.

Copilot · 2026-04-09T21:39:43Z

src/Instrumentation/HookDefinition.php

+     * @param (\Closure(object, array, string, string): array<string, mixed>)|null $attributeCallback
+     *     Dynamic attributes callback: fn($instance, $params, $class, $function) => ['key' => 'value']


HookDefinition's attributeCallback phpdoc specifies an object instance parameter, but the hook pre-callback receives mixed $instance (and may be null for static methods). Consider updating the documented callback signature to object|null (or mixed) to match actual runtime values and avoid misleading users.

Suggested change

* @param (\Closure(object, array, string, string): array<string, mixed>)|null $attributeCallback

* Dynamic attributes callback: fn($instance, $params, $class, $function) => ['key' => 'value']

* @param (\Closure(object|null, array, string, string): array<string, mixed>)|null $attributeCallback

* Dynamic attributes callback: fn($instance, $params, $class, $function) => ['key' => 'value']; $instance may be null for static methods

Copilot · 2026-04-09T21:39:43Z

README.md

+    spanName: 'payment.charge',
+    kind: SpanKind::KIND_CLIENT,
+    attributes: ['payment.provider' => 'stripe'],
+    attributeCallback: fn($instance, $params) => [


The README example defines attributeCallback as fn($instance, $params) => ..., but the implementation invokes callbacks with 4 arguments ($instance, $params, $class, $function). In PHP 8 this will throw an ArgumentCountError (too many arguments). Update the example callback signature to accept the extra parameters (or use a variadic) to match runtime behavior.

Suggested change

attributeCallback: fn($instance, $params) => [

attributeCallback: fn($instance, $params, $class, $function) => [

Copilot · 2026-04-09T21:39:44Z

README.ja.md

+    spanName: 'payment.charge',
+    kind: SpanKind::KIND_CLIENT,
+    attributes: ['payment.provider' => 'stripe'],
+    attributeCallback: fn($instance, $params) => [


日本語 README の attributeCallback 例が fn($instance, $params) => ... になっていますが、実装側は 4 引数 ($instance, $params, $class, $function) でコールバックを呼び出します。PHP 8 では引数過多で ArgumentCountError になるため、例のシグネチャを 4 引数（または可変長）に合わせて修正してください。

Suggested change

attributeCallback: fn($instance, $params) => [

attributeCallback: fn($instance, $params, $class, $function) => [

Copilot · 2026-04-09T21:39:44Z

tests/TestCase/Integration/CustomInstrumentationTest.php

+            self::$instrumentationLoaded = true;
+        }
+    }
+


setUpBeforeClass() registers hooks and calls CustomInstrumentation::apply(), which mutates static global state for the rest of the test process. Add a tearDownAfterClass() (or similar) to call CustomInstrumentation::reset() so this test doesn't leak hooks into other tests / make order-dependent failures more likely.

Suggested change

public static function tearDownAfterClass(): void

{

CustomInstrumentation::reset();

self::$instrumentationLoaded = false;

parent::tearDownAfterClass();

}

Copilot · 2026-04-09T21:39:44Z

tests/TestCase/Integration/CustomInstrumentationTest.php

+    public static function setUpBeforeClass(): void
+    {
+        parent::setUpBeforeClass();
+        if (!self::$instrumentationLoaded) {
+            CustomInstrumentation::register(
+                DummyService::class,
+                'doWork',
+            );
+            CustomInstrumentation::register(


CustomInstrumentation::apply() is executed in setUpBeforeClass() before the per-test extension_loaded('opentelemetry') skip check. If the extension is missing (e.g., running with --ignore-platform-req=ext-opentelemetry), this may fatal before the test can be skipped. Consider guarding apply()/register with an extension_loaded check in setUpBeforeClass() as well.

- Allow hooks registered after apply() to be applied immediately - Add validation for required class/method keys in loadFromConfig() - Fix attributeCallback phpdoc to accept object|null for static methods - Add attributeCallback to loadFromConfig() phpdoc array shape - Fix README examples to use 4-argument callback signature - Add ext-opentelemetry guard in setUpBeforeClass() - Add tearDownAfterClass() to clean up static state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

kaz29 requested a review from Copilot April 9, 2026 21:35

kaz29 self-assigned this Apr 9, 2026

kaz29 added the enhancement New feature or request label Apr 9, 2026

Copilot started reviewing on behalf of kaz29 April 9, 2026 21:36 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

kaz29 and others added 3 commits April 10, 2026 07:42

fix

2e2e6f5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom instrumentation for user-defined class/method hooks#6

Add custom instrumentation for user-defined class/method hooks#6
kaz29 wants to merge 4 commits intomainfrom
feat/custom-instrumentation

kaz29 commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	* @param array<array{class: class-string, method: string, spanName?: string, kind?: int, attributes?: array<string, mixed>}> $configs
	* @param array<array{class: class-string, method: string, spanName?: string, kind?: int, attributes?: array<string, mixed>, attributeCallback?: \Closure}> $configs

		* @param (\Closure(object, array, string, string): array<string, mixed>)\|null $attributeCallback
		* Dynamic attributes callback: fn($instance, $params, $class, $function) => ['key' => 'value']

	attributeCallback: fn($instance, $params) => [
	attributeCallback: fn($instance, $params, $class, $function) => [

+    public static function tearDownAfterClass(): void
+    {
+        CustomInstrumentation::reset();
+        self::$instrumentationLoaded = false;
+        parent::tearDownAfterClass();
+    }

Conversation

kaz29 commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants