Metrics time selection #6360

rahulkeswani101 · 2024-11-04T14:57:48Z

Summary

Related Issues / PR's

Screenshots

Affected Areas and Manually Tested Areas

Important

Adds modal time selection to host metrics, logs, and traces with new components and styles.

Behavior:
- Adds isModalTimeSelection prop to HostMetricDetail and subcomponents for modal time selection.
- Integrates DateTimeSelectionV2 for time range selection in Metrics, HostMetricLogsDetailedView, and HostMetricTraces.
- Updates HostsList to pass isModalTimeSelection to HostMetricDetail.
Components:
- New HostMetricTraces component for displaying host metric traces with time selection.
- New HostMetricsLogs and HostMetricLogsDetailedView components for displaying logs with time selection.
Styles:
- Adds styles for HostMetricTraces and HostMetricsLogs in respective .scss files.
- Updates Metrics.styles.scss for new layout adjustments.
Utilities:
- Adds formatNanoToMS utility in HostMetricTraces/utils.ts for formatting durations.
- Adds constants for query payloads in HostMetricTraces/constants.ts and HostMetricsLogs/constants.ts.

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

github-actions · 2024-11-04T14:58:03Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

github-actions · 2024-11-04T14:58:24Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

ellipsis-dev

❌ Changes requested. Reviewed everything up to a510958 in 1 minute and 40 seconds

More details

Looked at 1191 lines of code in 14 files
Skipped 0 files when reviewing.
Skipped posting 5 drafted comments based on config settings.

1. frontend/src/components/HostMetricsDetail/HostMetricTraces/HostMetricTraces.tsx:22

Draft comment:
Since offset is never updated, consider using a constant instead of useState for better performance and clarity.

const offset = 0;

Reason this comment was not posted:
Confidence changes required: 50%
The use of useState<number>(0) for offset is unnecessary since offset is never updated. It can be a constant instead.

2. frontend/src/components/HostMetricsDetail/HostMetricTraces/constants.ts:54

Draft comment:
The offset parameter is always 0 and never changes, making it redundant. Consider removing it from the function signature and calls.
Reason this comment was not posted:
Confidence changes required: 50%
The offset parameter in getHostTracesQueryPayload is set to 0 by default, and since offset is never updated, this parameter is redundant.

3. frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricsLogs.tsx:32

Draft comment:
Ensure that the logic for updating prevOffset and afterOffset is correct and does not lead to potential bugs. Consider adding comments or refactoring for clarity.
Reason this comment was not posted:
Confidence changes required: 50%
The HostMetricsLogs component uses prevOffset and afterOffset to manage pagination, but the logic for updating these offsets is not clear. It might lead to potential bugs if not handled correctly.

4. frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricsLogs.tsx:33

Draft comment:
Ensure that the logic for updating prevOffset and afterOffset is correct and does not lead to potential bugs. Consider adding comments or refactoring for clarity.
Reason this comment was not posted:
Confidence changes required: 50%
The HostMetricsLogs component uses prevOffset and afterOffset to manage pagination, but the logic for updating these offsets is not clear. It might lead to potential bugs if not handled correctly.

5. frontend/src/components/HostMetricsDetail/index.tsx:12

Draft comment:
Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code.
Reason this comment was not posted:
Comment was on unchanged code.

Workflow ID: wflow_nW0E3CKcnChQe6rS

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.

ellipsis-dev · 2024-11-04T14:59:37Z

frontend/src/components/HostMetricsDetail/HostMetricTraces/HostMetricTraces.styles.scss

@@ -0,0 +1,30 @@
+.host-metric-traces {
+    .ant-table {
+        background: #121212;


Use design tokens or predefined color constants instead of hardcoding color values for background and color. This applies to lines 3, 4, 7, 8, 9, 13, 14, 15, 19, and 26.

Suggested change

background: #121212;

background: $background-color;

ellipsis-dev · 2024-11-04T14:59:37Z

frontend/src/components/HostMetricsDetail/HostMetricTraces/HostMetricTraces.tsx

+			/>
+			{isLoading && traces.length > 0 && (
+				<Skeleton
+					style={{


Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This also applies to the inline styles on lines 166 and 182 in HostMetricsLogs.tsx.

@SagarRajput-7 Let me add these changes of Traces in the new PR.

ellipsis-dev · 2024-11-04T14:59:37Z

frontend/src/components/HostMetricsDetail/HostMetricTraces/utils.ts

@@ -0,0 +1,4 @@
+export const formatNanoToMS = (nanoSeconds: number): string => {


The formatNanoToMS function duplicates the conversion logic of nanoToMilli. Consider using nanoToMilli and appending 'ms' to its result to avoid duplication.

function nanoToMilli (timeUtils.ts)

github-actions · 2024-11-06T06:52:03Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

SagarRajput-7 · 2024-11-06T07:07:48Z

frontend/src/components/HostMetricsDetail/HostMetricTraces/constants.ts

+					queryName: 'A',
+					aggregateOperator: 'noop',
+					aggregateAttribute: {
+						id: '------false',


check if there's an id available - which includes key also, something like 'host.name--string--resource--false'

SagarRajput-7 · 2024-11-06T07:07:58Z

frontend/src/components/HostMetricsDetail/HostMetricTraces/constants.ts

+					aggregateAttribute: {
+						id: '------false',
+						dataType: DataTypes.EMPTY,
+						key: '',


frontend/src/components/HostMetricsDetail/HostMetricTraces/constants.ts

SagarRajput-7 · 2024-11-06T07:09:29Z

frontend/src/components/HostMetricsDetail/HostMetricTraces/constants.ts

+			limit: 10,
+			offset,
+		},
+		selectColumns: [


check if we can use the constant - defaultTraceSelectedColumns

frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricLogsDetailedView.tsx

frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricsLogs.tsx

frontend/src/components/HostMetricsDetail/HostMetricsLogs/constants.ts

SagarRajput-7

Please check the comments

github-actions · 2024-11-06T07:30:38Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

ellipsis-dev

👍 Looks good to me! Incremental review on 9368e74 in 52 seconds

More details

Looked at 99 lines of code in 3 files
Skipped 0 files when reviewing.
Skipped posting 5 drafted comments based on config settings.

1. frontend/src/components/HostMetricsDetail/HostMetricsLogs/constants.ts:57

Draft comment:
Consider whether generating a new UUID for each query is necessary. If the ID is not used for distinguishing between different queries, a static ID might suffice.
Reason this comment was not posted:
Confidence changes required: 50%
The use of uuidv4() for generating query IDs ensures uniqueness, which is good for preventing potential conflicts. However, it might be unnecessary if the ID is not used for distinguishing between different queries in a meaningful way.

2. frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricsLogs.tsx:163

Draft comment:
Good use of the Skeleton component for loading states. The change from inline styles to a CSS class improves maintainability.
Reason this comment was not posted:
Confidence changes required: 0%
The Skeleton component is used for loading states, which is a good practice for user experience. However, the inline styles were replaced with a class, which is a good change for maintainability and consistency.

3. frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricsLogs.tsx:170

Draft comment:
Consider refactoring the repeated logic in useEffect hooks for processing prevData and afterData into a separate function to avoid duplication.
Reason this comment was not posted:
Confidence changes required: 50%
The useEffect hooks are correctly used to update the logs state when new data is fetched. However, the logic for checking and mapping the data is repeated in both hooks, which could be refactored into a separate function to avoid duplication.

4. frontend/src/components/HostMetricsDetail/HostMetricsLogs/HostMetricsLogs.tsx:170

Draft comment:
Avoid using inline styles in React components. Move the style to an external stylesheet or use styled components.
Reason this comment was not posted:
Comment looked like it was already resolved.

5. frontend/src/components/HostMetricsDetail/HostMetricsLogs/constants.ts:7

Draft comment:
Do not add non-ClickHouse related functions like uuidv4() to the ClickHouseReader interface. Use the DAO in the telemetry instance instead.
Reason this comment was not posted:
Comment was on unchanged code.

Workflow ID: wflow_ivm4CZTtj8nsk6n6

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

rahulkeswani101 · 2024-11-06T07:32:29Z

Please check the comments

@SagarRajput-7 Resolved some of the comments. For traces I am testing some changes so, I will address the changes for traces in that PR.

SagarRajput-7 · 2024-11-06T07:55:35Z

Please check the comments

@SagarRajput-7 Resolved some of the comments. For traces I am testing some changes so, I will address the changes for traces in that PR.

sure, I am approving this, please make sure to address the remaining comment in followup PR

ellipsis-dev

Skipped PR review on df43f0e because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.

* feat: added the host list view and filters * feat: removed group by filter and added autocomplete for where clause * feat: updated the table view and added the pagination * feat: pass updated filters to api to get filtered data in the list * feat: added global time range and order by for cpu,memory,iowait,load * feat: added order by and color codes for cpu and memory usage progress bar * refactor: removed inline styles * Host lists improvement (#6366) * style: added new style changes for date time selection in host lists view * style: added padding to date time selector * style: removed unnecessary styles for host tabs * style: removed unused css * feat: added the host detail view (#6267) * Host containers (#6297) * feat: added the host detail view * feat: completed containers and processes details view * Show host metrics panels in metrics tab. (#6306) * feat: added the host detail view * feat: completed containers and processes details view * feat: added host metrics panels in metrics tabs * refactor: removed inline styles from host containers and processes tabs * style: added top and bottom margin to containers and processes tab * Metrics time selection (#6360) * feat: added the host detail view * feat: completed containers and processes details view * feat: added host metrics panels in metrics tabs * refactor: removed inline styles from host containers and processes tabs * feat: added logs and traces tab in host metrics detail view * chore: removed console statements * feat: added DateTimeSelection component in metrics tab * style: added top and bottom margin to containers and processes tab * style: removed inline styles * feat: added logs and traces tab in host metrics detail view (#6359) * feat: added the host detail view * feat: completed containers and processes details view * feat: added host metrics panels in metrics tabs * refactor: removed inline styles from host containers and processes tabs * feat: added logs and traces tab in host metrics detail view * chore: removed console statements * feat: added filters and time selection in traces tab * fix: resolved metrics,logs and traces tab issues * feat: added navigation for logs and traces to respective explorer pages * fix: added the code for logs tab and navigation to respective explorer page * fix: added fixes for date time selection custom issue * style: added styles for light mode * refactor: removed unused code and added comments * refactor: added new code for host metric attribute keys * feat: reset query data once we are on infra monitoring page * chore: remove optional parameter from get attributes and groupby interfaces * feat: update ui as per the designs * fix: logs list, time select and other ui issues * feat: update title for infra monitoring page * feat: update copies * feat: update styles for light mode * fix: reset page size on filter, open explorers in new tab, enable horizontal scroll * feat: traces tab updates * feat: move infra monitoring behind ff * fix: remove sorting from host listing page --------- Co-authored-by: Yunus M <myounis.ar@live.com>

* feat: added the host list view and filters (#6210) * feat: added the host list view and filters * feat: removed group by filter and added autocomplete for where clause * feat: updated the table view and added the pagination * feat: pass updated filters to api to get filtered data in the list * feat: added global time range and order by for cpu,memory,iowait,load * feat: added order by and color codes for cpu and memory usage progress bar * refactor: removed inline styles * Host lists improvement (#6366) * style: added new style changes for date time selection in host lists view * style: added padding to date time selector * style: removed unnecessary styles for host tabs * style: removed unused css * feat: added the host detail view (#6267) * Host containers (#6297) * feat: added the host detail view * feat: completed containers and processes details view * Show host metrics panels in metrics tab. (#6306) * feat: added the host detail view * feat: completed containers and processes details view * feat: added host metrics panels in metrics tabs * refactor: removed inline styles from host containers and processes tabs * style: added top and bottom margin to containers and processes tab * Metrics time selection (#6360) * feat: added the host detail view * feat: completed containers and processes details view * feat: added host metrics panels in metrics tabs * refactor: removed inline styles from host containers and processes tabs * feat: added logs and traces tab in host metrics detail view * chore: removed console statements * feat: added DateTimeSelection component in metrics tab * style: added top and bottom margin to containers and processes tab * style: removed inline styles * feat: added logs and traces tab in host metrics detail view (#6359) * feat: added the host detail view * feat: completed containers and processes details view * feat: added host metrics panels in metrics tabs * refactor: removed inline styles from host containers and processes tabs * feat: added logs and traces tab in host metrics detail view * chore: removed console statements * feat: added filters and time selection in traces tab * fix: resolved metrics,logs and traces tab issues * feat: added navigation for logs and traces to respective explorer pages * fix: added the code for logs tab and navigation to respective explorer page * fix: added fixes for date time selection custom issue * style: added styles for light mode * refactor: removed unused code and added comments * refactor: added new code for host metric attribute keys * feat: reset query data once we are on infra monitoring page * chore: remove optional parameter from get attributes and groupby interfaces * feat: update ui as per the designs * fix: logs list, time select and other ui issues * feat: update title for infra monitoring page * feat: update copies * feat: update styles for light mode * fix: reset page size on filter, open explorers in new tab, enable horizontal scroll * feat: traces tab updates * feat: move infra monitoring behind ff * fix: remove sorting from host listing page --------- Co-authored-by: Yunus M <myounis.ar@live.com> * chore: fix lint errors --------- Co-authored-by: rahulkeswani101 <rahul@signoz.io>

rahulkeswani101 added 7 commits October 25, 2024 10:28

feat: added the host detail view

e3fdf42

feat: completed containers and processes details view

7c5081a

feat: added host metrics panels in metrics tabs

aabce6e

refactor: removed inline styles from host containers and processes tabs

dc9985a

feat: added logs and traces tab in host metrics detail view

c8bfe87

chore: removed console statements

698a771

feat: added DateTimeSelection component in metrics tab

a510958

rahulkeswani101 requested a review from srikanthccv November 4, 2024 14:57

rahulkeswani101 requested a review from YounixM as a code owner November 4, 2024 14:57

ellipsis-dev bot reviewed Nov 4, 2024

View reviewed changes

rahulkeswani101 added 3 commits November 5, 2024 11:24

style: added top and bottom margin to containers and processes tab

803f72b

chore: resolve merge conflicts

3c6dab3

Merge branch 'host-metrics' into metrics-time-selection

4112d04