Skip to content

[APM] Show transaction rate per minute on Observability Overview page#70336

Merged
cauemarcondes merged 6 commits intoelastic:masterfrom
cauemarcondes:apm-transaction-per-min
Jul 2, 2020
Merged

[APM] Show transaction rate per minute on Observability Overview page#70336
cauemarcondes merged 6 commits intoelastic:masterfrom
cauemarcondes:apm-transaction-per-min

Conversation

@cauemarcondes
Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes commented Jun 30, 2020

closes #70332

Returns the transaction rate per minute in a given time range instead of transaction count.

@cauemarcondes cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jun 30, 2020
@cauemarcondes cauemarcondes requested a review from a team June 30, 2020 15:38
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Jun 30, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 30, 2020

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

smith
smith approved these changes Jun 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

This looks ok. How do I view the dashboard in the UI at this time?

x-pack/plugins/apm/public/services/rest/observability_dashboard.ts Outdated
),
value: sum(transactionCoordinates.map((coordinates) => coordinates.y)),
value:
mean(transactionCoordinates.map((coordinates) => coordinates.y)) || 0,
Copy link
Copy Markdown
Contributor

@smith smith Jun 30, 2020

Choose a reason for hiding this comment

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

Beware that mean can return NaN if any of the coordinates.y values here are non-numeric.

Copy link
Copy Markdown
Contributor Author

@cauemarcondes cauemarcondes Jul 1, 2020

Choose a reason for hiding this comment

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

Thanks for the heads up @smith , I added a condition to check if the array is not empty and to return 0 in case corrdinate.y is undefined. ffedc7f#diff-cf9933de7395c67ff230849c1c748111R50-R54

Copy link
Copy Markdown
Contributor

@smith smith Jul 1, 2020

Choose a reason for hiding this comment

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

That will return 0 if any y value is non-numeric, which is fine if that's the behavior you want. When I ran into this I did values.map(value => value.y).filter(y => isFinite(y)).

(I can't remember if the data I was looking at had null or NaN or undefined, but let's say null for now) The first method makes it 0 if any null are found and the second gives you the mean of the the non-null values. I suppose you could also do values.map(value => value.y ?? 0), which would convert all the null values to zeroes and give you the mean of that.

sorenlouv
sorenlouv reviewed Jul 1, 2020
View reviewed changes
x-pack/plugins/apm/public/services/rest/observability_dashboard.ts
mean(
transactionCoordinates
.map(({ y }) => y)
.filter((y) => y && isFinite(y))
Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv Jul 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

nit: I think you can do:

Suggested change
.filter((y) => y && isFinite(y))
.filter(isFinite)

Copy link
Copy Markdown
Contributor Author

@cauemarcondes cauemarcondes Jul 1, 2020

Choose a reason for hiding this comment

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

isFinite requires a number, and y can be number | undefined. That's why I didn't do what you've suggested.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jul 2, 2020

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@cauemarcondes cauemarcondes merged commit 6aeda64 into elastic:master Jul 2, 2020
@cauemarcondes cauemarcondes deleted the apm-transaction-per-min branch July 2, 2020 09:01
@cauemarcondes cauemarcondes mentioned this pull request Jul 2, 2020
Merged
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Jul 2, 2020
@cauemarcondes
[APM] Show transaction rate per minute on Observability Overview page (…
49a0de5 
…elastic#70336)

* changing transaction count to transaction rate per second

* sanity check coordinates before calculate the mean

* sanity check coordinates before calculate the mean

* removing extend_bounds to return empty when no data is available
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 2, 2020
@gmmorris
Merge branch 'master' into alerting/built-in-alerts-feature
0e001e6 
* master: (46 commits)
  [Visualize] Add missing advanced settings and custom label for pipeline aggs (elastic#69688)
  Use dynamic: false for config saved object mappings (elastic#70436)
  [Ingest Pipelines] Error messages (elastic#70167)
  [APM] Show transaction rate per minute on Observability Overview page (elastic#70336)
  Filter out error when calculating a label (elastic#69934)
  [Visualizations] Each visType returns its supported triggers (elastic#70177)
  [Telemetry] Report data shippers (elastic#64935)
  Reduce SavedObjects mappings for Application Usage (elastic#70475)
  [Lens] fix dimension label performance issues (elastic#69978)
  Skip failing endgame tests (elastic#70548)
  [SIEM] Reenabling Cypress tests (elastic#70397)
  [SIEM][Security Solution][Endpoint] Endpoint Artifact Manifest Management + Artifact Download and Distribution (elastic#67707)
  [Security] Adds field mapping support to rule creation (elastic#70288)
  SECURITY-ENDPOINT: add fields for events to metadata document (elastic#70491)
  Fixed assertion in hybrid index pattern test to iterate through indices (elastic#70130)
  [SIEM][Exceptions] - Exception builder component (elastic#67013)
  [Ingest Manager] Rename data sources to package configs (elastic#70259)
  skip suites blocking es snapshot promomotion (elastic#70532)
  [Metrics UI] Fix asynchronicity and error handling in Snapshot API (elastic#70503)
  fix export response (elastic#70473)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 6, 2020
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jul 6, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jul 7, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 7, 2020
cauemarcondes added a commit that referenced this pull request Jul 7, 2020
@cauemarcondes @elasticmachine @sorenlouv
[7.x] [APM] Show transaction rate per minute on Observability Overvie…
f56a6c3 
…w page (#70336) (#70566)

* [APM] Show transaction rate per minute on Observability Overview page (#70336)

* changing transaction count to transaction rate per second

* sanity check coordinates before calculate the mean

* sanity check coordinates before calculate the mean

* removing extend_bounds to return empty when no data is available

* Fix lodash

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smith smith smith approved these changes

+1 more reviewer

@sorenlouv sorenlouv sorenlouv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.9.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[APM] Show transaction rate per minute on Observability Overview page

5 participants

@cauemarcondes @elasticmachine @kibanamachine @smith @sorenlouv