feat: update query builder to support spatial aggregations and functions #4569
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
WalkthroughThe updates across the frontend and backend systems introduce a new API version (v4) and enrich the query builder with additional metrics operators, functions, and keyboard shortcuts. These changes aim to provide more flexibility in querying metrics, improve user interaction with the platform, and enhance data aggregation capabilities. Changes
Related issues
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 8
Configuration used: CodeRabbit UI
Files selected for processing (25)
- frontend/src/api/apiV1.ts (1 hunks)
- frontend/src/api/index.ts (2 hunks)
- frontend/src/api/metrics/getQueryRange.ts (2 hunks)
- frontend/src/constants/queryBuilder.ts (4 hunks)
- frontend/src/constants/queryBuilderOperators.ts (1 hunks)
- frontend/src/constants/queryFunctionOptions.ts (1 hunks)
- frontend/src/container/ListOfDashboard/DashboardsList.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.tsx (7 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx (3 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts (1 hunks)
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts (1 hunks)
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts (3 hunks)
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts (7 hunks)
- frontend/src/lib/dashboard/getQueryResults.ts (2 hunks)
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts (1 hunks)
- frontend/src/types/api/dashboard/create.ts (1 hunks)
- frontend/src/types/api/dashboard/getAll.ts (1 hunks)
- frontend/src/types/api/queryBuilder/queryBuilderData.ts (1 hunks)
- frontend/src/types/common/operations.types.ts (2 hunks)
- frontend/src/types/common/queryBuilder.ts (2 hunks)
- frontend/tests/dashboards/utils.ts (3 hunks)
Files skipped from review due to trivial changes (2)
- frontend/src/api/apiV1.ts
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts
Additional comments: 23
frontend/src/types/api/dashboard/create.ts (1)
- 7-7: The addition of an optional
versionfield to thePropstype is a good approach to support API versioning. This change allows for flexibility in specifying the version of the dashboard being created, which aligns with the PR's objective to introduce enhanced query capabilities through versioning. Ensure that the consuming functions and components that utilizePropsare updated accordingly to handle this new field.frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss (1)
- 1-11: The added styling for
.selectOptionContainerinQueryBuilderSearch.styles.scssenhances the visual presentation of the select options within the QueryBuilderSearch component. The use of flex display, gap, and alignment properties improves the layout, while the scrollbar customization ensures a better user experience in cases of overflow. Ensure these styles are scoped appropriately to avoid unintended effects on other components and that they adhere to the application's design guidelines.frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss (1)
- 1-34: The styling added for
.add-function-btnand.query-functions-listinQueryFunctions.styles.scssis well-structured, enhancing the UI for managing query functions. The use of flex display, gap, and custom margins improves the layout and usability. Additionally, the styling for inputs and the visual connection between query function elements is thoughtfully designed. As with other style changes, ensure these styles are scoped correctly to prevent any unintended effects on other parts of the application.frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx (2)
- 1-2: The refactoring to import
QueryBuilderSearch.styles.scssinOptionRenderer.tsxis appropriate for applying the new styles to theSelectOptionContainercomponent. This change ensures that the component benefits from the styling enhancements made for select options within the QueryBuilderSearch context.- 13-19: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [16-28]
The structural adjustments made to the
SelectOptionContainercomponent, including the addition ofoption-valueandoption-meta-data-containerdivs, enhance the presentation of option labels and metadata. This change improves the readability and user experience by clearly distinguishing between the value and additional information such as type and data type. Ensure that these changes are tested across different browsers to maintain consistency in appearance and functionality.frontend/src/hooks/queryBuilder/useGetQueryRange.ts (1)
- 15-24: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [18-36]
The integration of
useDashboardto fetchselectedDashboarddata and determine theversionfor the query in theuseGetQueryRangehook is a smart approach to dynamically select the API version based on the dashboard context. This ensures that queries are executed against the correct API version, aligning with the dashboard's configuration. However, ensure that there's a fallback mechanism or default version defined (as done here with'v4') to handle cases where the dashboard data might not specify a version. This approach enhances the flexibility and robustness of the query execution process.frontend/src/api/index.ts (1)
- 126-134: The addition of a new API instance
ApiV4Instancewith axios V4 configuration is a crucial step in supporting the new version of the API. This change allows for the separation of concerns between different API versions, enabling more granular control over API requests and responses. Ensure that the base URL and any necessary configurations specific to the V4 API are correctly set up to avoid any issues with API communication.frontend/tests/dashboards/utils.ts (1)
- 173-174: The addition of the comment
// eslint-disable-next-line sonarjs/no-identical-functionsin thegetTimeSeriesQueryDatafunction is a minor change aimed at addressing a specific linting rule. While addressing linting issues is important, ensure that this comment is necessary and that there isn't a more appropriate way to address the underlying issue (e.g., refactoring to remove duplicate code if applicable).The addition of the linting comment is acceptable given the context. However, consider reviewing the necessity of duplicate code that triggered this linting rule.
frontend/src/types/common/queryBuilder.ts (1)
- 147-163: The addition of the
QueryFunctionsTypesenum introduces various query function types such asCUTOFF_MIN,CUTOFF_MAX, etc. This is a positive enhancement for type safety and code readability. Consider the following points:
- Correctness and Logic: The enum correctly defines various query function types, which will likely be used throughout the application to ensure consistency in function naming and usage.
- Best Practices: The enum is well-defined and follows naming conventions. Ensure that all uses of these function types in the application are updated to leverage this enum for improved type safety and maintainability.
The addition of the
QueryFunctionsTypesenum is a good practice for maintaining consistency and type safety in the application. Ensure its usage is consistently applied across the application.frontend/src/constants/queryBuilderOperators.ts (1)
- 306-422: The addition of various sets of options for different metric aggregate operators (e.g.,
metricsSumAggregateOperatorOptions,metricsGuageAggregateOperatorOptions, etc.) enhances the application's flexibility in handling different types of aggregations. Consider the following points:
- Correctness and Logic: The options are correctly defined and categorized by different types such as
Space,Gauge, andHistogram. This categorization aligns with the application's requirements for handling different aggregation scenarios.- Consistency: Ensure that these new sets of options are consistently used throughout the application wherever metric aggregate operators are needed. This includes updating any existing code that manually defines these options to use the new constants instead.
- Best Practices: The structure and naming conventions of the options follow best practices. Consider adding documentation/comments to describe each category's use case and applicability to aid in maintainability and understanding for future developers.
The addition of categorized sets of options for metric aggregate operators is a valuable enhancement. Ensure consistent usage throughout the application and consider adding documentation for clarity.
frontend/src/constants/queryBuilder.ts (2)
- 168-170: The addition of
timeAggregation,spaceAggregation, andfunctionsfields to theinitialQueryBuilderFormValuesobject is a significant enhancement. It aligns with the PR's objective to support spatial aggregations and functions. However, ensure that the rest of the application correctly handles these new fields, especially in areas whereIBuilderQueryobjects are consumed.Ensure that the application logic that consumes
IBuilderQueryobjects has been updated to handle the newtimeAggregation,spaceAggregation, andfunctionsfields appropriately.
- 291-296: The introduction of
ATTRIBUTE_TYPESenum is a good practice for maintaining a clear and manageable set of attribute types. This approach enhances code readability and maintainability by providing a centralized definition of attribute types used throughout the application.frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts (3)
- 10-14: The import of new constants for metrics space aggregate operator options is correctly done. This ensures that the hook can utilize these options for space aggregations, aligning with the PR's objectives to enhance spatial data processing capabilities.
- 125-135: The addition of
handleSpaceAggregationChangefunction is a good practice. It encapsulates the logic for handling changes to space aggregation in a dedicated function, improving code readability and maintainability. Ensure that this function is properly integrated and called in the relevant UI components where space aggregation options are presented to the user.Verify that the UI components which allow users to select space aggregation options correctly call
handleSpaceAggregationChangeupon changes.
- 263-276: The
handleQueryFunctionsUpdatesfunction is a valuable addition for handling updates to query functions. This aligns with the PR's objective to support advanced query functions. Ensure that this function is properly integrated and called in the relevant UI components where users can define or modify query functions.Verify that the UI components which allow users to define or modify query functions correctly call
handleQueryFunctionsUpdatesupon changes.frontend/src/container/ListOfDashboard/DashboardsList.tsx (1)
- 214-214: Adding the
version: 'v4'property to the dashboard creation request aligns with the PR's objective to introduce versioning in API endpoints. This change ensures that the newly created dashboards are compatible with the enhanced query capabilities introduced in this PR. Ensure that the backend API correctly handles this version parameter.Verify that the backend API has been updated to handle the
versionparameter correctly and that it supports the functionalities introduced in version 4.frontend/src/container/QueryBuilder/components/Query/Query.tsx (7)
- 5-5: The import of
ATTRIBUTE_TYPESfromconstants/queryBuilderis correctly added to support the new spatial aggregation features. This aligns with the PR's objective to enhance query capabilities.- 40-41: The addition of
QueryFunctionsandSpaceAggregationOptionscomponents is a significant enhancement, enabling users to leverage advanced query functions and spatial aggregation options. This change directly supports the PR's goal of improving data analysis and visualization capabilities.- 68-70: The introduction of
handleSpaceAggregationChangeandhandleQueryFunctionsUpdatesfunctions is crucial for managing the state and behavior of the new spatial aggregation and query functions features. It's important to ensure these handlers are thoroughly tested, especially for edge cases and error handling.- 288-290: The logic to disable the operator selector based on the presence of an
aggregateAttribute.keyis a thoughtful addition. It ensures that operators are only selectable when relevant, improving the user experience by preventing invalid configurations.- 331-340: The conditional rendering of the
OperatorsSelectcomponent based on the availability of operators and the newdisableOperatorSelectorlogic is well-implemented. This change enhances the UI's responsiveness to the query's context, aligning with the PR's objectives to improve the query builder's flexibility and user experience.- 388-401: The conditional rendering between
FilterLabelandSpaceAggregationOptionsbased onisMetricsDataSourceis a key update. It dynamically adjusts the UI to accommodate different data sources and aggregation types. However, ensure that theaggregatorAttributeTypeprop correctly handles all possibleATTRIBUTE_TYPESvalues, especially in edge cases where the type might not be directly mapped or is undefined.Verification successful
The
SpaceAggregationOptionscomponent is designed to handle specificATTRIBUTE_TYPESvalues (HISTOGRAM,SUM, andGAUGE) with explicit logic for each. For any other types or ifaggregatorAttributeTypeisnull, it defaults to handling forGAUGEtypes. This implementation suggests that the component has considered the handling of all expectedATTRIBUTE_TYPESvalues, with a default behavior for any unspecified types. The concern in the original review comment about ensuring correct handling of all possibleATTRIBUTE_TYPESvalues, especially in edge cases, appears to be addressed by the component's design and implementation.* 418-424: The integration of the `QueryFunctions` component for metrics data sources is a significant enhancement, allowing users to apply various functions to their queries. This addition is crucial for supporting advanced data analysis scenarios. Ensure that the component's `onChange` handler properly updates the query state and that the UI reflects these changes accurately.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Suggest verifying the handling of all ATTRIBUTE_TYPES values in SpaceAggregationOptions component. echo "Please verify that the SpaceAggregationOptions component correctly handles all possible ATTRIBUTE_TYPES values, especially for edge cases."Length of output: 286
Script:
#!/bin/bash # Search for the SpaceAggregationOptions component definition in the codebase rg "SpaceAggregationOptions" --files-with-matchesLength of output: 272
Script:
#!/bin/bash # Display the contents of SpaceAggregationOptions.tsx for review cat "frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx"Length of output: 1354
frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts
Outdated
Show resolved
Hide resolved
frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
frontend/public/locales/en-GB/dashboard.jsonis excluded by:!**/*.jsonfrontend/public/locales/en/dashboard.jsonis excluded by:!**/*.json
Files selected for processing (31)
- frontend/src/constants/queryBuilder.ts (4 hunks)
- frontend/src/constants/queryBuilderOperators.ts (1 hunks)
- frontend/src/constants/queryFunctionOptions.ts (1 hunks)
- frontend/src/constants/shortcuts/DashboardShortcuts.ts (1 hunks)
- frontend/src/constants/shortcuts/QBShortcuts.ts (1 hunks)
- frontend/src/container/FormAlertRules/QuerySection.tsx (2 hunks)
- frontend/src/container/GridCardLayout/GridCardLayout.styles.scss (1 hunks)
- frontend/src/container/GridCardLayout/GridCardLayout.tsx (2 hunks)
- frontend/src/container/GridCardLayout/styles.ts (1 hunks)
- frontend/src/container/ListOfDashboard/DashboardsList.tsx (2 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx (2 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/menuItems.tsx (1 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/styles.ts (1 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/Description.styles.scss (1 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/SettingsDrawer.tsx (2 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/index.tsx (5 hunks)
- frontend/src/container/NewWidget/LeftContainer/QuerySection/index.tsx (4 hunks)
- frontend/src/container/NewWidget/index.tsx (5 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.styles.scss (3 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.tsx (7 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx (2 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts (1 hunks)
- frontend/src/hooks/hotkeys/useKeyboardHotkeys.tsx (1 hunks)
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts (7 hunks)
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts (1 hunks)
- frontend/src/types/api/queryBuilder/queryBuilderData.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- frontend/src/container/GridCardLayout/styles.ts
Files skipped from review as they are similar to previous changes (13)
- frontend/src/constants/queryBuilder.ts
- frontend/src/constants/queryBuilderOperators.ts
- frontend/src/constants/queryFunctionOptions.ts
- frontend/src/container/ListOfDashboard/DashboardsList.tsx
- frontend/src/container/QueryBuilder/components/Query/Query.tsx
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts
- frontend/src/types/api/queryBuilder/queryBuilderData.ts
Additional comments: 13
frontend/src/container/GridCardLayout/GridCardLayout.styles.scss (1)
- 3-3: The change from a specific
margin-topto a shorthandmarginproperty affects all four margins of the.fullscreen-grid-container. Ensure this adjustment aligns with the intended layout design, as it could impact the spacing and alignment within the UI.frontend/src/constants/shortcuts/QBShortcuts.ts (1)
- 5-17: The implementation of keyboard shortcuts for the query builder, with consideration for different operating systems, is a good enhancement. Ensure to verify that these shortcuts do not conflict with any browser or system-level shortcuts that could interfere with the intended functionality.
frontend/src/constants/shortcuts/DashboardShortcuts.ts (1)
- 5-17: The addition of keyboard shortcuts for dashboard actions, with operating system consideration, enhances user experience. As with any keyboard shortcuts, verify that these do not conflict with browser or system-level shortcuts.
frontend/src/container/NewDashboard/ComponentsSlider/menuItems.tsx (1)
- 5-29: The menu items for the components slider, including icons and display names, are well-defined. Verify the visual consistency and accessibility of the icons to ensure they are easily recognizable and align with the overall UI design.
frontend/src/container/NewDashboard/ComponentsSlider/styles.ts (1)
- 12-34: The style enhancements to the
Cardcomponent, including adjustments tomin-height, the addition of atransitionproperty, and modifications to hover states, aim to improve visual appeal and interactivity. Verify the smoothness of the animations and the responsiveness of the hover states to ensure a positive user experience.frontend/src/container/FormAlertRules/QuerySection.tsx (2)
- 6-6: The import of
QBShortcutsfrom'constants/shortcuts/QBShortcuts'is correctly added to support keyboard shortcuts functionality.- 8-8: The import of
useKeyboardHotkeysfrom'hooks/hotkeys/useKeyboardHotkeys'is correctly added to enable the use of keyboard shortcuts within this component.frontend/src/container/GridCardLayout/GridCardLayout.tsx (1)
- 148-156: The addition of the
Tooltipcomponent from 'antd' around theButtoncomponent with the title "Open in Full Screen" is a good UX improvement. It provides users with a clear indication of the button's functionality, enhancing the overall user experience.frontend/src/container/NewWidget/LeftContainer/QuerySection/index.tsx (2)
- 6-6: The import of
QBShortcutsfrom'constants/shortcuts/QBShortcuts'is correctly added to support keyboard shortcuts functionality within this component.- 10-10: The import of
useKeyboardHotkeysfrom'hooks/hotkeys/useKeyboardHotkeys'is correctly added to enable the use of keyboard shortcuts within this component.frontend/src/container/NewWidget/index.tsx (3)
- 8-8: The import of
DashboardShortcutsfrom'constants/shortcuts/DashboardShortcuts'is correctly added to support keyboard shortcuts functionality within this component.- 10-10: The import of
useKeyboardHotkeysfrom'hooks/hotkeys/useKeyboardHotkeys'is correctly added to enable the use of keyboard shortcuts within this component.- 413-413: The update of the text key for translation from
'dashboar_ok_confirm'to'dashboard_ok_confirm'corrects a typo and ensures consistency in the translation keys used across the application.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/container/OnboardingContainer/Steps/LogsConnectionStatus/LogsConnectionStatus.tsx (1 hunks)
Additional comments: 1
frontend/src/container/OnboardingContainer/Steps/LogsConnectionStatus/LogsConnectionStatus.tsx (1)
- 75-77: The addition of
timeAggregation,spaceAggregation, andfunctionsfields with default empty values (''for strings and[]for arrays) in therequestDataobject is noted. However, it's important to ensure that these fields are appropriately handled downstream, especially since they are initialized with placeholder values. If these fields are expected to be populated or modified elsewhere in the code, consider adding validation or default handling logic to prevent potential issues with query execution or data fetching.
- For
timeAggregationandspaceAggregation, if there are specific default values that make sense for the application's logic or if these fields are mandatory for certain queries, it might be beneficial to set more meaningful defaults or add checks to ensure they are populated before executing a query.- For
functions, since it's initialized as an empty array, ensure that the consuming logic is capable of handling an empty array without causing errors or unintended behavior, especially in the context of building or executing queries.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 8
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
frontend/tests/fixtures/api/dashboard/dashboardGetCallWithTimeSeriesWidget200.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/dashboard/putDashboardWithTimeSeries200.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/traces/traceExplorerViews200.jsonis excluded by:!**/*.jsongo.sumis excluded by:!**/*.sum
Files selected for processing (10)
- frontend/src/constants/queryBuilder.ts (5 hunks)
- frontend/src/container/MetricsApplication/MetricsPageQueries/MetricsPageQueriesFactory.ts (2 hunks)
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts (1 hunks)
- frontend/tests/dashboards/utils.ts (3 hunks)
- pkg/query-service/app/clickhouseReader/reader.go (5 hunks)
- pkg/query-service/app/limit.go (1 hunks)
- pkg/query-service/app/metrics/v4/query_builder.go (2 hunks)
- pkg/query-service/app/parser.go (2 hunks)
- pkg/query-service/common/metrics.go (2 hunks)
- pkg/query-service/model/v3/v3.go (5 hunks)
Files skipped from review as they are similar to previous changes (3)
- frontend/src/constants/queryBuilder.ts
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts
- frontend/tests/dashboards/utils.ts
Additional comments: 10
pkg/query-service/common/metrics.go (1)
- 22-24: The implementation of
PastDayRoundOffcorrectly calculates the start of the current day in UTC time. However, if your application operates in specific time zones or requires time zone awareness, consider adjusting this function to account for the desired time zone.frontend/src/container/MetricsApplication/MetricsPageQueries/MetricsPageQueriesFactory.ts (2)
- 51-51: The change to use average aggregation (
reduceTo: 'avg') ingetQueryBuilderQueriesis noted. Ensure this change aligns with the intended data representation and verify its impact on the UI and the correctness of the displayed metrics.- 89-89: Similarly, the update to average aggregation (
reduceTo: 'avg') ingetQueryBuilderQuerieswithFormulashould be verified for its impact on the UI and the correctness of the displayed metrics, ensuring it aligns with the intended data representation.pkg/query-service/app/metrics/v4/query_builder.go (2)
- 31-32: The exclusion of
MetricTypeExponentialHistogramfrom certain quantile calculations appears to be a deliberate refinement. Ensure this aligns with the intended logic for handling different types of metric aggregations and does not inadvertently exclude necessary calculations for exponential histograms.- 86-86: The conditional logic for handling fixed-bucket histogram quantiles with UDF, excluding
MetricTypeExponentialHistogram, should be verified to ensure it correctly implements the intended behavior for quantile calculations across different metric types.pkg/query-service/model/v3/v3.go (2)
- 512-516: Renaming the
SpaceAggregationconstants for percentiles to a more concise format (pXX) improves readability and consistency with common percentile notation. This change should be reflected in all parts of the codebase where these constants are used to ensure compatibility.- 695-695: The validation logic that requires a non-
noopor non-rateAggregateOperatorwhenGroupByis used withDataSourceMetricsandSpaceAggregationis unspecified is a good practice. It ensures that meaningful aggregations are performed when grouping metrics data. This validation enhances the query's logical consistency and prevents potential user errors.pkg/query-service/app/clickhouseReader/reader.go (3)
- 4041-4045: In the method
GetMetricAttributeValues, the use ofsql.NullFloat64andsql.NullInt64for handling potential null values from the database is noted. This is a good practice for robustly handling nullable columns. However, ensure that the logic for checking.Validbefore appending to the result list is consistently applied across all types and that there's proper error handling for any unexpected null values that might impact the application logic.- 4006-4010: The method
GetMetricAutocompleteTagKeyperforms a query to fetch distinct tag keys matching a pattern. It's important to ensure that the pattern matching viaILIKEis efficient and does not lead to performance bottlenecks, especially with a potentially large number of tags. Consider verifying if the underlying database schema and indexes support efficient pattern matching queries. If performance issues are observed, consider alternative approaches such as caching common queries or adjusting the database schema to better support these operations.- 3970-3991: The handling of the
temporalityfield inGetMetricAggregateAttributesto determine the type of a metric (e.g., converting non-monotonic cumulative sums to gauges) is a critical piece of logic that directly affects how metrics are categorized and interpreted. Ensure that this logic is thoroughly tested with various metric types and temporality values to prevent any misclassification. Additionally, consider adding more detailed comments explaining the reasoning behind these conversions for future maintainers.
|
frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
Outdated
Show resolved
Hide resolved
frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
Outdated
Show resolved
Hide resolved
srikanthccv
had a problem deploying
to
testing
GitHub Actions
Failure
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- frontend/src/api/metrics/getQueryRange.ts (2 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx (3 hunks)
- frontend/src/container/QueryBuilder/components/Formula/Formula.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.tsx (8 hunks)
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx (1 hunks)
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts (7 hunks)
- frontend/src/lib/dashboard/getQueryResults.ts (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- frontend/src/api/metrics/getQueryRange.ts
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx
- frontend/src/container/QueryBuilder/components/Query/Query.tsx
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts
- frontend/src/lib/dashboard/getQueryResults.ts
Additional comments: 1
frontend/src/container/QueryBuilder/components/Formula/Formula.tsx (1)
- 149-149: The addition of
showFunctions={false}to theQBEntityOptionscomponent effectively hides function options within the formula entity. This change aligns with the PR's objectives to enhance the query builder's capabilities while maintaining a clean and user-friendly UI. Ensure that this behavior is consistent across all relevant components and that it aligns with user expectations for the new and existing API versions. It might be beneficial to verify if this change has been communicated clearly in the UI, especially for users transitioning between different versions of the query builder.
|
If the version is not present default version to v3 ✅ |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/lib/uPlotLib/plugins/tooltipPlugin.ts (4 hunks)
Additional comments: 2
frontend/src/lib/uPlotLib/plugins/tooltipPlugin.ts (2)
- 21-21: Adding
queryName: stringto theUplotTooltipDataPropsinterface is a good enhancement for providing more context in tooltips. This change aligns with the PR's objective to enhance data analysis capabilities.- 39-39: The introduction of
duplicatedLegendLabelsas aRecord<string, true>is a smart way to track duplicated labels. This approach is efficient for checking the existence of labels without maintaining a list or array of seen labels.
YounixM
force-pushed
the
metrics-qb-change
branch
from
e5edc71
to
656a93d
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (6)
frontend/public/locales/en-GB/dashboard.jsonis excluded by:!**/*.jsonfrontend/public/locales/en/dashboard.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/dashboard/dashboardGetCallWithTimeSeriesWidget200.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/dashboard/putDashboardWithTimeSeries200.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/traces/traceExplorerViews200.jsonis excluded by:!**/*.jsongo.sumis excluded by:!**/*.sum
Files selected for processing (54)
- frontend/src/api/apiV1.ts (1 hunks)
- frontend/src/api/index.ts (2 hunks)
- frontend/src/api/metrics/getQueryRange.ts (2 hunks)
- frontend/src/constants/queryBuilder.ts (5 hunks)
- frontend/src/constants/queryBuilderOperators.ts (1 hunks)
- frontend/src/constants/queryFunctionOptions.ts (1 hunks)
- frontend/src/constants/shortcuts/DashboardShortcuts.ts (1 hunks)
- frontend/src/constants/shortcuts/QBShortcuts.ts (1 hunks)
- frontend/src/container/FormAlertRules/QuerySection.tsx (2 hunks)
- frontend/src/container/GridCardLayout/GridCardLayout.styles.scss (1 hunks)
- frontend/src/container/GridCardLayout/GridCardLayout.tsx (2 hunks)
- frontend/src/container/GridCardLayout/styles.ts (1 hunks)
- frontend/src/container/ListOfDashboard/DashboardsList.tsx (2 hunks)
- frontend/src/container/MetricsApplication/MetricsPageQueries/MetricsPageQueriesFactory.ts (2 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx (4 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/menuItems.tsx (1 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/styles.ts (1 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/Description.styles.scss (1 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/SettingsDrawer.tsx (2 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/index.tsx (5 hunks)
- frontend/src/container/NewWidget/LeftContainer/QuerySection/index.tsx (4 hunks)
- frontend/src/container/NewWidget/index.tsx (5 hunks)
- frontend/src/container/OnboardingContainer/Steps/LogsConnectionStatus/LogsConnectionStatus.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Formula/Formula.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.styles.scss (3 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.tsx (7 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/filters/AggregatorFilter/AggregatorFilter.tsx (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx (2 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts (1 hunks)
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts (1 hunks)
- frontend/src/hooks/hotkeys/useKeyboardHotkeys.tsx (1 hunks)
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts (3 hunks)
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts (7 hunks)
- frontend/src/lib/dashboard/getQueryResults.ts (2 hunks)
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts (1 hunks)
- frontend/src/lib/uPlotLib/plugins/tooltipPlugin.ts (4 hunks)
- frontend/src/types/api/dashboard/create.ts (1 hunks)
- frontend/src/types/api/dashboard/getAll.ts (1 hunks)
- frontend/src/types/api/queryBuilder/queryBuilderData.ts (1 hunks)
- frontend/src/types/common/operations.types.ts (2 hunks)
- frontend/src/types/common/queryBuilder.ts (2 hunks)
- frontend/tests/dashboards/utils.ts (3 hunks)
- pkg/query-service/app/clickhouseReader/reader.go (5 hunks)
- pkg/query-service/app/limit.go (1 hunks)
- pkg/query-service/app/metrics/v4/query_builder.go (2 hunks)
- pkg/query-service/app/parser.go (2 hunks)
- pkg/query-service/common/metrics.go (2 hunks)
- pkg/query-service/model/v3/v3.go (5 hunks)
Files skipped from review as they are similar to previous changes (53)
- frontend/src/api/apiV1.ts
- frontend/src/api/index.ts
- frontend/src/api/metrics/getQueryRange.ts
- frontend/src/constants/queryBuilder.ts
- frontend/src/constants/queryBuilderOperators.ts
- frontend/src/constants/queryFunctionOptions.ts
- frontend/src/constants/shortcuts/DashboardShortcuts.ts
- frontend/src/constants/shortcuts/QBShortcuts.ts
- frontend/src/container/FormAlertRules/QuerySection.tsx
- frontend/src/container/GridCardLayout/GridCardLayout.styles.scss
- frontend/src/container/GridCardLayout/GridCardLayout.tsx
- frontend/src/container/GridCardLayout/styles.ts
- frontend/src/container/ListOfDashboard/DashboardsList.tsx
- frontend/src/container/MetricsApplication/MetricsPageQueries/MetricsPageQueriesFactory.ts
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx
- frontend/src/container/NewDashboard/ComponentsSlider/menuItems.tsx
- frontend/src/container/NewDashboard/ComponentsSlider/styles.ts
- frontend/src/container/NewDashboard/DashboardDescription/Description.styles.scss
- frontend/src/container/NewDashboard/DashboardDescription/SettingsDrawer.tsx
- frontend/src/container/NewDashboard/DashboardDescription/index.tsx
- frontend/src/container/NewWidget/LeftContainer/QuerySection/index.tsx
- frontend/src/container/NewWidget/index.tsx
- frontend/src/container/OnboardingContainer/Steps/LogsConnectionStatus/LogsConnectionStatus.tsx
- frontend/src/container/QueryBuilder/components/Formula/Formula.tsx
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.styles.scss
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx
- frontend/src/container/QueryBuilder/components/Query/Query.tsx
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts
- frontend/src/hooks/hotkeys/useKeyboardHotkeys.tsx
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts
- frontend/src/lib/dashboard/getQueryResults.ts
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts
- frontend/src/lib/uPlotLib/plugins/tooltipPlugin.ts
- frontend/src/types/api/dashboard/create.ts
- frontend/src/types/api/dashboard/getAll.ts
- frontend/src/types/api/queryBuilder/queryBuilderData.ts
- frontend/src/types/common/operations.types.ts
- frontend/src/types/common/queryBuilder.ts
- frontend/tests/dashboards/utils.ts
- pkg/query-service/app/clickhouseReader/reader.go
- pkg/query-service/app/limit.go
- pkg/query-service/app/metrics/v4/query_builder.go
- pkg/query-service/app/parser.go
- pkg/query-service/common/metrics.go
- pkg/query-service/model/v3/v3.go
Additional comments: 1
frontend/src/container/QueryBuilder/filters/AggregatorFilter/AggregatorFilter.tsx (1)
- 114-114: The addition of the optional chaining operator (
?.) before accessingattributeKeysis a good practice to handle potential null or undefined values inpayload. This change enhances the robustness of the code by preventing runtime errors that could occur ifpayloadwere null or undefined. It's a small but significant improvement in error handling within the application.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx
YounixM
force-pushed
the
metrics-qb-change
branch
from
16704f3
to
b04dead
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (6)
frontend/public/locales/en-GB/dashboard.jsonis excluded by:!**/*.jsonfrontend/public/locales/en/dashboard.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/dashboard/dashboardGetCallWithTimeSeriesWidget200.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/dashboard/putDashboardWithTimeSeries200.jsonis excluded by:!**/*.jsonfrontend/tests/fixtures/api/traces/traceExplorerViews200.jsonis excluded by:!**/*.jsongo.sumis excluded by:!**/*.sum
Files selected for processing (54)
- frontend/src/api/apiV1.ts (1 hunks)
- frontend/src/api/index.ts (2 hunks)
- frontend/src/api/metrics/getQueryRange.ts (2 hunks)
- frontend/src/constants/queryBuilder.ts (5 hunks)
- frontend/src/constants/queryBuilderOperators.ts (1 hunks)
- frontend/src/constants/queryFunctionOptions.ts (1 hunks)
- frontend/src/constants/shortcuts/DashboardShortcuts.ts (1 hunks)
- frontend/src/constants/shortcuts/QBShortcuts.ts (1 hunks)
- frontend/src/container/FormAlertRules/QuerySection.tsx (2 hunks)
- frontend/src/container/GridCardLayout/GridCardLayout.styles.scss (1 hunks)
- frontend/src/container/GridCardLayout/GridCardLayout.tsx (2 hunks)
- frontend/src/container/GridCardLayout/styles.ts (1 hunks)
- frontend/src/container/ListOfDashboard/DashboardsList.tsx (2 hunks)
- frontend/src/container/MetricsApplication/MetricsPageQueries/MetricsPageQueriesFactory.ts (2 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx (4 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/menuItems.tsx (1 hunks)
- frontend/src/container/NewDashboard/ComponentsSlider/styles.ts (1 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/Description.styles.scss (1 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/SettingsDrawer.tsx (2 hunks)
- frontend/src/container/NewDashboard/DashboardDescription/index.tsx (5 hunks)
- frontend/src/container/NewWidget/LeftContainer/QuerySection/index.tsx (4 hunks)
- frontend/src/container/NewWidget/index.tsx (5 hunks)
- frontend/src/container/OnboardingContainer/Steps/LogsConnectionStatus/LogsConnectionStatus.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Formula/Formula.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.styles.scss (3 hunks)
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.tsx (7 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx (1 hunks)
- frontend/src/container/QueryBuilder/filters/AggregatorFilter/AggregatorFilter.tsx (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx (2 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts (1 hunks)
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts (1 hunks)
- frontend/src/hooks/hotkeys/useKeyboardHotkeys.tsx (1 hunks)
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts (3 hunks)
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts (7 hunks)
- frontend/src/lib/dashboard/getQueryResults.ts (2 hunks)
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts (1 hunks)
- frontend/src/lib/uPlotLib/plugins/tooltipPlugin.ts (4 hunks)
- frontend/src/types/api/dashboard/create.ts (1 hunks)
- frontend/src/types/api/dashboard/getAll.ts (1 hunks)
- frontend/src/types/api/queryBuilder/queryBuilderData.ts (1 hunks)
- frontend/src/types/common/operations.types.ts (2 hunks)
- frontend/src/types/common/queryBuilder.ts (2 hunks)
- frontend/tests/dashboards/utils.ts (3 hunks)
- pkg/query-service/app/clickhouseReader/reader.go (5 hunks)
- pkg/query-service/app/limit.go (1 hunks)
- pkg/query-service/app/metrics/v4/query_builder.go (2 hunks)
- pkg/query-service/app/parser.go (5 hunks)
- pkg/query-service/common/metrics.go (2 hunks)
- pkg/query-service/model/v3/v3.go (5 hunks)
Files skipped from review as they are similar to previous changes (54)
- frontend/src/api/apiV1.ts
- frontend/src/api/index.ts
- frontend/src/api/metrics/getQueryRange.ts
- frontend/src/constants/queryBuilder.ts
- frontend/src/constants/queryBuilderOperators.ts
- frontend/src/constants/queryFunctionOptions.ts
- frontend/src/constants/shortcuts/DashboardShortcuts.ts
- frontend/src/constants/shortcuts/QBShortcuts.ts
- frontend/src/container/FormAlertRules/QuerySection.tsx
- frontend/src/container/GridCardLayout/GridCardLayout.styles.scss
- frontend/src/container/GridCardLayout/GridCardLayout.tsx
- frontend/src/container/GridCardLayout/styles.ts
- frontend/src/container/ListOfDashboard/DashboardsList.tsx
- frontend/src/container/MetricsApplication/MetricsPageQueries/MetricsPageQueriesFactory.ts
- frontend/src/container/NewDashboard/ComponentsSlider/index.tsx
- frontend/src/container/NewDashboard/ComponentsSlider/menuItems.tsx
- frontend/src/container/NewDashboard/ComponentsSlider/styles.ts
- frontend/src/container/NewDashboard/DashboardDescription/Description.styles.scss
- frontend/src/container/NewDashboard/DashboardDescription/SettingsDrawer.tsx
- frontend/src/container/NewDashboard/DashboardDescription/index.tsx
- frontend/src/container/NewWidget/LeftContainer/QuerySection/index.tsx
- frontend/src/container/NewWidget/index.tsx
- frontend/src/container/OnboardingContainer/Steps/LogsConnectionStatus/LogsConnectionStatus.tsx
- frontend/src/container/QueryBuilder/components/Formula/Formula.tsx
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.styles.scss
- frontend/src/container/QueryBuilder/components/QBEntityOptions/QBEntityOptions.tsx
- frontend/src/container/QueryBuilder/components/Query/Query.tsx
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
- frontend/src/container/QueryBuilder/components/SpaceAggregationOptions/SpaceAggregationOptions.tsx
- frontend/src/container/QueryBuilder/filters/AggregatorFilter/AggregatorFilter.tsx
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/OptionRenderer.tsx
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/QueryBuilderSearch.styles.scss
- frontend/src/container/QueryBuilder/filters/QueryBuilderSearch/style.ts
- frontend/src/container/TraceDetail/SelectedSpanDetails/config.ts
- frontend/src/hooks/hotkeys/useKeyboardHotkeys.tsx
- frontend/src/hooks/queryBuilder/useGetQueryRange.ts
- frontend/src/hooks/queryBuilder/useQueryBuilderOperations.ts
- frontend/src/lib/dashboard/getQueryResults.ts
- frontend/src/lib/newQueryBuilder/getMetricsOperatorsByAttributeType.ts
- frontend/src/lib/uPlotLib/plugins/tooltipPlugin.ts
- frontend/src/types/api/dashboard/create.ts
- frontend/src/types/api/dashboard/getAll.ts
- frontend/src/types/api/queryBuilder/queryBuilderData.ts
- frontend/src/types/common/operations.types.ts
- frontend/src/types/common/queryBuilder.ts
- frontend/tests/dashboards/utils.ts
- pkg/query-service/app/clickhouseReader/reader.go
- pkg/query-service/app/limit.go
- pkg/query-service/app/metrics/v4/query_builder.go
- pkg/query-service/app/parser.go
- pkg/query-service/common/metrics.go
- pkg/query-service/model/v3/v3.go
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- frontend/src/container/CreateAlertRule/defaults.ts (1 hunks)
- frontend/src/container/FormAlertRules/QuerySection.tsx (5 hunks)
- frontend/src/container/FormAlertRules/index.tsx (4 hunks)
- frontend/src/container/QueryBuilder/QueryBuilder.interfaces.ts (1 hunks)
- frontend/src/container/QueryBuilder/QueryBuilder.tsx (3 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.interfaces.ts (1 hunks)
- frontend/src/container/QueryBuilder/components/Query/Query.tsx (6 hunks)
- frontend/src/types/api/alerts/def.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- frontend/src/container/FormAlertRules/QuerySection.tsx
- frontend/src/container/QueryBuilder/components/Query/Query.tsx
Additional comments: 9
frontend/src/container/QueryBuilder/components/Query/Query.interfaces.ts (1)
- 10-10: The addition of
showFunctions?: boolean;toQueryPropsis a good enhancement for conditional UI rendering based on the presence of functions in the query builder. Ensure that all components consumingQueryPropsproperly handle this new optional property.frontend/src/types/api/alerts/def.ts (1)
- 25-25: The addition of
version?: string;to theAlertDefinterface aligns with the PR's objectives regarding version handling. Ensure that the version field is correctly utilized and handled in the logic that processesAlertDefinstances.frontend/src/container/QueryBuilder/QueryBuilder.interfaces.ts (1)
- 30-30: The addition of
showFunctions?: boolean;toQueryBuilderPropsis a positive step towards enhancing the query builder's functionality. Ensure that all components consumingQueryBuilderPropsproperly handle this new optional property.frontend/src/container/CreateAlertRule/defaults.ts (1)
- 28-28: The hardcoded addition of
version: 'v4'toalertDefaultsaligns with the PR's objectives regarding API versioning. Ensure that this version is consistently used and handled correctly across the application, especially in areas related to alert definitions.frontend/src/container/QueryBuilder/QueryBuilder.tsx (2)
- 28-28: The addition of the
showFunctionsprop to theQueryBuildercomponent and its subsequent passing to child components is consistent with the PR's objectives. Ensure that this prop is correctly utilized and handled in all child components that receive it.- 170-170: The usage of
showFunctionsin childQuerycomponents to conditionally render UI elements based on the presence of functions is a good implementation detail. Verify that theshowFunctionsprop is correctly handled in all scenarios within these components.Also applies to: 189-189
frontend/src/container/FormAlertRules/index.tsx (3)
- 295-295: Renaming
isAlertAvialabletoisAlertAvailablecorrects a typo and improves code readability. This is a good practice for maintaining clean and understandable code.- 447-448: The logic for determining
isAlertAvailableToSavebased onisAlertAvailable, the query type, and the alert type is a thoughtful addition. Ensure that this logic aligns with the intended behavior and user experience in the alert rule form.- 510-518: The update to the tooltip title based on
isAlertAvailableToSaveis a good user experience enhancement. It helps provide contextual information to the user based on the current state of the form.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss (1 hunks)
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- frontend/src/container/QueryBuilder/components/QueryFunctions/Function.tsx
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.styles.scss
- frontend/src/container/QueryBuilder/components/QueryFunctions/QueryFunctions.tsx
|
I'd appreciate it if you don't force push and avoid it. Since the commits get squashed you can simply merge instead of rebase if that's what making you do that. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (3)
- pkg/query-service/constants/constants.go (1 hunks)
- pkg/query-service/rules/apiParams.go (1 hunks)
- pkg/query-service/rules/thresholdRule.go (10 hunks)
Additional comments: 7
pkg/query-service/rules/apiParams.go (1)
- 53-54: The addition of the
Versionfield to thePostableRulestruct with the JSON tagversion,omitemptyis a good practice for supporting API versioning. This allows for backward compatibility and future enhancements without breaking existing functionality. Ensure that the rest of the codebase properly handles this new field, especially in serialization/deserialization logic and when making decisions based on the version specified in rules.pkg/query-service/constants/constants.go (1)
- 216-216: The addition of the
SIGNOZ_TIMESERIES_v4_1DAY_TABLENAMEconstant with the value"distributed_time_series_v4_1day"is consistent with the naming convention of other table name constants in the file. This addition supports the backend adjustments for the new spatial aggregation functionalities introduced in the PR. Ensure that this constant is used correctly in the SQL queries and parsing logic in the query service to support the new functionalities.pkg/query-service/rules/thresholdRule.go (5)
- 21-21: The import of
github.com/ClickHouse/clickhouse-go/v2/lib/driveris added to support database interactions for fetching temporality information. Ensure that this library version is compatible with the rest of the project's dependencies.- 35-35: The import of
go.signoz.io/signoz/pkg/query-service/app/metrics/v4indicates the introduction of version 4 metrics handling. Verify that all necessary functionalities and interfaces required byThresholdRulefor version 4 are implemented in this package.- 129-134: The creation of a new query builder for version 4 (
queryBuilderV4) usingmetricsV4.PrepareMetricQueryis a significant change. Ensure that thePrepareMetricQueryfunction is correctly implemented to handle spatial aggregations and functions.- 737-756: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [728-753]
The modifications in
prepareBuilderQueriesto handle version-specific query preparation and temporality population are significant. Ensure that the version check (r.version == "v4") correctly routes the query preparation through the appropriate query builder and that temporality information is accurately applied to the queries.
- 1062-1068: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1065-1071]
The changes in
buildAndRunQueryto accommodate version-specific query execution are crucial. Ensure that the method correctly handles the execution of queries based on the specified version and that any errors during query execution are properly logged and handled.
|
@YounixM please fix the removed list view from panel types. |
|
Yes @srikanthccv, aware of this issue. I have to verify the file with develop to make sure existing changes are not removed. That work is pending |
srikanthccv
approved these changes
Mar 1, 2024
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.
UI Changes related to - #4016
Impacted Areas:
Dashboard
Alerts
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor