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] Use oldest exit span instead of newest #113133

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

dgieselaar
Copy link
Member

For discovering connections, select the oldest span instead of the newest. This increases the possiblity of the relevant transaction being indexed into Elasticsearch at query time. If this tranasaction isn't there, exit spans might not be appropriately resolved to the downstream service, as reported by @felixbarny and @LucaWintergerst.

For discovering connections, select the oldest span instead of the newest. This increases the possiblity of the relevant transaction being indexed into Elasticsearch at query time.
@dgieselaar dgieselaar added release_note:fix Team:APM All issues that need APM UI Team support v7.16.0 v7.15.1 labels Sep 27, 2021
@dgieselaar dgieselaar requested a review from a team as a code owner September 27, 2021 15:30
@elasticmachine
Copy link
Contributor

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

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix. Do we need to update any tests for this (and if not, is that ok?)

@dgieselaar
Copy link
Member Author

@sqren:

Thanks for the quick fix. Do we need to update any tests for this (and if not, is that ok?)

This issue exhibits itself only when there are indexing delays (between the exit span and entry transaction). So it's quite hard to replicate with an Elasticsearch archive, e.g. you can't use a specific time range. My feeling is that it's not worth the overhead of a (very specific) API test, what do you think?

@sorenlouv
Copy link
Member

My feeling is that it's not worth the overhead of a (very specific) API test, what do you think?

Makes sense 👍

@dgieselaar dgieselaar merged commit 165cafd into elastic:master Sep 28, 2021
@dgieselaar dgieselaar deleted the oldest-connection-sample branch September 28, 2021 06:20
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113133 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 30, 2021
dgieselaar added a commit to dgieselaar/kibana that referenced this pull request Sep 30, 2021
dgieselaar added a commit to dgieselaar/kibana that referenced this pull request Sep 30, 2021
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team:APM All issues that need APM UI Team support v7.15.1 v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants