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] Reparenting spans to support inferred spans #63695

Merged
merged 3 commits into from
Apr 17, 2020

Conversation

cauemarcondes
Copy link
Contributor

closes #63575
closes #62952

Reparents spans based on the child_ids property.
Before:
Screenshot 2020-04-16 at 14 43 09
Loader#executeQueryStatement is an Inferred Span which should be child of OpenTracing product span

After:
Screenshot 2020-04-16 at 14 37 50
Adding the child_ids property on OpenTracing product span and reference it Loader#executeQueryStatement, makes the Waterfall to reparent the Loader#executeQueryStatement.

A question that maybe @graphaelli or @felixbarny might know:

  • Should I increase the duration of the new parent?
    e.g.:
--[new parent     ] 
----[reparented child     ]  

Should we change it to?

--[new parent             ]
----[reparented child     ]  

@felixbarny
Copy link
Member

Is the before screenshot taken from an actual transaction? Or is that a modified one to showcase the representing? I'm asking because I couldn't explain how that situation could have happened, maybe there's a lost activation event for the OpenTracing product span.

Here's another example for what's supposed to happen:
Screen Shot 2020-04-17 at 14 01 55

The SELECT FROM customers span is a child of the transaction. But it's supposed to be a child of the MutableEntityEntryFactory#createEntityEntry inferred span.
In this example, the order happens to not be affected by the incorrect parent/child relationships.

I have set profiling_inferred_spans_sampling_interval=5ms for this.

A question that maybe @graphaelli or @felixbarny might know:

Should I increase the duration of the new parent?

TL;DR: No, as normally the Java agent would take care of that.

I'm confused as to why the APIRestController#product span (the 3rd from above) doesn't exceed the OpenTracing product span. I think that is because the activation event of the latter got lost. Otherwise, the Java agent would have corrected the duration of the APIRestController#product span and the Loader#executeQueryStatement span would be a direct child of the OpenTracing product span.

In an ideal world, the two DB spans are supposed to be re-parented to the Loader#executeQueryStatement span. It seems the duration of that span has been auto-aligned by the Java agent to not start after the empty query span and to not end before the SELECT FROM products span.

@cauemarcondes
Copy link
Contributor Author

Is the before screenshot taken from an actual transaction? Or is that a modified one to showcase the representing?

I got it running apm-integration-test, but since it's not clear for me who should be parent of who I picked any Inferred span, in that case, Loader#executeQueryStatement, and said that it's child of OpenTracing product span.

I also did this because, in my opinion, the UI doesn't have to know about Inferred Spans, it knows how to reparent items based on the child_ids property.

I have set profiling_inferred_spans_sampling_interval=5ms for this.

I'm going to set this variable as 5ms on apm-integration-test and try to find a better example.

@felixbarny
Copy link
Member

I also did this because, in my opinion, the UI doesn't have to know about Inferred Spans, it knows how to reparent items based on the child_ids property.

Agreed. For an initial test, this is fine. But a soon as the APM Server supports representing, we should not forget to do another end-to-end test.

@cauemarcondes
Copy link
Contributor Author

I also did this because, in my opinion, the UI doesn't have to know about Inferred Spans, it knows how to reparent items based on the child_ids property.

Agreed. For an initial test, this is fine. But a soon as the APM Server supports representing, we should not forget to do another end-to-end test.

Agreed. Can you link the APM agent issue here, please?

@felixbarny
Copy link
Member

It's linked from the meta issue elastic/apm#247

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

LGTM!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@cauemarcondes cauemarcondes merged commit 675c589 into elastic:master Apr 17, 2020
@cauemarcondes cauemarcondes deleted the inferred-spans branch April 17, 2020 16:19
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Apr 20, 2020
* reparening spans

* adding unit test

* adding unit test
cauemarcondes added a commit that referenced this pull request Apr 20, 2020
* reparening spans

* adding unit test

* adding unit test
@ogupte ogupte self-assigned this Jun 1, 2020
@ogupte ogupte added the apm:test-plan-done Pull request that was successfully tested during the test plan label Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-7.8.0 apm:test-plan-done Pull request that was successfully tested during the test plan release_note:enhancement v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Reparenting spans to support inferred spans APM Inferred spans
6 participants