feat(summary view): added tree map metric details apis | 7087

[aniketio-ctrl]

Summary

Added treemap apis

Related Issues / PR's

#7087

---

Important

Add APIs and backend support for treemap metrics, including new routes, functions, and interfaces.

• APIs:
  • Add /api/v1/metrics/treemap route in api.go for treemap metrics.
  • Implement GetTreeMap in summary.go to handle treemap metric requests.
• Reader Functions:
  • Add GetMetricsCardinalityPercentage and GetMetricsDataPointsPercentage in reader.go for treemap data processing.
• Parser:
  • Add ParseTreeMapMetricsParams in parser.go for parsing treemap metric requests.
• Summary Service:
  • Implement GetMetricsTreemap in summary.go to process treemap metrics.
• Interfaces:
  • Update Reader interface in interface.go to include new treemap functions.

^{This description was created by for bab1399. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to bab1399 in 2 minutes and 50 seconds

More details

• Looked at 262 lines of code in 6 files
• Skipped 0 files when reviewing.
• Skipped posting 11 drafted comments based on config settings.

1. pkg/query-service/app/metricsexplorer/summary.go:205

• Draft comment:
  In GetMetricsTreemap, validate that the 'Treemap' field in the request is correctly set. Consider adding a default case to return a meaningful error if the type is unknown.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  The comment makes a reasonable suggestion - returning nil, nil in the default case is not very informative. However, this could be a valid design choice if the API is meant to silently ignore unknown treemap types. Without knowing the API contract or seeing the TreeMapMetricsRequest definition, we can't be certain that this is actually an issue.
  I don't have visibility into the API contract or the TreeMapMetricsRequest type definition. The current behavior could be intentional.
  While the suggestion seems reasonable, we should err on the side of assuming the current implementation is intentional without strong evidence otherwise.
  Delete the comment since we don't have enough context to be confident that returning nil for unknown types is incorrect.

2. pkg/query-service/app/metricsexplorer/summary.go:95

• Draft comment:
  The concurrent error handling in GetMetricsSummary uses an error channel with capacity 1; ensure this design is robust under high–error load to prevent goroutine blocking.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. pkg/query-service/interfaces/interface.go:130

• Draft comment:
  Ensure that the new tree map detail functions in the Reader interface (GetMetricsCardinalityPercentage and GetMetricsDataPointsPercentage) remain strictly metric–related and do not leak non–ClickHouse logic.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  The comment aligns with one of the additional rules about not adding non-ClickHouse related functions to the ClickHouseReader interface. However, the comment is phrased as "Ensure that..." which violates the rule about not asking authors to verify things. The method names and return types clearly indicate these are metric-related functions, so the concern seems speculative rather than pointing out a clear issue.
  The methods could potentially contain non-ClickHouse logic in their implementation, which we can't see from just the interface. The naming and return types suggest they are metric-related, but we can't be 100% certain.
  While the implementation details aren't visible, the interface definition itself shows these are clearly metric-focused methods returning metric-specific types (CardinalityTreemap and DataPointTreemap). The comment is more speculative than identifying a concrete issue.
  Delete the comment because it violates the rule about not asking authors to verify things ("Ensure that...") and is speculative rather than pointing out a concrete issue. The methods appear to be appropriately metric-focused based on their signatures.

4. pkg/query-service/app/metricsexplorer/summary.go:154

• Draft comment:
  The conversion of dashboard data via JSON marshalling/unmarshalling introduces unnecessary overhead. Consider optimizing this conversion if possible.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. pkg/query-service/app/metricsexplorer/parser.go:28

• Draft comment:
  ParseFilterValueSuggestions decodes the request body into the FilterValueRequest struct. Ensure that the error message is clear and consider logging the incoming body if debugging issues arise.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. pkg/query-service/app/metricsexplorer/parser.go:15

• Draft comment:
  Minor: In ParseFilterKeySuggestions, consider validating the request query parameters (e.g. checking for empty searchText) and closing r.Body if needed. This is a stylistic suggestion affecting robustness.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50%
  None

7. pkg/query-service/app/metricsexplorer/summary.go:94

• Draft comment:
  Suggestion: Instead of manually managing a WaitGroup with an error channel in GetMetricsSummary, consider using the errgroup package for more concise error propagation and cancellation.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. pkg/query-service/app/metricsexplorer/summary.go:210

• Draft comment:
  Note: In GetMetricsTreemap, the default case returns nil, nil. This may hide an unsupported treemap type. Consider returning an explicit error for invalid 'Treemap' values.
• Reason this comment was not posted:
  Marked as duplicate.

9. pkg/query-service/app/metricsexplorer/summary.go:169

• Draft comment:
  Observation: The dashboards lookup converts data via JSON marshal/unmarshal. If possible, refactor to avoid double serialization if types are already compatible.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

10. pkg/query-service/interfaces/interface.go:127

• Draft comment:
  Comment: New methods for metrics explorer (e.g. GetMetricsCardinalityPercentage, GetMetricsDataPointsPercentage, GetAllMetricFilterAttributeValues/Units/Keys) are added. Adding brief documentation comments for these interface methods would boost maintainability and clarity.
• Reason this comment was not posted:
  Marked as duplicate.

11. pkg/query-service/app/metricsexplorer/summary.go:111

• Draft comment:
  Minor: There are two concurrent goroutines updating the 'LastReceived' field. Verify that both data sources are intended to write to the same field and that race conditions (if any) are acceptable or intentionally overridden.
• Reason this comment was not posted:
  Confidence changes required: 40% <= threshold 50%
  None

Workflow ID: wflow_qXtPnJpZdrOt5NNc

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.