Skip to content
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

fix: added handling for new key - duration_nano in traces #6658

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SagarRajput-7
Copy link
Contributor

@SagarRajput-7 SagarRajput-7 commented Dec 18, 2024

Summary

We have added duration_nano in as a suggestion when adding columns in traces table, but the ms conversion handling was missing. Added that.

Related Issues / PR's

https://github.com/SigNoz/engineering-pod/issues/2095

Screenshots

Before:

image

After:

image

Affected Areas and Manually Tested Areas


Important

Add handling for duration_nano key alongside durationNano in various components and utilities.

  • Behavior:
    • Add handling for duration_nano key in addition to durationNano in TimeSeriesViewContainer in index.tsx.
    • Update getSpanOrderParam in util.ts to handle duration_nano.
    • Modify getListColumns in utils.tsx to support duration_nano key for rendering duration in milliseconds.

This description was created by Ellipsis for b6159a7. It will automatically update as commits are pushed.

@github-actions github-actions bot added the bug Something isn't working label Dec 18, 2024
Copy link

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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to b6159a7 in 12 seconds

More details
  • Looked at 40 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/container/TimeSeriesView/index.tsx:32
  • Draft comment:
    Consider defining a constant for 'durationNano' and 'duration_nano' to avoid repetition and potential errors in future changes. This applies to similar checks in other files as well.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code changes are consistent across files, ensuring both 'durationNano' and 'duration_nano' are handled. However, the code could be improved for maintainability by using a constant for these keys.
2. frontend/src/container/TracesExplorer/ListView/utils.tsx:102
  • Draft comment:
    Avoid using inline styles. Consider using CSS classes or styled components instead. This is also applicable in other parts of the code where inline styles are used.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_hZzwP0HPyFePJ6Og


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

Copy link

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

@pranay01 pranay01 deleted the branch main December 19, 2024 13:03
@pranay01 pranay01 closed this Dec 19, 2024
@SagarRajput-7 SagarRajput-7 reopened this Dec 21, 2024
@SagarRajput-7 SagarRajput-7 changed the base branch from develop to main December 21, 2024 06:17
Copy link

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

Copy link

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

1 similar comment
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants