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

Store label values sent from association properties #198

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Store label values sent from association properties #198

merged 2 commits into from
Nov 12, 2024

Conversation

dinmukhamedm
Copy link
Member

@dinmukhamedm dinmukhamedm commented Nov 12, 2024

  • Fix some filters in queries

Important

Enhance label handling and span type filtering, update database schema, and refine frontend filters and templates.

  • Behavior:
    • Add CODE to LabelSource enum in labels.rs.
    • Implement record_labels_to_db() in utils.rs to store label values from association properties.
    • Add FromStr implementation for SpanType in spans.rs.
    • Modify get_label_classes_by_project_id() in labels.rs to order by created_at.
    • Add filter for top_span_type in trace.rs.
  • Database:
    • Update label_source enum to include CODE in 0005_misty_leper_queen.sql.
    • Create playgrounds table in 0005_misty_leper_queen.sql.
    • Add new indexes in 0005_misty_leper_queen.sql.
  • Frontend:
    • Modify filtersToSql() in modifiers.ts to support castType.
    • Remove semantic_event_eval templates from template-select.tsx.
    • Add span_type filter handling in route.ts.

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

* fix span type filter in traces view

* remove unused pipeline templates group
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 b7b9afc in 1 minute and 6 seconds

More details
  • Looked at 4695 lines of code in 16 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. app-server/src/db/labels.rs:112
  • Draft comment:
    The create_label_class function has been removed. Ensure that this functionality is not required elsewhere in the codebase, or provide an alternative implementation if necessary.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. app-server/src/traces/utils.rs:153
  • Draft comment:
    Avoid using unwrap on serde_json::from_value. Consider handling the error gracefully to prevent potential panics.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
3. app-server/src/traces/spans.rs:213
  • Draft comment:
    Ensure that strip_prefix is used safely by checking if the prefix exists before calling it to avoid unexpected behavior.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The get_labels function in SpanAttributes uses strip_prefix without checking if the prefix exists. This could lead to unexpected behavior if the prefix is not present.

Workflow ID: wflow_aFAGu3FcnYlyemM5


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.

app-server/src/db/trace.rs Show resolved Hide resolved
@dinmukhamedm dinmukhamedm merged commit 8553e63 into main Nov 12, 2024
1 check passed
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.

1 participant