Feat/7077 | Related metrics api #7149
Open
+549
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 843d9e1 in 3 minutes and 1 seconds
More details
- Looked at
615lines of code in9files - Skipped
0files when reviewing. - Skipped posting
11drafted comments based on config settings.
1. pkg/query-service/app/metricsexplorer/summary.go:150
- Draft comment:
Dashboard info is obtained by marshalling then unmarshalling JSON; consider a direct transform to improve performance. - 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.
2. pkg/query-service/app/summary.go:107
- Draft comment:
Check error when reading request body before reassigning r.Body. Re-reading may mask underlying errors. - 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/utils/format.go:340
- Draft comment:
Using math.Pow with float conversion in GetEpochNanoSecs can be brittle; consider an integer-based calculation. - 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.
4. pkg/query-service/utils/format.go:156
- Draft comment:
Clean up the inline comment formatting in QuoteEscapedStringForContains; remove extra spaces in the URL reference. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%
None
5. pkg/query-service/interfaces/interface.go:135
- Draft comment:
Ensure downstream callers adhere to the DAO pattern for any non-ClickHouse functions; the GetRelatedMetrics method addition appears appropriate. - Reason this comment was not posted:
Comment was on unchanged code.
6. pkg/query-service/utils/format.go:162
- Draft comment:
Remove extra space in the comment URL in QuoteEscapedStringForContains (e.g. "https: //clickhouse...") to improve clarity. - 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.
7. pkg/query-service/utils/format.go:306
- Draft comment:
Document the rationale for truncating typeName by one character in GetClickhouseColumnName; the slicing (typeName[:len(typeName)-1]) is non-obvious. - 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/utils/format.go:262
- Draft comment:
Consider simplifying pointer dereferencing in getPointerValue using reflect.Indirect to cover all pointer cases in a generic way. - 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.
9. pkg/query-service/utils/format.go:209
- Draft comment:
Instead of using fmt.Sprint and strings.Fields for formatting numeric slices in ClickHouseFormattedValue, consider iterating and formatting each element to preserve formatting control. - 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.
10. pkg/query-service/utils/format.go:339
- Draft comment:
Clarify expected input units and scale in GetEpochNanoSecs; adding documentation about whether it expects seconds, milliseconds, etc. - 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.
11. pkg/query-service/utils/format.go:187
- Draft comment:
Consider controlling the precision when formatting float values in ClickHouseFormattedValue to avoid overly long decimals. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
Workflow ID: wflow_O7TFifSVr06gMUPl
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -621,3 +621,97 @@ func GetDashboardsWithMetricName(ctx context.Context, metricName string) ([]map[ | |||
|
|
|||
| return result, nil | |||
| } | |||
|
|
|||
| func GetDashboardsWithMetricNames(ctx context.Context, metricNames []string) (map[string][]map[string]string, *model.ApiError) { | |||
There was a problem hiding this comment.
Consider refactoring the existing GetDashboardsWithMetricName function to use this more general version.
- function
GetDashboardsWithMetricName(model.go)
| // Here we're using 0.5 for each, but adjust as needed. | ||
| finalScores := make(map[string]float64) | ||
| for metric, scores := range relatedMetricsMap { | ||
| finalScores[metric] = scores.NameSimilarity*0.5 + scores.AttributeSimilarity*0.5 |
There was a problem hiding this comment.
The weights (0.5 for name and attribute similarity) are hardcoded. Consider declaring them as constants for clarity and easier future adjustments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Related metrics api
Related Issues / PR's
Important
Introduces a new API to fetch related metrics based on name and attribute similarity, with changes across multiple files to support this functionality.
GetRelatedMetricsfunction toClickHouseReaderinreader.goto fetch related metrics based on name and attribute similarity./api/v1/metrics/relatedendpoint inhttp_handler.goto expose related metrics API.RelatedMetricsRequest,RelatedMetricsResponse, andRelatedMetricsScoretometrics_explorer/summary.go.ParseRelatedMetricsParamsinparser.goto parse request parameters for related metrics.GetRelatedMetricsinsummary.goto compute and return related metrics.interfaces/interface.goto includeGetRelatedMetricsin theReaderinterface.This description was created by
for 843d9e1. It will automatically update as commits are pushed.