[Telemetry] Report Data Importer setup as usage.data_importer - #689
Merged
Conversation
Reports whether the Data Importer is set up, as an L3 "configured" boolean in the usage.* telemetry namespace: at least one import configuration is active. Active rather than merely existing - a disabled import is one somebody built and then switched off. Whether an import has run is the exercised fact and deliberately not what this key reports, so the namespace stays comparable bundle to bundle. Import configurations are Data Hub configurations of this bundle's adapter type, so the question is put to Data Hub's shared, memoised DataHubConfigurationUsage - the same read the Data Hub satellites use. That costs no additional statement and is location-aware: a probe against plugin_datahub_config would report "no configurations" for every customer whose Data Hub config lives in Symfony config files. An unreadable store yields null (unknown), never false. Requires the usage.* extension point on pimcore/pimcore 2026.x and DataHubConfigurationUsage on pimcore/data-hub 2026.x, neither tagged yet - hence both constraints move to ^2026.3, as the Data Hub satellite bundles already did. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The production adapter mapping that determines the reported usage state lacks regression coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds daily usage.data_importer telemetry based on active Data Hub importer configurations.
Changes:
- Adds the telemetry provider and Data Hub adapter.
- Registers services and raises required Pimcore/Data Hub versions.
- Adds provider-level unit tests.
Review contract:
- The Data Hub abstraction addresses configuration-location handling at the correct service boundary (
DataHubImportConfigurations.php:38-40). - The change is additive and introduces no public API break.
- Provider behavior is tested, but the adapter mapping itself remains untested (
DataImporterUsageProviderTest.php:54-59). - No documentation change appears necessary for this internal telemetry signal.
- Remaining risk: adapter-type drift could silently produce incorrect telemetry.
File summaries
| File | Description |
|---|---|
composer.json |
Requires telemetry-capable dependency versions. |
src/DependencyInjection/PimcoreDataImporterExtension.php |
Loads telemetry services. |
src/Resources/config/telemetry.yaml |
Registers and wires provider services. |
src/Telemetry/DataHubImportConfigurations.php |
Queries active importer configurations. |
src/Telemetry/DataImporterUsageProvider.php |
Exposes the usage key and state. |
src/Telemetry/ImportConfigurationsInterface.php |
Defines the configuration-query seam. |
tests/unit/Telemetry/DataImporterUsageProviderTest.php |
Tests provider delegation and tri-state behavior. |
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.
Review follow-up on #689: the provider tests mock the seam, so nothing exercised DataHubImportConfigurations or checked that the type it names is the one Data Hub stores import configurations under. DataHubImportConfigurationsTest seeds a configuration exactly as Data Hub's Dao stores it (settings-store scope pimcore_data_hub), reads it back through Configuration::getList() and checks the adapter: an active import is used, an inactive one is false rather than unknown, another adapter's active configuration is not an import, and the seeded type is the one this bundle registers under pimcore_data_hub.supported_types. Data Hub's Dao memoises the list in a private static for the lifetime of the process; the test resets it between cases. Seeded entries carry unique names and are removed in _after(). Co-Authored-By: Claude <noreply@anthropic.com>
SonarCloud flagged the ReflectionProperty write that cleared Data Hub's private static configuration-list memo between cases. Data Hub's Dao memoises the list for the lifetime of the process and offers no reset, so the test now keeps exactly one store-backed case, which runs against the first non-empty listing: an active import configuration seeded with the type this bundle registers under pimcore_data_hub.supported_types - read from that registration - as the only active configuration in the store. The adapter therefore reads true only if it asks Data Hub for exactly that type, which is the drift the review asked to pin. Whether an inactive configuration or another adapter's configuration counts is DataHubConfigurationUsage::hasActiveOfType()'s decision and is no longer re-tested from this bundle. A second, store-free test pins the registration itself to the identifier Data Hub knows this bundle by. Co-Authored-By: Claude <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the
usage.data_importerprovider for Pimcore product telemetry: a boolean answering "is the Data Importer set up" (L3 – configured), emitted once a day inside the maintenance snapshot.Set up means at least one import configuration is active. Active rather than merely existing: a disabled import is one somebody built and then switched off, which is the opposite of adoption. Whether an import has actually run is the exercised (L4) fact, lives in the last-execution table, and is deliberately not what this key reports, so the
usage.*namespace stays comparable bundle to bundle.How
Import configurations are Data Hub configurations of this bundle's adapter type (
dataImporterDataObject). So the question is put to Data Hub's shared, memoisedDataHubConfigurationUsage— the same instance the four Data Hub satellites already use (pimcore/data-hub#1138) — viahasActiveOfType().That is a deliberate choice over a table probe:
usage.data_hubanddatahub.*; this key is answered from that same pass.pimcore_data_hub.config_location.data_hubselects between the settings store and Symfony config files undervar/config/data_hub/. A probe againstplugin_datahub_configwould report "no configurations" for every customer on the file target — inventing an adoption gap rather than measuring one. Going through the configuration API gets both locations right.Layout:
DataImporterUsageProvider— theBundleUsageProviderInterfaceimplementation;null(unknown, key omitted) when the store could not be read, neverfalse.ImportConfigurationsInterface+DataHubImportConfigurations— the seam to Data Hub's read, holding the one adapter-type constant.telemetry.yaml, loaded from the extension. Data Hub is a hard dependency of this bundle, so the wiring is unconditional.Content-never: one boolean leaves the install. Configuration names are never emitted.
Verification
unitsuite, where the bundle's Codeception run finds them:DataImporterUsageProviderTest(4) — an active import is used; none active isfalse, not unknown; an unreadable store is passed through asnull; and the key. Mocks the seam.DataHubImportConfigurationsTest(2, one DB-backed) — the adapter exercised against the real Data Hub read: an import configuration is seeded exactly as Data Hub's Dao stores it (settings-store scopepimcore_data_hub), read back throughConfiguration::getList(), and the adapter's answer checked. The seed carries the type this bundle registers underpimcore_data_hub.supported_types, read from that registration, and is the only active configuration in the store — so the adapter readstrueonly if it asks Data Hub for exactly that type. A typo or drift indataImporterDataObjectfails this test while the mocked provider tests would still pass. The second test pins the registration itself to that identifier. One store-backed case deliberately: Data Hub's Dao memoises the configuration list for the process and offers no reset, so a second listing would still see the first one's store; whether an inactive or another adapter's configuration counts isDataHubConfigurationUsage::hasActiveOfType()'s decision, not this bundle's.usage.data_importerreadstrueon the demo install, and the provider resolves and is tagged in the container. The round trip was also run against the demo's booted kernel: a seeded active import is read back with the right type and turns the adaptertrue, and is gone after cleanup.src/Telemetry. php-cs-fixer not run locally; CI validates it.Requires
pimcore/pimcore: ^2026.3— theusage.*extension point (BundleUsageProviderInterface) is on core2026.x(commit d6491530d7) but not yet tagged.pimcore/data-hub: ^2026.3—DataHubConfigurationUsagelanded in [Telemetry] Report Data Hub adoption via usage.* and datahub.* data-hub#1138 on2026.x, not yet tagged.Both are the same bumps the four Data Hub satellite bundles already carry on their
2026.x.Refs pimcore/product-management#1414
🤖 Generated with Claude Code