moomindani commented on PR #16250: URL: https://github.com/apache/iceberg/pull/16250#issuecomment-4589460958
**dev@ [DISCUSS] thread update — consensus reached** The [DISCUSS] thread on dev@ ([link](https://lists.apache.org/thread/vn4gglocg2g40p69mfrrh86qzkn1rr4b)) has converged: - **Q1 (dependency):** No objection to adding `opentelemetry-api` as `compileOnly` in iceberg-core. Ryan Blue confirmed it's fine — the new dependency-leak checks guard the runtime Jars, and the usability concern is addressed by the docs change (`opentelemetry-api` is the only thing on the runtime classpath; the host owns the SDK and the metric exporter, slf4j/log4j-style). - **Q2 (module placement):** No objection to keeping it in iceberg-core. - The high-cardinality point raised by Grant Nicholas is addressed by the `iceberg.otel.metrics.attributes` allowlist (`5d867e49d`), with a complementary framework-level table-name filter proposed in #16573. Span-based observability is deferred to a separate Issue/PR (semantic mismatch with the `MetricsReporter` callback). +1s from Ryan Blue, Yufei Gu, Talat Uyarer, vaquar khan, and Grant Nicholas. @jbonofre offered to review the PR. The dependency question that motivated the thread is settled, so this should be unblocked for review. Thanks all! -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
