dimas-b commented on code in PR #3385:
URL: https://github.com/apache/polaris/pull/3385#discussion_r2850582318


##########
site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_persistence_metrics.md:
##########
@@ -0,0 +1,35 @@
+---
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+title: smallrye-polaris_persistence_metrics
+build:
+  list: never
+  render: never
+---
+
+Configuration for selecting the 
(`org.apache.polaris.core.persistence.metrics.MetricsPersistence`) 
implementation.  
+
+This configuration allows selecting the metrics persistence backend 
independently from the  entity metastore. Available types include:   
+
+ * `noop` (default) - No persistence, metrics are discarded    

Review Comment:
   > Some of the other changes suggested (removing of CDI beans produced by 
ServiceProducers) warrant that requestId and principalName be exposed via the 
SPI
   
   I'm saddened to see this PR having to go back-and-forth between previously 
discussed options.
   
   I do not wish to block or delay the merge of this valuable contribution, but 
I maintain my previously mentioned points that `requestId` and `principalName` 
like OTel data form a "cross-cutting concern" as far as the Metrics Persistence 
SPI goes. A caller of the SPI should normally not worry about the request ID 
since it is general part of the request handling infrastructure, not a specific 
feature of metrics persistence. The caller of the SPI should only be concerned 
with the metrics data that is intended to be persisted.
   
   CDI provides an excellent framework for injecting request-related (-scoped) 
information without exposing cross-cutting concerns in the metrics SPI.
   
   As to path forward, from a practical POV, I'd like to propose following 
@flyrain 's advice as far as schema files and versions are concerned, but I'd 
appreciate keeping the SPI and CDI framework that was achieved in this PR at 
the point of my last approval.



-- 
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