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

[APM] Add API tests for storage explorer #141391

Merged
merged 6 commits into from
Sep 22, 2022

Conversation

gbamparop
Copy link
Contributor

Add API tests for storage explorer.

Related to #140199

@gbamparop gbamparop added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.5.0 labels Sep 22, 2022
@gbamparop gbamparop requested a review from a team as a code owner September 22, 2022 11:59
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM. just some suggestions

Comment on lines 119 to 121
const transactionStats = body.processorEventStats.find(
({ processorEvent }) => processorEvent === ProcessorEvent.transaction
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could use keyBy to create an object per processorEvent. With that, you won't need to use find for each type.

Suggested change
const transactionStats = body.processorEventStats.find(
({ processorEvent }) => processorEvent === ProcessorEvent.transaction
);
const statsGroupedByEvent = keyBy(body.processorEventStats, 'processorEvent')
const transactionStats = statsGroupedByEvent[ProcessorEvent.transaction]
const spanStats = statsGroupedByEvent[ProcessorEvent.span]
....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, added it in b8679d2

Comment on lines 112 to 114
const goServiceStats = body.serviceStatistics.find(
({ serviceName }) => serviceName === 'opbeans-go'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here


expect(status).to.be(200);
expect(body.numberOfServices).to.be(2);
expect(Math.round(body.tracesPerMinute)).to.be(2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in b8679d2

@gbamparop gbamparop enabled auto-merge (squash) September 22, 2022 16:50
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@gbamparop gbamparop merged commit 4137fb3 into elastic:main Sep 22, 2022
kibanamachine pushed a commit that referenced this pull request Sep 22, 2022
* Add API tests for storage explorer

(cherry picked from commit 4137fb3)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Sep 22, 2022
* Add API tests for storage explorer

(cherry picked from commit 4137fb3)

Co-authored-by: Giorgos Bamparopoulos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants