obelix74 commented on code in PR #3385:
URL: https://github.com/apache/polaris/pull/3385#discussion_r2850737665


##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -437,4 +443,75 @@ public PolarisMetricsReporter metricsReporter(
       MetricsReportingConfiguration config, @Any 
Instance<PolarisMetricsReporter> reporters) {
     return reporters.select(Identifier.Literal.of(config.type())).get();
   }
+
+  @Produces
+  @Identifier("noop")
+  @RequestScoped
+  public MetricsPersistence noopMetricsPersistence() {
+    return MetricsPersistence.NOOP;
+  }
+
+  /**
+   * Produces a {@link MetricsPersistence} instance for JDBC-based metrics 
storage.
+   *
+   * <p>This producer creates a new request-scoped {@link
+   * org.apache.polaris.persistence.relational.jdbc.JdbcMetricsPersistence} 
instance using the
+   * metrics datasource from the underlying JdbcBasePersistenceImpl. This 
approach avoids storing
+   * request-scoped state in the shared JdbcBasePersistenceImpl instance.
+   *
+   * <p>Configuration example:
+   *
+   * <pre>
+   * # Same database, different schema
+   * 
quarkus.datasource.metrics.jdbc.url=jdbc:postgresql://localhost:5432/polaris?currentSchema=metrics
+   *
+   * # Separate database
+   * 
quarkus.datasource.metrics.jdbc.url=jdbc:postgresql://localhost:5432/polaris_metrics
+   * </pre>
+   *
+   * @param metaStoreManagerFactory the metastore manager factory
+   * @param realmContext the realm context for the current request
+   * @param polarisPrincipal the authenticated principal for the current 
request (may be null)
+   * @param requestIdSupplier supplier for obtaining the server-generated 
request ID
+   * @return a MetricsPersistence implementation for JDBC
+   */
+  @Produces
+  @Identifier("relational-jdbc")
+  @RequestScoped
+  public MetricsPersistence jdbcMetricsPersistence(

Review Comment:
   In order to retain the SPI as suggested by Dmitri here: 
https://github.com/apache/polaris/pull/3385#discussion_r2850582318, I have to 
keep this method.



##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -437,4 +443,75 @@ public PolarisMetricsReporter metricsReporter(
       MetricsReportingConfiguration config, @Any 
Instance<PolarisMetricsReporter> reporters) {
     return reporters.select(Identifier.Literal.of(config.type())).get();
   }
+
+  @Produces
+  @Identifier("noop")
+  @RequestScoped
+  public MetricsPersistence noopMetricsPersistence() {
+    return MetricsPersistence.NOOP;
+  }
+
+  /**
+   * Produces a {@link MetricsPersistence} instance for JDBC-based metrics 
storage.
+   *
+   * <p>This producer creates a new request-scoped {@link
+   * org.apache.polaris.persistence.relational.jdbc.JdbcMetricsPersistence} 
instance using the
+   * metrics datasource from the underlying JdbcBasePersistenceImpl. This 
approach avoids storing
+   * request-scoped state in the shared JdbcBasePersistenceImpl instance.
+   *
+   * <p>Configuration example:
+   *
+   * <pre>
+   * # Same database, different schema
+   * 
quarkus.datasource.metrics.jdbc.url=jdbc:postgresql://localhost:5432/polaris?currentSchema=metrics
+   *
+   * # Separate database
+   * 
quarkus.datasource.metrics.jdbc.url=jdbc:postgresql://localhost:5432/polaris_metrics
+   * </pre>
+   *
+   * @param metaStoreManagerFactory the metastore manager factory
+   * @param realmContext the realm context for the current request
+   * @param polarisPrincipal the authenticated principal for the current 
request (may be null)
+   * @param requestIdSupplier supplier for obtaining the server-generated 
request ID
+   * @return a MetricsPersistence implementation for JDBC
+   */
+  @Produces
+  @Identifier("relational-jdbc")
+  @RequestScoped
+  public MetricsPersistence jdbcMetricsPersistence(
+      MetaStoreManagerFactory metaStoreManagerFactory,
+      RealmContext realmContext,
+      Instance<PolarisPrincipal> polarisPrincipal,
+      Instance<RequestIdSupplier> requestIdSupplier) {
+
+    BasePersistence persistence = 
metaStoreManagerFactory.getOrCreateSession(realmContext);
+    if (!(persistence instanceof JdbcBasePersistenceImpl jdbcPersistence)) {
+      return MetricsPersistence.NOOP;
+    }
+
+    // If no metrics datasource is configured, return NOOP
+    if (!jdbcPersistence.hasMetricsDatasource()) {
+      return MetricsPersistence.NOOP;
+    }
+
+    // Create a new request-scoped JdbcMetricsPersistence instance
+    // This avoids storing request-scoped state in the shared 
JdbcBasePersistenceImpl
+    PolarisPrincipal principal = polarisPrincipal.isResolvable() ? 
polarisPrincipal.get() : null;
+    RequestIdSupplier supplier =
+        requestIdSupplier.isResolvable() ? requestIdSupplier.get() : 
RequestIdSupplier.NOOP;
+
+    return new JdbcMetricsPersistence(
+        jdbcPersistence.getMetricsDatasourceOperations(),
+        realmContext.getRealmIdentifier(),
+        principal,
+        supplier);
+  }
+
+  @Produces
+  @RequestScoped
+  public MetricsPersistence metricsPersistence(

Review Comment:
   In order to retain the SPI as suggested by Dmitri here: 
https://github.com/apache/polaris/pull/3385#discussion_r2850582318, I have to 
keep this method.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to