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

feat: filter spans #6785

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: filter spans #6785

wants to merge 1 commit into from

Conversation

smbera
Copy link

@smbera smbera commented Jan 9, 2025

Summary

Related Issues / PR's

Related to #6784

Screenshots

image


Important

Adds span filtering feature to TraceDetail component, allowing users to filter spans by name and serviceName.

  • Behavior:
    • Adds span filtering feature in TraceDetail component in index.tsx.
    • Introduces search input to filter spans by name and serviceName.
    • Displays Empty component if no spans match the filter.
  • Functions:
    • Adds getFilteredData in utils.ts to filter spans based on search input.
    • Uses DEFAULT_FILTER_KEYS for filtering criteria.
  • UI:
    • Updates TraceDetail to include Input.Search for filtering spans.
    • Uses useDebouncedFn to debounce search input changes.

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

@smbera smbera requested a review from YounixM as a code owner January 9, 2025 03:43
Copy link

welcome bot commented Jan 9, 2025

Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗

@CLAassistant
Copy link

CLAassistant commented Jan 9, 2025

CLA assistant check
All committers have signed the CLA.

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 4fd73e8 in 1 minute and 23 seconds

More details
  • Looked at 213 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/container/TraceDetail/utils.ts:101
  • Draft comment:
    Using JSON.stringify for string comparison is inefficient. Consider using direct string comparison instead.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The use of JSON.stringify() here is actually reasonable because: 1) val could be any type, not just string 2) We need a way to convert complex objects to searchable text 3) Direct string comparison wouldn't work for object/array values. The comment's suggestion could actually introduce bugs.
    Maybe there's a more efficient way to serialize just the needed fields rather than the entire object? Maybe type checking could avoid stringify for simple types?
    While optimizations are possible, the current approach is safe and handles all cases. The performance impact is likely negligible for the typical size of trace data.
    The comment should be deleted as it suggests a change that could introduce bugs, and the current implementation is a reasonable approach for handling heterogeneous data types.
2. frontend/src/container/TraceDetail/index.tsx:279
  • Draft comment:
    The key prop should be a unique identifier, like tree.id, instead of tree as never.
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. frontend/src/container/TraceDetail/index.tsx:277
  • Draft comment:
    The key prop should be a unique identifier, like tree.id, instead of tree as never.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_c7EXwssIIEoCjDd8


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

@smbera smbera force-pushed the feature/issues/6784 branch from 4fd73e8 to 4979106 Compare January 9, 2025 03:50
@smbera smbera force-pushed the feature/issues/6784 branch from 4979106 to d244159 Compare January 9, 2025 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants