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] Sort by score to get transaction samples with sampled:true #26389

Merged
merged 3 commits into from
Dec 3, 2018

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Nov 28, 2018

Follow up to #26266.

The previous PR was supposed to have fix the issue with buckets not being highlighted.
On the Traces overview, transaction samples were sorted by date (instead of by score) so the top sample wasn't necessarily sampled.
Now transactions samples are sorted by score, and I have confirmed it to fix the problem, by manually testing with cloud data.

@sorenlouv sorenlouv changed the title [APM] Sort by score to get buckets with sampled:true [APM] Sort by score to get transaction samples with sampled:true Nov 28, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sorenlouv sorenlouv merged commit 47c163b into elastic:master Dec 3, 2018
@sorenlouv sorenlouv deleted the fix-sorting-top-hits branch December 3, 2018 14:23
sorenlouv added a commit that referenced this pull request Dec 4, 2018
… | [APM] Sort by score to get transaction samples with sampled:true (#26389) | [APM] Move impact calculation to Elasticsearch (#26436) (#26581)

* [APM] Split API into transactions and transaction_groups (#26349)

* [APM] Sort by score to get transaction samples with sampled:true (#26389)

* [APM] Move impact calculation to Elasticsearch (#26436)

* [APM] Move impact calculation to Elasticsearch

* Renamed “durationSum” to “sum” and went back to single, mutable impact
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.

2 participants