Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/23518 )
Change subject: IMPALA-14480: Optional OpenTelemetry DCHECKs ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/23518/7/be/src/observe/otel-flags-trace.cc File be/src/observe/otel-flags-trace.cc: http://gerrit.cloudera.org:8080/#/c/23518/7/be/src/observe/otel-flags-trace.cc@68 PS7, Line 68: // > I'd make this a DEFINE_bool_hidden. Done. Originally this flag was wrapped with a #ifndef NDEBUG which meant it was only in debug builds, but I did not want startup failures if this flag was specified a release build. http://gerrit.cloudera.org:8080/#/c/23518/7/be/src/observe/span-manager.cc File be/src/observe/span-manager.cc: http://gerrit.cloudera.org:8080/#/c/23518/7/be/src/observe/span-manager.cc@52 PS7, Line 52: #ifndef NDEBUG > I assume this avoids a unused variable warning? Yes, in release builds the FLAGS_otel_trace_exhaustive_dchecks is unused. http://gerrit.cloudera.org:8080/#/c/23518/7/be/src/observe/span-manager.cc@189 PS7, Line 189: if (FLAGS_otel_trace_exhaustive_dchecks) { > An alternative would be to use The idea behind this patch is to selectively enable and disable these aggressive DCHECKS when the binary is built in debug mode. Thus, the if statement is necessary because otherwise the DCHECK would fail when built in debug mode and FLAGS_otel_trace_exhaustive_dchecks is false. http://gerrit.cloudera.org:8080/#/c/23518/7/tests/custom_cluster/test_otel_trace.py File tests/custom_cluster/test_otel_trace.py: http://gerrit.cloudera.org:8080/#/c/23518/7/tests/custom_cluster/test_otel_trace.py@36 PS7, Line 36: "--otel_trace_exhaustive_dchecks --otel_file_flush_interval_ms=500 " \ > Is this going to continue to result in intermittent test failures? It shouldn't because adding this flag means that all DCHECKs being run today will still be run after this patch is merged. -- To view, visit http://gerrit.cloudera.org:8080/23518 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6507f3f0e23ecf7c2bece9a6b6c2d86bfac1e57 Gerrit-Change-Number: 23518 Gerrit-PatchSet: 8 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Tue, 25 Nov 2025 21:34:07 +0000 Gerrit-HasComments: Yes
