fix: ensure correct run context for mysql2 instrumentation

[trentm]

This fixes the 'mysql2' instrumentation to not have the mysql span
context be active in user code. This ensures that user code cannot
create a child span of the mysql span, which would (a) be misleading
and (b) cause problems for coming exit span and compressed span work.

Refs: #2430

(This is very similar to the equivalent change for the 'mysql' package instrumentation: #2463)

Checklist

• Implement code
• Add tests
• Add CHANGELOG.asciidoc entry

[trentm]

review notes

[trentm]

REVIEW NOTE: This change to return early if !enabled -- as is done in most instrumentations -- is a similar issue as was discussed for the recent ioredis change here: #2460 (comment)

Before this change the mysql2 instrumentation would still do some work, even if disableInstrumentations=mysql2,.... This is presumably one of the "continuation patches" mentioned at #353 (comment) without further details.

My understanding of the intent is to avoid a user's callback to a mysql2.query(..., cb) call from being attached to the run context of unrelated code due to the mysql2 package's internal callback queuing (which it uses to serialize queries to MySQL) when:

1. the user is using the mysql2 package, and
2. configured with disableInstrumentations=mysql2,...

I imagine this is a rare configuration. Also, it is slightly odd that "disableInstrumentations=mysql2" doesn't turn off that code path completely. I don't believe there are any test cases that cover this.

@astorm What do you think: Is this change a blocker/breaking change or could it be deferred to a separate issue?

[astorm]

@astorm I don't think there's an easy answer to that, and as a result I have no strong opinions. My own bias would tend to be conservative and not change the behavior of long standing code legacy-ish code, but if you think there's value in fixing this slight oddity then I don't object.

[trentm]

@astorm Thanks. I've played with this a little bit more and have some cases to show. I think it is subtle enough and debatable enough that I'd like a separate ticket to hold the discussion. I'll give that issue some time before merging this one, in case we decide we want to attempt to somewhat persist the old edge-case behaviour.

I'll link the ticket back here when I have it. I started writing it up and it got long.

[trentm]

#2498

[apmmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2021-12-01T22:47:37.794+0000

• Duration: 20 min 43 sec

• Commit: 8e364f2

Test stats 🧪

Test	Results
Failed	0
Passed	22
Skipped	0
Total	22

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

• run benchmark tests : Run the benchmark test only.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[astorm]

👍 approving, does the thing.