feat(metrics): durable JDBC metrics extension (Scope 2)#4756
Open
obelix74 wants to merge 39 commits into
Open
Conversation
added 26 commits
April 10, 2026 07:26
Adds read-only REST endpoints at /api/metrics-reports/v1/ to expose persisted Iceberg scan and commit metrics without requiring direct database access. Changes: - spec/metrics-reports-service.yml: OpenAPI 3.0.3 spec for the new API - api/metrics-reports-service/: JAX-RS code generation module - polaris-core: TABLE_READ_METRICS privilege (id=103), LIST_TABLE_METRICS authorizable operation, default no-op read methods on MetricsPersistence, MetricsReportToken for keyset cursor pagination - persistence/relational-jdbc: JDBC implementations of listScanReports/ listCommitReports using DESC keyset pagination; MetricsModelUtils shared ObjectMapper/parseMetadata utility - runtime/service: MetricsReportsService — resolves catalog/namespace/table via PolarisResolutionManifest, authorizes with TABLE_READ_METRICS, delegates to MetricsPersistence; always bounded by LIMIT (default 100) - Tests: MetricsReportsServiceTest (including 403/404 scenarios), MetricsReportTokenTest, metadata round-trip tests for model classes
Restructures each report in the response from a flat object into a stable envelope with nested actor/request/object/payload sub-objects. This decouples the API shape from the DB schema and allows payload schemas to evolve independently without breaking clients. - MetricsReportsService: replace baseRecordFields + flat maps with per-type envelope builders (actor, request, object, payload.data) - spec/metrics-reports-service.yml: replace flat ScanMetricsReport / CommitMetricsReport with envelope schemas; add MetricsActor, MetricsRequest, ScanMetricsObject, CommitMetricsObject, ScanPayload/CommitPayload, ScanPayloadData/CommitPayloadData - Tests: add scanReportHasEnvelopeStructure and commitReportHasEnvelopeStructure to assert the nested shape - docs: add envelope structure example to telemetry.md
…tsService
- Remove flat inline errorResponse() helper; all 400s now throw IllegalArgumentException
so IcebergExceptionMapper produces a consistent Iceberg error envelope instead of
a divergent flat {message, type, code} body
- Replace BadRequestException (JAX-RS WebApplicationException) with
IllegalArgumentException in decodeNamespace so the same mapper handles it
- Guard Integer.parseInt in ModelScanMetricsReport.toRecord() against malformed
projected_field_ids tokens in the DB — silently skips non-numeric tokens
- Remove unused @nullable import from MetricsReportsService
- Add test for PATH_COULD_NOT_BE_FULLY_RESOLVED resolver status (namespace/table
path not found); update 5 test expectations to match exception-throwing contract
…esponse These are DB-internal surrogate IDs with no meaning to API clients, who already know the catalog, namespace, and table from the request URL. The snapshotId (an Iceberg-level concept) is retained in the object envelope as it identifies which snapshot the metrics apply to.
…w feedback - Replace executeSelectOverStream + AtomicReference with executeSelect, which returns List<T> directly and is simpler for single-threaded use - Remove the unused `columns` parameter from buildMetricsQuery; use SELECT * since the parameter was always ALL_COLUMNS per model
- spec: remove max page size (impl concern, not API concern) - spec: clarify "epoch milliseconds" → "Unix epoch milliseconds" - spec: remove securitySchemes (auth handled uniformly by Polaris) - spec: update namespace description to reference Polaris Iceberg REST API convention - Move MetricsReportToken from polaris-core to relational-jdbc, the sole consumer; register TokenType service descriptor there - Move MetricsReportsService into new extensions/metrics-reports/impl module following the OPA Authorizer pattern; wire as runtimeOnly in runtime/server so it is elective
…undException in unit tests
…porter Move the @beta signal to the actual SPI interfaces rather than only on MetricsPersistence, which is a BasePersistence concern not visible to callers of the service layer.
Introduces ScanMetricsReport, CommitMetricsReport, MetricsListResponse, MetricsReportActor, and MetricsReportRequest as lightweight Java records with @JsonProperty annotations, replacing the ad-hoc Map<String,Object> approach. This gives explicit control over JSON field names and structure and makes the response shape easier to reuse across call sites. Adds jackson-annotations dependency to the impl module (previously only available transitively through polaris-core).
Resolved conflicts in import statements: kept pagination/metrics additions from feature branch while adopting jspecify annotations and ObjectMapper imports from main.
…tence interfaces MetricsPersistence and PolarisMetricsManager were added after the JSpecify migration commit and still imported jakarta.annotation, which is not a declared dependency of polaris-core.
…ache#4397 TODOs - Change projectedFieldIds (List<Integer>) and projectedFieldNames (List<String>) from comma-separated strings to proper JSON arrays in ScanMetricsReport and the OpenAPI spec, avoiding secondary encoding inside JSON - Add TODO on totalDurationMs 0L ambiguity in ModelCommitMetricsReport, tying it to the planned nullable column fix in apache#4397 - Add TODO on listScanMetrics/listCommitMetrics in PolarisMetricsManager noting these route through the transitional aggregate surface and should be rerouted once apache#4397 moves MetricsPersistence to a standalone SPI
Kept polaris-extensions-metrics-reports alongside new entries added in main (auth-opa-tests, auth-ranger, federation extensions, spark modules).
…polaris-core, remove MetricsPersistence from BasePersistence Addresses architecture review feedback that the metrics reporting SPI was defined at the wrong layer (CDI-coupled in runtime/service instead of polaris-core), and that durable metrics persistence was bleeding through BasePersistence into every metastore backend. Scope 1 changes (this commit): - Add stable CDI-agnostic IcebergMetricsReporter SPI to polaris-core - Remove MetricsPersistence from BasePersistence extends clause - Remove PolarisMetricsManager from PolarisMetaStoreManager extends clause - Delete all durable-path code: JDBC models, converters, PersistingMetricsReporter - Add no-op (default) and log-only reporters to extensions/metrics-reports/impl - Stub REST read path in MetricsReportsService to return empty results - Fix namespace decoding to use NamespaceUtils.splitNamespace (canonical) - Add multi-level namespace test; fix CHANGELOG accuracy Durable JDBC metrics persistence is deferred to a follow-up extension module (extensions/metrics-reports/persistence/relational-jdbc) that will back the read API without touching BasePersistence.
…licts Conflicts resolved by keeping HEAD: metrics SPI classes deleted in Scope 1 refactor (MetricsPersistence removed from BasePersistence, MetricsReportPersistenceTest and PolarisMetricsManager removed) take precedence over main's edits to those files.
Main-branch merge brought in references to MetricsPersistence (PolarisCallContext.getMetricsPersistence, MetaStoreManagerFactory .getOrCreateMetricsPersistence, etc.) that Scope 1 had deleted. Restore the six SPI types as standalone interfaces decoupled from BasePersistence so all existing callers compile correctly.
6 tasks
… fix factory
JdbcBasePersistenceImpl still declared implements MetricsPersistence after
the Scope 1 refactor removed the import and all metric method bodies,
causing a cannot-find-symbol compile error at the class declaration line.
JdbcMetaStoreManagerFactory still passed JdbcBasePersistenceImpl to the
two-arg PolarisCallContext constructor (which requires P extends both
BasePersistence and MetricsPersistence) and returned it from
getOrCreateMetricsPersistence(), causing no-suitable-constructor and
incompatible-types errors.
Fixes:
- Remove MetricsPersistence from JdbcBasePersistenceImpl implements clause
- Use PolarisCallContext(realmContext, metaStore, new MetricsPersistence(){})
in bootstrap, purge, and bootstrap-check code paths
- Return a no-op MetricsPersistence from getOrCreateMetricsPersistence();
the real JDBC implementation is provided by the extension module in Scope 2
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
2174e2e to
1b5205e
Compare
… catalogName Java prohibits lambda parameters from shadowing enclosing-method parameters of the same name. The no-op metricsReporter lambda introduced in the Scope 1 refactor used catalogName as a parameter name inside a createHandler override whose method parameter is also catalogName, causing a compile error.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
1b5205e to
de05a82
Compare
1. Remove stale MetricsReportToken$MetricsReportTokenType entry from persistence/relational-jdbc service file — the class was deleted in Scope 1 but ServiceLoader still found the registration, causing ServiceConfigurationError during pagination in LocalIcebergCatalog tests. 2. Fix PolarisCallContext constructor calls in JDBC test classes — both AtomicMetastoreManagerWithJdbcBasePersistenceImplTest and JdbcGrantRecordsIdempotencyTest used the two-arg constructor, which requires the second arg to implement MetricsPersistence. Now that JdbcBasePersistenceImpl no longer implements MetricsPersistence, the calls must explicitly pass a no-op MetricsPersistence. 3. Add polaris-extensions-metrics-reports as a runtimeOnly dependency to runtime/service — IcebergCatalogHandlerFactory injects IcebergMetricsReporter via CDI; without this module the NoOpMetricsReporter and LoggingMetricsReporter beans are absent, causing CDI injection failures in reportMetrics tests and integration tests.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
de05a82 to
a3db6fc
Compare
…match application.properties application.properties sets polaris.iceberg-metrics.reporting.type=default, but LoggingMetricsReporter was annotated @Identifier("log") causing UnsatisfiedResolutionException at runtime. Rename identifier to "default" and update MetricsReportingConfiguration.type() default value to match. Also regenerate config doc for MetricsReportingConfiguration.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
a3db6fc to
e1b00d9
Compare
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
21b2c93 to
cf8eea0
Compare
…ics-reports/spi IcebergMetricsReporter was never used inside polaris-core itself — it is called from IcebergCatalogHandler in runtime/service, so polaris-core was not the right home. Extract it into a new, minimal extensions/metrics-reports/spi module so that: * polaris-core has no dependency on (or knowledge of) the Iceberg metrics SPI * downstream servers can opt out of the metrics extension without dragging in polaris-core machinery they do not need * the SPI boundary is clear: runtime/service and the impl/jdbc extension modules all declare an explicit dep on polaris-extensions-metrics-reports-spi MetricsRecordConverter (only used by the persisting JDBC reporter) is also removed from polaris-core; it will live in the jdbc extension module on the Scope 2 branch. runtime/service: change runtimeOnly -> testRuntimeOnly for polaris-extensions-metrics-reports so the impl is not silently pushed onto the runtime classpath of every downstream server that depends on runtime/service.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
cf8eea0 to
f57d803
Compare
…/service The @QuarkusIntegrationTest tests start a fully packaged application built from runtimeClasspath. testRuntimeOnly excludes the dependency from that packaged app, causing IcebergMetricsReporter CDI injection failures at startup and testSendMetricsReport() to fail with a non-204 response. runtimeOnly is required here so that LoggingMetricsReporter and NoOpMetricsReporter are available when the Quarkus app is packaged and started for integration tests.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
f57d803 to
7d519d5
Compare
The runtimeOnly dep on polaris-extensions-metrics-reports was leaking into downstream consumers' runtime classpaths. Instead, make the IcebergMetricsReporter CDI producer resilient: if no reporter is found for the configured type, log a warning and fall back to a no-op. For deployments, runtime/server declares its own runtimeOnly dep on the impl module so LoggingMetricsReporter is always present there. For runtime/service integration tests (@QuarkusIntegrationTest), the no-op fallback satisfies CDI injection and the metrics endpoint returns 204.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
7d519d5 to
5bab615
Compare
added 5 commits
June 16, 2026 11:53
…mments - Move NoOpMetricsReporter from extensions/metrics-reports/impl to the SPI module so any project with polaris-extensions-metrics-reports-spi on its classpath gets a built-in no-op without reinventing one - Add jandex + CDI/SmallRye annotation deps to the SPI module so Quarkus discovers the bean via classpath scanning - Change MetricsReportingConfiguration code default to 'no-op' (was 'default') so it always resolves to the SPI bean when no explicit type is set; production deployments continue to use type=default via application.properties (LoggingMetricsReporter) - Override type=no-op in application-test.properties and application-it.properties so unit and integration tests in runtime/service (which do not have the impl module on runtimeClasspath) always get the SPI no-op without needing the impl module - Revert ServiceProducers.metricsReporter() to simple .get() — the SPI no-op guarantees the lookup always succeeds for the configured defaults - Remove redundant testImplementation(jakarta.ws.rs.api) from impl build (already covered by implementation() at line 35) - Delete comments-only META-INF/services registration left after MetricsReportToken was removed in Scope 1
- Restore MetricsPersistence SPI types to polaris-core (MetricsPersistence, PolarisMetricsManager, MetricsRecordIdentity, ScanMetricsRecord, CommitMetricsRecord, MetricsRecordConverter) as standalone interfaces decoupled from BasePersistence - Add new extension module extensions/metrics-reports/persistence/relational-jdbc with JdbcMetricsPersistence (@ApplicationScoped) and PersistingMetricsReporter (@RequestScoped @Identifier("persisting")) - Add JDBC model classes (ModelScanMetricsReport, ModelCommitMetricsReport) with keyset-pagination support via MetricsReportToken - Wire MetricsReportsService to use Instance<MetricsPersistence> for real reads when the JDBC extension is on the classpath; falls back to empty list otherwise - Register new module in gradle/projects.main.properties and bom/build.gradle.kts
- Move Token$TokenType service registration to the extension module (polaris-extensions-metrics-reports-jdbc) where MetricsReportToken lives; remove the stale entry from polaris-relational-jdbc to avoid ServiceConfigurationError when running without the extension - Inject MetricsPersistence interface in PersistingMetricsReporter instead of concrete JdbcMetricsPersistence - Remove unused LOGGER from JdbcMetricsPersistence - Remove stale TODO comments and update class javadocs now that the durable read path is wired
…d JDBC helpers - Add iceberg-core, jspecify, postgresql as compile deps to the metrics-jdbc extension module (CommitReport/ScanReport are in iceberg-core, not iceberg-api) - Make DatasourceOperations.getDatabaseType() and QueryGenerator.getFullyQualifiedTableName() public so the extension module (different package) can call them
…s-core MetricsRecordConverter was only ever used by PersistingMetricsReporter in the JDBC extension module, so polaris-core was not the right home. Move it into the jdbc extension module (same package as PersistingMetricsReporter) and add an explicit dep on polaris-extensions-metrics-reports-spi.
obelix74
force-pushed
the
implement_metrics_jdbc_extension
branch
from
5bab615 to
69bf79f
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This PR adds
polaris-extensions-metrics-reports-jdbc, an optional CDI extension module that provides durable JDBC storage for Iceberg scan/commit metrics reports.MetricsPersistenceSPI inJdbcMetricsPersistence(@ApplicationScoped) using the existingDatasourceOperationspool and theSCAN_METRICS_REPORT/COMMIT_METRICS_REPORTtables (schema-v4)PersistingMetricsReporter(@RequestScoped,@Identifier("persisting")): converts IcebergScanReport/CommitReportto SPI records and delegates toMetricsPersistenceMetricsReportsServiceviaInstance<MetricsPersistence>: when the extension is on the classpath, list endpoints return real results with keyset pagination; absent → empty lists (graceful degradation)MetricsReportToken$MetricsReportTokenTypeviaMETA-INF/servicesin the extension module only, preventingServiceConfigurationErrorwhen running without the extensionKey design decisions
Instance<MetricsPersistence>— no compile dependency from the base module on the JDBC extensionMetricsReportTokenencodes(timestamp_ms DESC, report_id DESC)for stable ordering without offset driftServiceConfigurationErrorwhen the extension is absent from the classpathTest plan
./gradlew :extensions:metrics-reports:persistence:relational-jdbc:build— new module compiles and tests pass./gradlew :extensions:metrics-reports:impl:test—MetricsReportsServiceTestpasses./gradlew :persistence:relational-jdbc:build— base module unaffected