lfrancke opened a new pull request, #2586: URL: https://github.com/apache/phoenix/pull/2586
### What changes were proposed in this pull request? > [!IMPORTANT] > This builds upon #2394 which has stalled for four months now. The four commits from @xavifeds8 are taken as is and later commits build on top of those four. Removes Apache HTrace from Phoenix and replaces it with a thin OpenTelemetry facade, `PhoenixTracing`, adapted from HBase's `TraceUtil` (HBASE-22120). Per the previous discussions on the mailing list and issue this doesn't depend on an opentelemetry SDK itself and instead relies on the one provided by HBase (2.5+). HTrace is banned going forward by maven-enforcer. **References** - JIRA: [PHOENIX-5215](https://issues.apache.org/jira/browse/PHOENIX-5215) - Supersedes #2394 (@xavifeds8); earlier attempts #1721 (@stoty, draft) and #1282 - Prior art: [HBASE-22120](https://issues.apache.org/jira/browse/HBASE-22120) - dev@: [[DISCUSS] OpenTelemetry support](https://lists.apache.org/thread/6y61s9j8yk1gpznwj0g4ptgv6l1h8jyz) - dev@: [Remove/replace HTrace (coordination)](https://lists.apache.org/thread/pxgs4t5gtjdfyx3kvbkz0zo9m1xvnh49) - dev@: [[DISCUSS] Migrating Phoenix tracing from Htrace to Opentelemetry](https://lists.apache.org/thread/20w1nhjvfnc3xoln970dnrwmf11r2l8q) Yes, this has been a long time coming :) **Changes on top of #2394** Rebase onto master: - `PhoenixMapReduceUtil.addPhoenixDependencyJars`, added to master after #2394 was written, still shipped `htrace-core` to YARN containers. - Removed three now-obsolete `htrace-core4` exclusions on the Hadoop dependencies. Hadoop 3.4.2 has no htrace, so they excluded nothing. Correctness: - Pinned `opentelemetry.version` to 1.15.0. #2394 used 1.49.0, but HBase 2.5.3, 2.5.10 and 2.6.1 ship 1.15.0 (2.5.14 and 2.6.3+ ship 1.49.0). Since the dependency is `provided`, compiling against a newer API than the runtime supplies risks `NoSuchMethodError`. Depending on the older version should be safer. - Stopped storing an OpenTelemetry `Scope` on `PhoenixConnection` and `TracingIterator`. A `Scope` restores a thread-local and must be closed on the thread that opened it, but a `Connection` is closed by an arbitrary thread (including the HA framework) and an iterator by whichever thread finishes with it. - Reverted two unrelated exception-handling changes in `MutationState`: a `catch (Throwable)` that wrapped `Error` into `SQLException`, and a `catch (RuntimeException)` that bypassed the index-metadata retry path. `MutationState` now differs from master only in tracing. - Credited HBase's `TraceUtil` properly in the `PhoenixTracing` javadoc, and fixed javadoc placement on the deprecated tracing properties, which sat after `@Deprecated` and so was not javadoc at all. ### Why are the changes needed? Phoenix's tracing has been non-functional for years, so this removes a dead dependency (with vulnerabilities) and paves the way for a more modern OpenTelemetry pipeline integrated into the stuff HBase provides ### Does this PR introduce _any_ user-facing change? Yes, though only to a feature that was already non-functional anyway (see above). **Removed: writing traces to an HBase table.** `PhoenixMetricsSink` and `TraceWriter` are gone, so `SYSTEM.TRACING_STATS` is no longer written. The sink configuration in `bin/hadoop-metrics2-phoenix.properties` and `bin/hadoop-metrics2-hbase.properties` is retained but commented out, with a note pointing at the OpenTelemetry agent. `SYSTEM.TRACING_STATS` itself, the tracing webapp and `bin/traceserver.py` are untouched, that is the scope of #1721. **Changed: `TRACE ON` is now inert.** The SQL grammar is untouched and the hook remains, so `TRACE ON` and `TRACE OFF` still parse and execute without error, but no Phoenix-side trace is started. Previously Phoenix owned a sampler and started its own root traces; under OpenTelemetry, sampling belongs to the SDK and the root span comes from whatever instrumented the caller. I decided to keep this PR to the removal of HTrace and replacing it with OTel. Making `TRACE ON` useful again can come in a follow-up. **Deprecated: Phoenix tracing properties.** `phoenix.trace.frequency`, `phoenix.trace.probability.threshold`, `phoenix.trace.enabled`, `phoenix.trace.batchSize`, `phoenix.trace.threadPoolSize`, `phoenix.trace.traceBufferSize` and `phoenix.trace.read.pagesize` are marked `@Deprecated` and no longer read. I'd also be happy to just remove them. Opinions welcome. OpenTelemetry is configured through the agent, for example `OTEL_EXPORTER_OTLP_ENDPOINT`. **Unchanged: everything else.** With no agent attached there is no behaviour change and no overhead. ### How was this patch tested? New `PhoenixTracingIT` covers the no-op path with no SDK present, `TRACE ON` / `TRACE OFF` round-tripping, and that no thread is left pinned to a stale `Context` after the connection closes. Locally, against the HBase 2.6 profile: - Full build green across all 15 modules, including checkstyle, RAT, maven-enforcer and dependency analysis. - Unit tests green. - `MutationStateIT` (27 tests) and `PhoenixTracingIT` (3 tests) green. The former covers the reverted `MutationState` exception handling. - `phoenix-pherf` including its integration tests green. - `mvn spotless:check` clean across all modules. Packaging, verified by inspecting the built artifacts: ### Was this patch authored or co-authored using generative AI tooling? Yes. Partially Generated-by: Claude Opus 5 (Claude Code) The four commits by @xavifeds8 are preserved unchanged from #2394; I cannot speak to the tooling used for those. The commits above them carry `Generated-by:` and `Co-authored-by:` trailers individually, but even those are partially human authored. I've attempted this whole exercise already two or so years ago.... Per the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html): `PhoenixTracing` is adapted from HBase's `TraceUtil`, which is Apache-2.0 and therefore compatible, and this derivation is stated in the class javadoc. -- 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]
