Use publicly_disabled_labeled_metrics table for available_label_fqns - #5022
Use publicly_disabled_labeled_metrics table for available_label_fqns#5022IvanIvanoff wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughA new SQL filter helper function is added to exclude publicly-disabled labels from metric queries. The filter is imported and applied consistently across multiple query builders for available label and metric datetime computations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/sanbase/metric/sql_query_helper.ex`:
- Around line 215-249: The public function not_publicly_disabled_label_filter/1
is missing a `@spec` and an example in its `@doc`; add a typespec such as `@spec`
not_publicly_disabled_label_filter(keyword()) :: String.t() (or more specific
Keyword.t() -> String.t()) placed above the def, and update the `@doc` to include
a minimal usage example showing how to call it with an argument_name and
filter_by (e.g. opts = [argument_name: "metric_name", filter_by: :label_id] and
call not_publicly_disabled_label_filter(opts) — include one example for
:label_id and one brief variant for :fqn if desired) so the doc demonstrates
expected input and returned SQL string.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/sanbase/clickhouse/metric/sql_query/metric_sql_query.exlib/sanbase/metric/sql_query_helper.ex
Changes
The table
publicly_disabled_labeled_metricsallows the bigdata/metrics teams to selectively disable some labels for some metrics, if there is an issue with that computation.Coverting to draft as we would need to also support the
statusand not disable always.Sometimes it would need only to warn
Ticket
Checklist:
Summary by CodeRabbit