kfaraz commented on code in PR #18569:
URL: https://github.com/apache/druid/pull/18569#discussion_r2380828591
##########
sql/src/main/java/org/apache/druid/sql/http/SqlResource.java:
##########
@@ -97,7 +98,8 @@ public SqlResource(
final SqlEngineRegistry sqlEngineRegistry,
final SqlResourceQueryResultPusherFactory resultPusherFactory,
final DefaultQueryConfig defaultQueryConfig,
- final ServerConfig serverConfig
+ final ServerConfig serverConfig,
+ final QueryCountStatsProvider counter
Review Comment:
Agreed, we should address the core problem of publishing SQL metrics, but
not at the cost of affecting existing metric values, with no way to tell
whether a metric is originating from SQL or native.
The refactor can definitely be done in a follow up PR, but in that case, we
should use the multiple bindings approach in this PR. The multiple bindings
approach would have a very small footprint that can be easily removed later.
If we just a single instance with no dimensions in this PR,
I fear we might miss the Druid 35 code freeze (around Oct 7) and end up
releasing a partial (possibly regressive) feature.
I would rather hold off on this PR until we have a complete solution, since
we have never emitted SQL metrics anyway.
Let me know if you agree.
--
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]
