feat: add timezone support to the graphs throughout the app #6520

ahmadshaheer · 2024-11-24T15:29:07Z

Add timezone support to the graphs throughout the app

Service details page

Overview tab

Latency
Rate (ops/s)
Apdex
Error Percentage

DB Call Metrics tab

DB Calls RPS
Database Calls Avg Duration

External Metrics tab

External Call Error Percentage
External Call duration
External Call RPS(by Address)
External Call duration(by Address)

Traces explorer

Time Series Graph

Logs explorer

Time Series Graph

Dashboard

Details page, New Panel, Edit Panel

Time Series Graph
Bar Graph

Messaging queues

Alert Details

Overview Tab

Graph
Anomaly based alert graph

Alert History Tab

Timeline Graph

Trace details

Trace details chart tooltip
Trace details

Note:

This PR doesn't include the logs explorer preview chart (which is built using Chart.js), it'll be picked up separately

Related Issues / PR's

Part of https://github.com/SigNoz/engineering-pod/issues/2005

Screenshots

Affected Areas and Manually Tested Areas

Important

Add timezone support to graphs and charts using a custom hook for consistent date formatting across the application.

Timezone Support:
- Introduced useTimezone hook in providers/Timezone.tsx to manage timezone settings.
- Applied timezone formatting to graphs in Graph.tsx, AnomalyAlertEvaluationView.tsx, ChartPreview/index.tsx, NodeMetrics.tsx, PodMetrics.tsx, UplotPanelWrapper.tsx, TimeSeriesView.tsx, DateTimeSelectionV2/index.tsx, TraceDetail/index.tsx.
- Updated tooltipPlugin.ts to format timestamps using the current timezone.
Graph Components:
- Modified getUplotChartOptions.ts to include tzDate and timezone options for uPlot.
- Updated various graph components to use timezone-aware date formatting.
Miscellaneous:
- Note: Logs explorer preview chart changes are not included and will be handled separately.

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

…artOptions traces, logs time series, dashboard, service details (time series, bar), Node, Pod, Alert graphs

…eholder

This reverts commit f8411ab.

github-actions · 2024-11-24T15:29:21Z

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

github-actions · 2024-11-24T15:29:38Z

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

github-actions · 2024-11-24T15:29:55Z

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! Reviewed everything up to 234d71a in 1 minute and 10 seconds

More details

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

1. frontend/src/container/AlertHistory/Timeline/Graph/Graph.tsx:123

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment claims that timezone is always defined, but without seeing the implementation of useTimezone, it's speculative. The use of optional chaining is a defensive programming practice to prevent runtime errors if timezone is undefined. Without strong evidence that timezone is always defined, the comment seems speculative.
I might be missing the context of how useTimezone is implemented, which could confirm whether timezone is always defined. The comment could be correct if useTimezone guarantees a defined timezone.
Without access to the useTimezone implementation, I should err on the side of caution and assume the comment is speculative. Optional chaining is a common practice to handle potential undefined values safely.
The comment should be deleted as it is speculative without evidence that timezone is always defined. Optional chaining is a safe practice.

2. frontend/src/container/AnomalyAlertEvaluationView/AnomalyAlertEvaluationView.tsx:157

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

3. frontend/src/container/AnomalyAlertEvaluationView/AnomalyAlertEvaluationView.tsx:263

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

4. frontend/src/container/FormAlertRules/ChartPreview/index.tsx:244

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

5. frontend/src/container/FormAlertRules/ChartPreview/index.tsx:245

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

6. frontend/src/container/GantChart/Span/index.tsx:45

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

7. frontend/src/container/LogDetailedView/InfraMetrics/NodeMetrics.tsx:94

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

8. frontend/src/container/LogDetailedView/InfraMetrics/NodeMetrics.tsx:95

Draft comment:
The use of optional chaining with timezone?.value is unnecessary since timezone is always defined in the context. Consider using timezone.value instead.
Reason this comment was not posted:
Marked as duplicate.

Workflow ID: wflow_cHZt1i2FXP4VlGNK

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

github-actions · 2024-11-24T15:31:08Z

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

github-actions · 2024-11-24T15:31:19Z

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

…ker placeholder" This reverts commit 429c87a.

github-actions · 2024-11-25T04:49:26Z

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

github-actions · 2024-11-25T04:57:29Z

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 d5dc65a in 24 seconds

More details

Looked at 32 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 2 drafted comments based on config settings.

1. frontend/src/container/TopNav/DateTimeSelectionV2/index.tsx:664

Draft comment:
The removal of timezone handling here might lead to incorrect time display if the timezone is not managed elsewhere. Ensure that the timezone is correctly applied to the date-time values used in the component.
Reason this comment was not posted:
Comment did not seem useful.

2. frontend/src/container/TopNav/DateTimeSelectionV2/index.tsx:618

Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is also applicable at line 632.
Reason this comment was not posted:
Comment was not on a valid diff hunk.

Workflow ID: wflow_398CDOsA4DRscp64

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

github-actions · 2024-11-25T05:12:07Z

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

* feat: display timezone adjusted time range in time picker * fix: open the timepicker on clicking the input after closing the timezone picker by clicking outside (#6522)

SagarRajput-7 · 2024-12-02T08:47:02Z

frontend/src/container/AnomalyAlertEvaluationView/tooltipPlugin.ts

@@ -17,7 +19,7 @@ const tooltipPlugin = (
 			return value.toFixed(3);
 		}
 		if (value instanceof Date) {
-			return value.toLocaleString();
+			return dayjs(value).tz(timezone).format('MM/DD/YYYY, h:mm:ss A');


earlier this was string coming from consumer, you had a format, just recheck if this is expected, and you are not overriding anything

@SagarRajput-7, this is the tooltip plugin for anomaly alert. No, we aren't overriding anything, this is the only change that we have made, i.e., introduced timezone-adjusted timestamp formatting.

…app (#6565) * fix: make x axis and tooltip time format consistent in uPlot graphs * feat: custom hook for formatting timezone * feat: add timezone support to traces explorer timestamp column * feat: add timezone support to saved views ...

ahmadshaheer added 8 commits November 21, 2024 09:53

chore: custom hook for handling timezone formatting and caching

f8411ab

chore: remove undefined from Timezone type

932d9d9

feat: handle timezone adjusted timestamps for consumers of getUPlotCh…

d47cba5

…artOptions traces, logs time series, dashboard, service details (time series, bar), Node, Pod, Alert graphs

feat: apply timezone adjustment to anomaly based alerts graph

f239132

feat: apply timezone adjustment to alert history graph

4cd2935

feat: apply timezone to trace details page

d35ee88

feat: display timezone adjusted custom time range in time picker plac…

429c87a

…eholder

Revert "chore: custom hook for handling timezone formatting and caching"

234d71a

This reverts commit f8411ab.

ahmadshaheer requested a review from YounixM as a code owner November 24, 2024 15:29

github-actions bot added the enhancement New feature or request label Nov 24, 2024

ahmadshaheer changed the title ~~feat: timezone formatter custom hook and adjust graphs timezone~~ feat: add timezone support to the graphs throughout the app Nov 24, 2024

ellipsis-dev bot reviewed Nov 24, 2024

View reviewed changes

ahmadshaheer changed the title ~~feat: add timezone support to the graphs throughout the app~~ feat: add timezone support to the graphs throughout the app and display timezone adjusted time range in date and time picker Nov 24, 2024

Revert "feat: display timezone adjusted custom time range in time pic…

d5dc65a

…ker placeholder" This reverts commit 429c87a.

ahmadshaheer changed the title ~~feat: add timezone support to the graphs throughout the app and display timezone adjusted time range in date and time picker~~ feat: add timezone support to the graphs throughout the app Nov 25, 2024

ellipsis-dev bot reviewed Nov 25, 2024

View reviewed changes

feat: display timezone adjusted time range in time picker (#6521)

89cb5af

* feat: display timezone adjusted time range in time picker * fix: open the timepicker on clicking the input after closing the timezone picker by clicking outside (#6522)

SagarRajput-7 reviewed Dec 2, 2024

View reviewed changes

SagarRajput-7 approved these changes Dec 2, 2024

View reviewed changes

ahmadshaheer merged commit 1b48eb8 into feat/timezone-picker-basic-functionality Dec 3, 2024
6 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add timezone support to the graphs throughout the app #6520

feat: add timezone support to the graphs throughout the app #6520

ahmadshaheer commented Nov 24, 2024 •

edited

Loading

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 24, 2024

ellipsis-dev bot left a comment

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 25, 2024

github-actions bot commented Nov 25, 2024

ellipsis-dev bot left a comment

github-actions bot commented Nov 25, 2024

SagarRajput-7 Dec 2, 2024

ahmadshaheer Dec 2, 2024

feat: add timezone support to the graphs throughout the app #6520

feat: add timezone support to the graphs throughout the app #6520

Conversation

ahmadshaheer commented Nov 24, 2024 • edited Loading

Add timezone support to the graphs throughout the app

Service details page

Overview tab

DB Call Metrics tab

External Metrics tab

Traces explorer

Logs explorer

Dashboard

Details page, New Panel, Edit Panel

Messaging queues

Alert Details

Overview Tab

Alert History Tab

Trace details

Note:

Related Issues / PR's

Screenshots

Affected Areas and Manually Tested Areas

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 24, 2024

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 24, 2024

github-actions bot commented Nov 25, 2024

github-actions bot commented Nov 25, 2024

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 25, 2024

SagarRajput-7 Dec 2, 2024

Choose a reason for hiding this comment

ahmadshaheer Dec 2, 2024

Choose a reason for hiding this comment

ahmadshaheer commented Nov 24, 2024 •

edited

Loading