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

feature(frontend): adding test spec snippets #2366

Merged
merged 3 commits into from
Apr 11, 2023

Conversation

xoscar
Copy link
Collaborator

@xoscar xoscar commented Apr 11, 2023

This PR adds a way for users to select a predefined test spec snippet so they have a quick start when adding checks for a trace

Changes

  • Updating add test spec button
  • Adding source for snippets

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

https://www.loom.com/share/dee7d821ab7d4bb1b4f53d81659e02c3

@xoscar xoscar self-assigned this Apr 11, 2023
@xoscar xoscar linked an issue Apr 11, 2023 that may be closed by this pull request

export type TSnippet = Required<IValues>;

export const HTTP_SPANS_STATUS_CODE: TSnippet = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here's where we can continue adding snippets, you can add it manually using the UI and then copy the details in this file @kdhamric

],
};

export const TEST_SPEC_SNIPPETS: TSnippet[] = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Dont forget to add it to this array

@xoscar xoscar marked this pull request as ready for review April 11, 2023 18:36
Copy link
Contributor

@jorgeepc jorgeepc left a comment

Choose a reason for hiding this comment

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

🔥 Looks great @xoscar Added one question, please take a look.

Comment on lines +47 to +51
children: TEST_SPEC_SNIPPETS.map(snippet => ({
label: snippet.name,
key: snippet.name,
onClick: () => onSnippetClick(snippet),
})),
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking if we should include an option to create an empty test spec. I know you can click the left part of the button but maybe users won't get it.

Screenshot 2023-04-11 at 14 08 33

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would it behave differently than clicking the left button? I think we can gather some feedback on the current UX and then we can think of updating it.

We also need @kdhamric feedback on what other snippets he'd like to add

Copy link
Contributor

Choose a reason for hiding this comment

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

It should have the same behavior as clicking the left button. It could be the last item in the list (with a visual separator).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Creating a list now.

I am afraid many people will not find this list of available test specs. A cheesy, but possibly workable solution, would be to have the dropdown list 'open' by default Proposal: if you go to the Test screen and there are no Test Specs defined, the dropdown list is Open. @olha23 thoughts?

Copy link
Collaborator

@kdhamric kdhamric Apr 11, 2023

Choose a reason for hiding this comment

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

Here are 3 more 'prompted test specs'

export const TRIGGER_SPAN_RESPONSE_BODY_CONTAINS: TSnippet = {
    name: 'Trigger Span: Response body contains "this string"',
    selector: 'span[tracetest.span.type="general" name="Tracetest trigger"]',
    assertions: [
      {
        left: 'attr:tracetest.response.body',
        comparator: 'contains',
        right: '"this string"',
      },
    ],
  };


  export const GRPC_SPANS_STATUS_CODE: TSnippet = {
    name: 'All gRPC Spans: Status is Ok',
    selector: 'span[tracetest.span.type="rpc" rpc.system="grpc"]',
    assertions: [
      {
        left: 'attr:grpc.status_code',
        comparator: '=',
        right: '0',
      },
    ],
  };


  export const DB_SPANS_QUALITY_DB_STATEMENT_PRESENT: TSnippet = {
    name: 'All Database Spans: db.statement should always be defined (QUALITY)',
    selector: 'span[tracetest.span.type="database"]',
    assertions: [
      {
        left: 'attr:db.system',
        comparator: '!=',
        right: '""',
      },
    ],
  };

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all done

@xoscar xoscar merged commit e0f422b into main Apr 11, 2023
@xoscar xoscar deleted the 2174-add-test-snippets-user-can-use branch April 11, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test snippets user can use
4 participants