elastic / apm

Elastic Application Performance Monitoring - resources and general issue tracking for Elastic APM.
https://www.elastic.co/apm
Apache License 2.0
384 stars 114 forks source link

Add spec for 'span.sync' field #802

Closed SylvainJuge closed 1 year ago

SylvainJuge commented 1 year ago

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.

trentm commented 1 year ago

This was discussed a while back for the Node.js APM agent in https://github.com/elastic/apm-agent-nodejs/issues/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 https://github.com/elastic/kibana/issues/109661 (implemented in https://github.com/elastic/kibana/pull/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 commented 1 year ago

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 c08703dd8ffa06444d5a10c1c22980dae68537ea to add platform-specific behavior.

stevejgordon commented 1 year ago

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.