[Telemetry] Report import trigger counts and manual import starts

[martineiber]

Changes in this pull request

Part of pimcore/product-management#1408 (row 5).

Snapshot, data_importer.* (DataImporterSnapshotCollector): config_count_scheduled (a job schedule with a date, or a cron definition in the execution config) and config_count_manual (neither) over the active import configurations. ImportConfigurationsInterface gains activeExecutionConfigs(), implemented by the existing DataHubImportConfigurations over Data Hub's configuration listing. An unreadable listing leaves the keys absent.

Event, datahub.import_manual_started (ImportStartTelemetrySubscriber): one content-never event per import a person starts from Studio. It listens to ImportStartEvent (pre_response.data_importer.import_start), which only the Studio start service dispatches. Cron runs and the push API call the preparation service directly and never raise it; the PostPreparationEvent they share with manual starts is deliberately not subscribed, and a test pins that. Properties: adapter, surface, success. No import class is touched.

The datahub. prefix follows core's event-name taxonomy (no underscore in the first segment).

Additional info

• Verified locally with a kernel-less PHPUnit run against the demo autoloader (5 + 4 tests), PHPStan level 6 and this bundle's php-cs-fixer config; CI validates the Codeception suite.
• Live --dry-run on the demo: scheduled 1, manual 2.
• Depends only on core classes already on 2026.x; pimcore/pimcore already requires ^2026.3.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The new configuration-store extraction lacks a store-backed regression test.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds telemetry for import scheduling configuration and Studio-triggered manual starts.

Changes:

• Adds scheduled/manual configuration snapshot counts (DataImporterSnapshotCollector.php:48-64).
• Emits anonymized manual-start events (ImportStartTelemetrySubscriber.php:49-55).
• Registers services and adds focused unit tests.

Review contract:

• Claim: Report trigger counts and Studio manual starts.
• Root cause/boundary: Implemented at the configuration adapter and Studio event seam (DataHubImportConfigurations.php:52-72, ImportStartTelemetrySubscriber.php:42-55).
• Call sites: Studio dispatches the subscribed event (ImportService.php:64-67); other preparation paths bypass it.
• Compatibility: The expanded interface is internal (ImportConfigurationsInterface.php:19-34).
• Tests: Collector and subscriber logic are covered, but the store-backed adapter behavior is not (DataHubImportConfigurations.php:52).
• Documentation: Telemetry contracts and registration are documented inline (telemetry.yaml:18-23).
• Remaining risk: The external relay allowlist dependency could not be verified.

File summaries

File	Description
src/Telemetry/DataHubImportConfigurations.php	Extracts active execution configurations.
src/Telemetry/ImportConfigurationsInterface.php	Adds the execution-configuration seam.
src/Telemetry/DataImporterSnapshotCollector.php	Produces scheduled/manual counts.
src/Telemetry/ImportStartTelemetrySubscriber.php	Captures Studio manual-start events.
src/Resources/config/telemetry.yaml	Registers telemetry services.
tests/unit/Telemetry/DataImporterSnapshotCollectorTest.php	Tests snapshot classification.
tests/unit/Telemetry/ImportStartTelemetrySubscriberTest.php	Tests event scope and payload.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Active configuration normalization can produce counts inconsistent with existing import and usage telemetry behavior.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Telemetry/DataHubImportConfigurations.php:90

• This applies different “active” semantics from both hasActive() and import execution. Configuration::isActive() may return a string, but Data Hub’s usage reader casts it to bool and ImportPreparationService uses normal PHP truthiness; FILTER_VALIDATE_BOOLEAN instead treats values such as 'false' as inactive. The same configuration can therefore make hasActive() report true while being omitted from these counts. Match the existing (bool) semantics (and cover the differing string case in this test).

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[martineiber]

Re the suppressed comment on DataHubImportConfigurations::isActive() (no thread to reply in): fixed in 6459db3. The reader now applies the same (bool) cast as DataHubConfigurationUsage and the truthiness ImportPreparationService checks, so a configuration stored with the string false is active here exactly as it is for an import run. testReadsTheActiveFlagWithTheProductsTruthiness pins on, 1, false as active and 0, empty, false (bool) as inactive. The same alignment goes to the file-export and simple-rest readers in their PRs.

[Copilot]

🟢 Approved

The telemetry boundaries, privacy constraints, failure semantics, and regression coverage are consistent with the stated requirements.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Schedule classification can report configurations that the scheduler rejects, and adapter registration drift is no longer tested.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/Telemetry/DataImporterSnapshotCollector.php:83

• hasValue() does not match the scheduler behavior this metric claims to represent. For example, the stored string '0' is counted as scheduled here, but SchedulerFactory rejects it via empty() at src/Processing/Scheduler/SchedulerFactory.php:32,41, so that configuration cannot fire. Please share the trigger predicate with the scheduler-owning code (including schedule-value validation) instead of maintaining a looser telemetry copy; otherwise malformed/file-managed configurations inflate config_count_scheduled.
  tests/unit/Telemetry/DataHubImportConfigurationsTest.php:44
• This replacement test hard-codes dataImporterDataObject, while the patch removes the prior assertion that read the bundle's registered supported type. That leaves src/Resources/config/pimcore/config.yml:26 free to drift from DataHubImportConfigurations::ADAPTER_TYPE; both telemetry paths would then silently report no imports while these tests still pass. Please retain a focused registration-to-reader regression test.

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[martineiber]

Re the two suppressed comments (no threads to reply in), fixed in 48aaa3b.

1. DataImporterSnapshotCollector: the schedule predicate now mirrors SchedulerFactory exactly - the values PHP's empty() covers (null, "", "0", 0, false, []) are unscheduled, so a stored "0" no longer inflates config_count_scheduled. It is spelled out in the collector rather than shared with the scheduler because these telemetry PRs deliberately do not touch the scheduler-owning code; the test carries the "0" cases for both cron and job schedules.

2. DataHubImportConfigurationsTest: you are right, and this one was my mistake - c9b577a replaced the existing store-backed test instead of extending it, which dropped the registration-to-reader regression. The original test (seeding the settings store with the type read from config.yml, asserting hasActive() and pinning the registered identifier) is restored verbatim from 2026.x, and the kernel-less cases through the injected listing now sit next to it in the same class.

[Copilot]

🟢 Approved

The implementation matches the stated telemetry scope and has focused regression coverage without unresolved correctness issues.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🟢 Approved

The implementation is correctly scoped, privacy-preserving, and covered by focused regression tests.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced