Closed medikoo closed 1 year ago
This looks good 👍 does this mean that if you have a project built with ES build and you haven’t taken the necessary steps to expose the SDK libraries you won’t see anything?
It's not related to ES build in any way.
We instrument both HTTP requests and AWS SDK requests, yet AWS SDK request underneath issue HTTP requests, and in such case we do not want to expose spans for underlying HTTP requests. This PR improves a method in which we black box AWS SDK request spans, so HTTP request spans that happen underneath are not exposed.
@medikoo But if you use esbuild and your service is not configured correctly to let our SDK know that you are using esbuild the only span you will see are the underlying http requests to the AWS services.
So I am curious if we black box these http request spans does that mean that these traces (with the misconfigured sdk) will have no captured spans or will the http requests not be black boxed because we don't know they are using the AWS SDK?
Here is a sample trace where I am using esbuild and not configuring our sdk manually to know I am using the AWS SDK 👇
Would this trace not include the node.https.request
span?
@Danwakeem, nothing will change in this case.
This PR is an improvement to how we black box AWS SDK spans. Note that we black box them already, but with a primitive method, which in a very rare scenario fails. This patch ensures that in this very rare scenario, it'll work as expected.
HTTP spans are black boxed only in context of AWS SDK spans. if there are no AWS SDK spans, there's no black boxing implied (this logic is initiated by AWS SDK instrumentation) So in case of ESBuild when AWS SDK instrumentation doesn't pick up, obviously HTTP spans will appear as they appear now.
Got it, thanks for the clarification 👍
Internally we have a use case of not exposing sub spans when we're in context of specific trace span.
e.g. when in context of AWS SDK span we do not want to expose underlying HTTP request with extra span. So far we mitigated that with a primitive approach, where we were assuming that HTTP request is initialized in sequence synchronously and we were marking given HTTP request to not be instrumented.
This doesn't work for case where AWS SDK approaches temporary network issue and implies a retry, as then following HTTP request happens async and appears as reported, which is not wanted. Very occasionally we observed integration test failures due to that (e.g. https://github.com/serverless/console/actions/runs/4810414588/jobs/8563030944)
This PR introduces an internal concept of black box trace spans. Where we can mark span as black box and that way ensure no child spans of it will be exposed. Even if some logic will add spans to such black box span (which is not prevented in any way), they will not be exposed in final trace report, or propagated to dev mode
Additionally removed debug logs, which were added to help investigate the issue of HTTP spans being exposed when run in context of AWS SDK request