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.

Sign up for GitHub

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

Jump to bottom

feat(transaction-summary): Add analytics to track biases sufficiency assessment #48865

Merged
merged 15 commits into from
May 15, 2023
Merged

feat(transaction-summary): Add analytics to track biases sufficiency assessment #48865

merged 15 commits into from
May 15, 2023

Conversation

priscilawebdev
Copy link
Member

closes #47138

@priscilawebdev priscilawebdev requested a review from a team as a code owner May 10, 2023 14:19
@priscilawebdev priscilawebdev changed the title feat(transaction-summary): Add analytics to track biases-sufficiency-… feat(transaction-summary): Add analytics to track biases sufficiency assessment May 10, 2023
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 10, 2023
matejminar
matejminar reviewed May 10, 2023
View reviewed changes
Copy link
Member

@matejminar matejminar left a comment

Choose a reason for hiding this comment

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

image
We should fire no_samples event on top of baseline event in this scenario.

image
We should not fire baseline if chart is empty.

static/app/utils/analytics/dynamicSamplingAnalyticsEvents.tsx Outdated Show resolved Hide resolved
static/app/views/performance/transactionSummary/transactionOverview/index.tsx Outdated Show resolved Hide resolved
@priscilawebdev
Copy link
Member Author

Thanks @matejminar ... this was not clear to me in the issue description... I have updated the code

@priscilawebdev priscilawebdev requested a review from a team as a code owner May 11, 2023 06:41
@priscilawebdev
Copy link
Member Author

checking tests...

@priscilawebdev
Copy link
Member Author

checking issue with the acceptance tests...

@matejminar
Copy link
Member

We should not fire no_samples if there are no metrics results :(
image

@priscilawebdev
Copy link
Member Author

@matejminar thanks for your feedback. I am sorry about that but it was not described in the acceptance criteria of the issue... I will update it 😉

@priscilawebdev priscilawebdev enabled auto-merge (squash) May 12, 2023 12:22
@priscilawebdev priscilawebdev enabled auto-merge (squash) May 15, 2023 06:58
@priscilawebdev priscilawebdev merged commit d8bdb05 into master May 15, 2023
@priscilawebdev priscilawebdev deleted the priscila/feat/add-analytics-code-to-track-biases-sufficiency-assessment branch May 15, 2023 07:25
@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matejminar matejminar matejminar approved these changes

Assignees
No one assigned
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Biases Sufficiency Assessment
2 participants
@priscilawebdev @matejminar