-
Notifications
You must be signed in to change notification settings - Fork 338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: dashboard variables adding options #3543
Conversation
9c6a572
to
5ce6b24
Compare
27f5ea9
to
5c67054
Compare
5c67054
to
4ae753a
Compare
95b8a3b
to
213a964
Compare
Warning Rate limit exceeded@ktx-vaidehi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 27 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughRecent updates to the dashboard components introduced new percentile aggregation functions (P50, P90, P95, P99) across various Vue components and backend modules. These changes enhance the statistical analysis capabilities of the dashboard by allowing users to select these additional percentile options in dropdown menus and query builders. Additionally, the handling of asynchronous operations and error management has been improved in the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant Dashboard
participant Backend
User->>Dashboard: Select aggregation type
Dashboard->>Backend: Fetch data with selected aggregation (p50, p90, p95, p99)
Backend-->>Dashboard: Return aggregated data
Dashboard-->>User: Display aggregated data
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
47b2ce1
to
a82203a
Compare
2e22862
to
a87b59c
Compare
a87b59c
to
41e624e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- src/common/meta/dashboards/v3/mod.rs (1 hunks)
- web/src/components/dashboards/addPanel/ConfigPanel.vue (2 hunks)
- web/src/components/dashboards/addPanel/DashboardMapQueryBuilder.vue (1 hunks)
- web/src/components/dashboards/addPanel/DashboardQueryBuilder.vue (2 hunks)
- web/src/components/dashboards/addPanel/DashboardQueryEditor.vue (3 hunks)
- web/src/components/dashboards/addPanel/DashboardSankeyChartBuilder.vue (1 hunks)
- web/src/components/dashboards/addPanel/Drilldown.vue (2 hunks)
- web/src/components/dashboards/addPanel/DrilldownPopUp.vue (4 hunks)
- web/src/composables/useDashboardPanel.ts (1 hunks)
- web/src/locales/languages/en.json (1 hunks)
- web/src/views/Dashboards/addPanel/AddPanel.vue (2 hunks)
Files skipped from review due to trivial changes (2)
- web/src/components/dashboards/addPanel/DashboardMapQueryBuilder.vue
- web/src/locales/languages/en.json
Additional context used
Path-based instructions (1)
src/common/meta/dashboards/v3/mod.rs (1)
Pattern
**/*.rs
: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
Biome
web/src/composables/useDashboardPanel.ts
[error] 36-36: Unexpected any. Specify a different type.
[error] 130-130: Unexpected any. Specify a different type.
[error] 135-135: Unexpected any. Specify a different type.
[error] 164-164: Unexpected any. Specify a different type.
[error] 214-214: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 217-217: Unexpected any. Specify a different type.
[error] 245-245: Unexpected any. Specify a different type.
[error] 275-275: Unexpected any. Specify a different type.
[error] 288-288: Unexpected any. Specify a different type.
[error] 307-307: Unexpected any. Specify a different type.
[error] 307-307: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 320-324: Template literals are preferred over string concatenation.
[error] 329-329: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 333-333: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 334-334: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 344-344: Unexpected any. Specify a different type.
[error] 362-362: Unexpected any. Specify a different type.
[error] 362-362: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 375-379: Template literals are preferred over string concatenation.
[error] 384-384: Use === instead of ==.
== is only allowed when comparing againstnull
Additional comments not posted (10)
web/src/components/dashboards/addPanel/Drilldown.vue (2)
87-87
: PassingvariablesData
toDrilldownPopUp
enhances the component's functionality by allowing it to access variable data, aligning with the PR's objectives.
105-105
: Properly declaringvariablesData
in props facilitates the passing of additional data to the component, enhancing its interactivity and functionality.src/common/meta/dashboards/v3/mod.rs (1)
146-149
: Adding percentile functions (P50
,P90
,P95
,P99
) to theAggregationFunc
enum extends the analytical capabilities of the dashboard, aligning with the PR's objectives to introduce more detailed data analysis options.web/src/components/dashboards/addPanel/DrilldownPopUp.vue (1)
328-331
: AddingvariablesData
as a prop with a default value offalse
is a prudent choice, ensuring that the component can function without this data if not provided, thus maintaining backward compatibility.web/src/components/dashboards/addPanel/ConfigPanel.vue (2)
734-734
: The newvariablesData
prop is correctly passed to theDrilldown
component.
749-749
: ThevariablesData
prop is correctly declared in the component props.web/src/components/dashboards/addPanel/DashboardQueryEditor.vue (1)
355-374
: The implementation of new percentile aggregation functions (p50
,p90
,p95
,p99
) using switch statements is correctly done across different chart types.Also applies to: 496-522, 652-663
web/src/components/dashboards/addPanel/DashboardSankeyChartBuilder.vue (1)
738-753
: The addition of percentile options (p50
,p90
,p95
,p99
) to thetriggerOperators
array is correctly implemented with proper internationalization and value assignment.web/src/views/Dashboards/addPanel/AddPanel.vue (1)
258-261
: The addition ofvariablesData
as a prop toConfigPanel
aligns with the PR objectives to enhance variable data handling. The syntax is correct and follows Vue.js best practices.web/src/components/dashboards/addPanel/DashboardQueryBuilder.vue (1)
960-975
: Added new percentile options to the dropdown as per requirements.The addition of
p50
,p90
,p95
, andp99
to thetriggerOperators
array aligns with the PR objectives to introduce these options on the y-axis. This change enhances the dashboard's functionality by allowing users to select these new aggregation functions.
currentQuery.fields.filter = []; | ||
} | ||
|
||
// Add the new filter item | ||
currentQuery.fields.filter.push({ | ||
type: "list", | ||
values: [], | ||
column: name, | ||
operator: null, | ||
value: null, | ||
}); | ||
|
||
// Ensure the filterValue array is initialized | ||
if (!dashboardPanelData.meta.filterValue) { | ||
dashboardPanelData.meta.filterValue = []; | ||
} | ||
|
||
// remove any existing data | ||
const find = dashboardPanelData.meta.filterValue.findIndex( | ||
(it: any) => it.column == name | ||
); | ||
if (find >= 0) { | ||
dashboardPanelData.meta.filterValue.splice(find, 1); | ||
} | ||
try { | ||
const res = await StreamService.fieldValues({ | ||
org_identifier: store.state.selectedOrganization.identifier, | ||
stream_name: currentQuery.fields.stream, | ||
start_time: new Date( | ||
dashboardPanelData.meta.dateTime["start_time"].toISOString() | ||
).getTime(), | ||
end_time: new Date( | ||
dashboardPanelData.meta.dateTime["end_time"].toISOString() | ||
).getTime(), | ||
fields: [name], | ||
size: 10, | ||
type: currentQuery.fields.stream_type, | ||
}); | ||
|
||
StreamService.fieldValues({ | ||
org_identifier: store.state.selectedOrganization.identifier, | ||
stream_name: | ||
dashboardPanelData.data.queries[ | ||
dashboardPanelData.layout.currentQueryIndex | ||
].fields.stream, | ||
start_time: new Date( | ||
dashboardPanelData.meta.dateTime["start_time"].toISOString() | ||
).getTime(), | ||
end_time: new Date( | ||
dashboardPanelData.meta.dateTime["end_time"].toISOString() | ||
).getTime(), | ||
fields: [name], | ||
size: 10, | ||
type: dashboardPanelData.data.queries[ | ||
dashboardPanelData.layout.currentQueryIndex | ||
].fields.stream_type, | ||
}) | ||
.then((res: any) => { | ||
dashboardPanelData.meta.filterValue.push({ | ||
column: name, | ||
value: res?.data?.hits?.[0]?.values | ||
.map((it: any) => it.zo_sql_key) | ||
.filter((it: any) => it), | ||
}); | ||
}) | ||
.catch((error: any) => { | ||
const errorDetailValue = | ||
error.response?.data.error_detail || | ||
error.response?.data.message || | ||
"Something went wrong!"; | ||
const trimmedErrorMessage = | ||
errorDetailValue.length > 300 | ||
? errorDetailValue.slice(0, 300) + " ..." | ||
: errorDetailValue; | ||
$q.notify({ | ||
type: "negative", | ||
message: trimmedErrorMessage, | ||
timeout: 5000, | ||
}); | ||
dashboardPanelData.meta.filterValue.push({ | ||
column: name, | ||
value: res?.data?.hits?.[0]?.values | ||
.map((it: any) => it.zo_sql_key) | ||
.filter((it: any) => it), | ||
}); | ||
} catch (error: any) { | ||
const errorDetailValue = | ||
error.response?.data.error_detail || | ||
error.response?.data.message || | ||
"Something went wrong!"; | ||
const trimmedErrorMessage = | ||
errorDetailValue.length > 300 | ||
? errorDetailValue.slice(0, 300) + " ..." | ||
: errorDetailValue; | ||
$q.notify({ | ||
type: "negative", | ||
message: trimmedErrorMessage, | ||
timeout: 5000, | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor addFilteredItem
to improve error handling and maintainability.
- The function now correctly initializes arrays and handles asynchronous operations with error handling. However, the error message handling can be improved by using template literals for better readability and maintainability.
- const trimmedErrorMessage =
- errorDetailValue.length > 300
- ? errorDetailValue.slice(0, 300) + " ..."
- : errorDetailValue;
+ const trimmedErrorMessage = `${errorDetailValue.slice(0, 300)}${errorDetailValue.length > 300 ? '...' : ''}`;
- Consider adding more specific error logging or handling mechanisms to better diagnose issues.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const addFilteredItem = async (name: string) => { | |
const currentQuery = | |
dashboardPanelData.data.queries[ | |
dashboardPanelData.layout.currentQueryIndex | |
].fields.filter = []; | |
} | |
]; | |
if ( | |
!dashboardPanelData.data.queries[ | |
dashboardPanelData.layout.currentQueryIndex | |
].fields.filter.find((it: any) => it.column == name) | |
) { | |
dashboardPanelData.data.queries[ | |
dashboardPanelData.layout.currentQueryIndex | |
].fields.filter.push({ | |
type: "list", | |
values: [], | |
column: name, | |
operator: null, | |
value: null, | |
}); | |
// Ensure the filter array is initialized | |
if (!currentQuery.fields.filter) { | |
currentQuery.fields.filter = []; | |
} | |
// Add the new filter item | |
currentQuery.fields.filter.push({ | |
type: "list", | |
values: [], | |
column: name, | |
operator: null, | |
value: null, | |
}); | |
// Ensure the filterValue array is initialized | |
if (!dashboardPanelData.meta.filterValue) { | |
dashboardPanelData.meta.filterValue = []; | |
} | |
// remove any existing data | |
const find = dashboardPanelData.meta.filterValue.findIndex( | |
(it: any) => it.column == name | |
); | |
if (find >= 0) { | |
dashboardPanelData.meta.filterValue.splice(find, 1); | |
} | |
try { | |
const res = await StreamService.fieldValues({ | |
org_identifier: store.state.selectedOrganization.identifier, | |
stream_name: currentQuery.fields.stream, | |
start_time: new Date( | |
dashboardPanelData.meta.dateTime["start_time"].toISOString() | |
).getTime(), | |
end_time: new Date( | |
dashboardPanelData.meta.dateTime["end_time"].toISOString() | |
).getTime(), | |
fields: [name], | |
size: 10, | |
type: currentQuery.fields.stream_type, | |
}); | |
StreamService.fieldValues({ | |
org_identifier: store.state.selectedOrganization.identifier, | |
stream_name: | |
dashboardPanelData.data.queries[ | |
dashboardPanelData.layout.currentQueryIndex | |
].fields.stream, | |
start_time: new Date( | |
dashboardPanelData.meta.dateTime["start_time"].toISOString() | |
).getTime(), | |
end_time: new Date( | |
dashboardPanelData.meta.dateTime["end_time"].toISOString() | |
).getTime(), | |
fields: [name], | |
size: 10, | |
type: dashboardPanelData.data.queries[ | |
dashboardPanelData.layout.currentQueryIndex | |
].fields.stream_type, | |
}) | |
.then((res: any) => { | |
dashboardPanelData.meta.filterValue.push({ | |
column: name, | |
value: res?.data?.hits?.[0]?.values | |
.map((it: any) => it.zo_sql_key) | |
.filter((it: any) => it), | |
}); | |
}) | |
.catch((error: any) => { | |
const errorDetailValue = | |
error.response?.data.error_detail || | |
error.response?.data.message || | |
"Something went wrong!"; | |
const trimmedErrorMessage = | |
errorDetailValue.length > 300 | |
? errorDetailValue.slice(0, 300) + " ..." | |
: errorDetailValue; | |
$q.notify({ | |
type: "negative", | |
message: trimmedErrorMessage, | |
timeout: 5000, | |
}); | |
dashboardPanelData.meta.filterValue.push({ | |
column: name, | |
value: res?.data?.hits?.[0]?.values | |
.map((it: any) => it.zo_sql_key) | |
.filter((it: any) => it), | |
}); | |
} catch (error: any) { | |
const errorDetailValue = | |
error.response?.data.error_detail || | |
error.response?.data.message || | |
"Something went wrong!"; | |
const trimmedErrorMessage = | |
errorDetailValue.length > 300 | |
? errorDetailValue.slice(0, 300) + " ..." | |
: errorDetailValue; | |
$q.notify({ | |
type: "negative", | |
message: trimmedErrorMessage, | |
timeout: 5000, | |
}); | |
} | |
const addFilteredItem = async (name: string) => { | |
const currentQuery = | |
dashboardPanelData.data.queries[ | |
dashboardPanelData.layout.currentQueryIndex | |
]; | |
// Ensure the filter array is initialized | |
if (!currentQuery.fields.filter) { | |
currentQuery.fields.filter = []; | |
} | |
// Add the new filter item | |
currentQuery.fields.filter.push({ | |
type: "list", | |
values: [], | |
column: name, | |
operator: null, | |
value: null, | |
}); | |
// Ensure the filterValue array is initialized | |
if (!dashboardPanelData.meta.filterValue) { | |
dashboardPanelData.meta.filterValue = []; | |
} | |
try { | |
const res = await StreamService.fieldValues({ | |
org_identifier: store.state.selectedOrganization.identifier, | |
stream_name: currentQuery.fields.stream, | |
start_time: new Date( | |
dashboardPanelData.meta.dateTime["start_time"].toISOString() | |
).getTime(), | |
end_time: new Date( | |
dashboardPanelData.meta.dateTime["end_time"].toISOString() | |
).getTime(), | |
fields: [name], | |
size: 10, | |
type: currentQuery.fields.stream_type, | |
}); | |
dashboardPanelData.meta.filterValue.push({ | |
column: name, | |
value: res?.data?.hits?.[0]?.values | |
.map((it: any) => it.zo_sql_key) | |
.filter((it: any) => it), | |
}); | |
} catch (error: any) { | |
const errorDetailValue = | |
error.response?.data.error_detail || | |
error.response?.data.message || | |
"Something went wrong!"; | |
const trimmedErrorMessage = `${errorDetailValue.slice(0, 300)}${errorDetailValue.length > 300 ? '...' : ''}`; | |
$q.notify({ | |
type: "negative", | |
message: trimmedErrorMessage, | |
timeout: 5000, | |
}); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/src/components/dashboards/addPanel/DashboardQueryBuilder.vue (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- web/src/components/dashboards/addPanel/DashboardQueryBuilder.vue
9baa245
to
b833685
Compare
b833685
to
97dcf39
Compare
- Multiple filters for the same field. - Added p99, p95, p90 and p50 as an option on y-axis - In Drilldown popup will now get variables value of current dashboard <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added new percentile aggregation options (`p50`, `p90`, `p95`, `p99`) across various dashboard components. - Introduced `variablesData` prop to enhance data handling in several components. - **Improvements** - Enhanced error handling and asynchronous operations in dashboard panel functionalities. - Improved readability and maintainability of aggregation functions in query components. - **Localization** - Added new keys for percentiles (`p50`, `p90`, `p95`, `p99`) in the English language file. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
New Features
p50
,p90
,p95
,p99
) across various dashboard components.variablesData
prop to enhance data handling in several components.Improvements
Localization
p50
,p90
,p95
,p99
) in the English language file.