Add spec for 'span.sync' field

[SylvainJuge]

Adds a small spec for span.sync field.

This is more a "specification of how it works currently" rather than "specification of how it should work", so feel free to provide input if any agent implementation does not fit what is described here.

• Create PR as draft
• Approval by at least one other agent
• Mark as Ready for Review (automatically requests reviews from all agents and PM via CODEOWNERS)
  • Remove PM from reviewers if impact on product is negligible
  • Remove agents from reviewers if the change is not relevant for them
• Merge after 2 business days passed without objections
  To auto-merge the PR, add /schedule YYYY-MM-DD to the PR description.

[SylvainJuge]

Maybe here we should clarify that the agents should only be able to capture the (a)synchronous "nature" of a specific span, for example when a synchronous or asynchronous API is used.

It might sound obvious, but if an asynchronous span is (mis)used in a synchronous way this can't easily be detected by agents at runtime, so from the user perspective it's only in the "shape" of the trace and the relative position of the blocking or async spans that we can infer if that's an issue or not.

Few examples:

• an asynchronous API is used in a blocking way: having the async span making the parent span/transaction longer indicates that refactoring code could help.
• starting multiple async tasks from a single span/transaction and then awaiting their completion in a blocking way would not be considered a problem because the async tasks are started in parallel. However, doing the same with a single span would make it fall in the "async API used in a blocking way" described above.

[trentm]

This was discussed a while back for the Node.js APM agent in elastic/apm-agent-nodejs#1881
In that issue the Node.js APM agent considered changing to not set span.sync=false so that the APM UI would not show "async" badges in the waterfall for most spans -- because async is by far the common case in Node.js-land.

A result of that discussion was elastic/kibana#109661 (implemented in elastic/kibana#123038) where the APM UI was changed to only show a badge for the "non-typical" case for a given language. This shipped in 8.1.
Currently the "non-typical" case is defined here:
https://github.com/elastic/kibana/blob/291f3994bf776428f42673084a46c6429dac38ab/x-pack/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/badge/sync_badge.tsx#L35-L48

So, I think the current "In UI" block is incorrect.

[SylvainJuge]

This was discussed a while back for the Node.js APM agent in elastic/apm-agent-nodejs#1881 In that issue the Node.js APM agent considered changing to not set span.sync=false so that the APM UI would not show "async" badges in the waterfall for most spans -- because async is by far the common case in Node.js-land.

A result of that discussion was elastic/kibana#109661 (implemented in elastic/kibana#123038) where the APM UI was changed to only show a badge for the "non-typical" case for a given language. This shipped in 8.1. Currently the "non-typical" case is defined here: https://github.com/elastic/kibana/blob/291f3994bf776428f42673084a46c6429dac38ab/x-pack/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/badge/sync_badge.tsx#L35-L48

So, I think the current "In UI" block is incorrect.

yes, you are correct, I've updated the UI part in c08703d to add platform-specific behavior.

[stevejgordon]

LGTM

[stevejgordon]

We don't currently have any implementation for this field in the .NET agent today. We'd need to research how practical it would be to determine this in our instrumentation, especially outside our profiler-based collections. I'll open an issue for us to track researching this further for the .NET client.