Add option to not include couchbase internal spans

[brentm5]

What Does This Do

This adds an option to disable couchbase internal traces from being generated.

Motivation

The couchbase internal spans don't provide much more value over the top level couchbase span. By removing these internal spans we reduce the overall amount of spans significantly and provide and a less complex experience.

The feature is opt-in and can be activated by setting:

• Environment var: DD_TRACE_COUCHBASE_INTERNAL_SPANS_ENABLED=false
• System prop: -Ddd.trace.couchbase.internal-spans.enabled=false

Additional Notes

First time contributor and not really familiar with gradle as a build tool.

[PerfectSlayer]

Hi @brentm5

Thanks for you contribution!
First, I can’t tell about the feature itself, but maybe @amarziali can chime in here 👋

About the implementation, I understand the main goal is to disable the CouchbaseRequestTracer.
This class is registered in the Couchbase environment builder using the CoreEnvironmentBuilderAdvice advice, which is added by the CoreEnvironmentBuilderInstrumentation instrumentation.

I would rather use instrumentation alternative names to disable the whole instrumenter.
You can do it by adding another name to the instrumentations name, like couchbase-internal, and then use environment configuration or System properties to disable it using DD_INTEGRATION_COUCHBASE_INTERNAL_ENABLED=true .
This would avoid creating a dedicated config entry and also disable context store instrumentation which will be no more used.

[amarziali]

👋 @brentm5 the PR looks OK. However the original request span should be wrapped again with particular care otherwise it will be finished prematurely. I'll need to add some tests to cover that feature flag and to bring slight changes. I'll add a commit for it

[amarziali]

I would rather use instrumentation alternative names to disable the whole instrumenter. You can do it by adding another name to the instrumentations name, like couchbase-internal, and then use environment configuration or System properties to disable it using DD_INTEGRATION_COUCHBASE_INTERNAL_ENABLED=true .

This would have been the best option but unfortunately we still need that couchbase tracing callback to enrich parent span so it's not sounding possible to me to go this way. The implementation proposed looks good enough to solve that need

[brentm5]

👋 @brentm5 the PR looks OK. However the original request span should be wrapped again with particular care otherwise it will be finished prematurely. I'll need to add some tests to cover that feature flag and to bring slight changes. I'll add a commit for it

Thank you! Looking at what traces were generated it does indeed look like something isn't quite right with this. It looks like it currently ends the trace after the first couchbase.internal call ends. So obviously my approach does not 100% work. Here are two screenshots. (Its worth noting that these traces are on different processes that are running so they won't match up 100%. They do have a similar amount of time within the Custom span Couchbase.SaveAndRetrieve.

This shows the internal couchbase spans enabled (IE current state). With all the time in the custom parent span Couchbase.SaveAndRetrieve being attribute to couchbase.

This shows the internal couchbase spans disabled. With this the couchbase spans appear to be shorter and mimic the time in the first couchbase.internal span thats reported.

[amarziali]

👋 @brentm5 the PR looks OK. However the original request span should be wrapped again with particular care otherwise it will be finished prematurely. I'll need to add some tests to cover that feature flag and to bring slight changes. I'll add a commit for it

Thank you! Looking at what traces were generated it does indeed look like something isn't quite right with this. It looks like it currently ends the trace after the first couchbase.internal call ends. So obviously my approach does not 100% work.

@brentm5 thanks for the detailed throubleshoot. Yes as previously told it was expected since you passed the first request span when the internal span was called hence couchbase was triggering an endspan on the parent when it was related to the child (internal).
I added a commit that is correcting a bit the way the first implementation was proposed. It also add tests for couchbase 3.1 and 3.2+. I also changed the wording of the feature flag (now is dd.trace.couchbase.internal-spans.enabled)

Could you please try with this version if you can? If now the behaviour is closer to the expected, please feel free to undraft that PR and I'll go on with the internal review.
Cheers

Andrea

[brentm5]

Thank you so much for making this work. I ran this in my test environment and things look correct.

I did default this to true for backwards compatible reasons but it might make sense to have this default to false down the road. For my organization there have been maybe 1 or 2 instances where someone used these additional spans but not sure if that is true for others.