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: raw log data not rendering in log context #6991

Merged
merged 4 commits into from
Jan 30, 2025

Conversation

ahmadshaheer
Copy link
Collaborator

@ahmadshaheer ahmadshaheer commented Jan 30, 2025

Summary

  • fix the issue of no data in log context
  • add null check to traceData

Related Issues / PR's

Thread: https://signoz-team.slack.com/archives/C089D1B5516/p1738256980047269

Screenshots

Before:
Screenshot 2025-01-30 at 10 39 09 PM

After:
image

Affected Areas and Manually Tested Areas

  • Log context of right sidebar in logs explorer

Important

Fix raw log data rendering in ContextLogRenderer.tsx and add null check for trace data in TraceDetailV2.tsx.

  • Log Rendering:
    • Add defaultLogsSelectedFields in ContextLogRenderer.tsx to ensure raw log data is rendered with default selected fields.
    • Pass selectedFields to RawLogView in ContextLogRenderer.tsx.
  • Trace Data Handling:
    • Add null check for traceData?.payload?.spans?.length in TraceDetailV2.tsx to prevent errors when spans are undefined.
  • Misc:
    • Minor adjustments to useEffect dependencies in ContextLogRenderer.tsx and TraceDetailV2.tsx.

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

@ahmadshaheer ahmadshaheer requested a review from YounixM as a code owner January 30, 2025 18:01
@github-actions github-actions bot added the bug Something isn't working label Jan 30, 2025
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.

❌ Changes requested. Reviewed everything up to 5cbdd3a in 1 minute and 44 seconds

More details
  • Looked at 62 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/pages/TraceDetailV2/TraceDetailV2.tsx:71
  • Draft comment:
    Consider adding null checks for traceData?.payload in other usages to ensure consistency and avoid potential null reference errors.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. frontend/src/container/LogDetailedView/ContextView/ContextLogRenderer.tsx:110
  • Draft comment:
    Avoid using inline styles. Consider using CSS classes or styled components instead. This is also applicable to other inline styles in the file.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_Kw9IgRBP3O6CKKOC


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.

frontend/src/container/OptionsMenu/constants.ts Outdated Show resolved Hide resolved
@YounixM YounixM force-pushed the fix/raw-log-data-not-rendering-in-log-context branch from 5cbdd3a to b389d37 Compare January 30, 2025 18:03
@ahmadshaheer ahmadshaheer force-pushed the fix/raw-log-data-not-rendering-in-log-context branch from b389d37 to 44cd538 Compare January 30, 2025 18:07
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! Incremental review on 44cd538 in 22 seconds

More details
  • Looked at 57 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/container/LogDetailedView/ContextView/ContextLogRenderer.tsx:22
  • Draft comment:
    Ensure defaultLogsSelectedFields is not used elsewhere in the codebase, as its redefinition here might break other parts of the application.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_ehOCQd52ZFwT7wAz


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

@vikrantgupta25 vikrantgupta25 enabled auto-merge (squash) January 30, 2025 18:16
@vikrantgupta25 vikrantgupta25 merged commit d910d99 into main Jan 30, 2025
15 of 16 checks passed
@vikrantgupta25 vikrantgupta25 deleted the fix/raw-log-data-not-rendering-in-log-context branch January 30, 2025 18:16
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.

3 participants