Fix instrumentation for Kafka Streams 2.6+

[tylerbenson]

The method signature of what we're instrumenting changed in 2.6 from process() to process(long). By removing the no arg constraint, we now instrument either method.

I've also extended the test to verify the latest version of the library. In order to do so, I had to split the tests to resolve a compilation issue.

Inspired by open-telemetry/opentelemetry-java-instrumentation#3438. Thanks @GuillaumeWaignier!

[richardstartin]

What do the checkpoint timelines look like?

[tylerbenson]

Here's output from running locally. Not quite sure how to interpret it though:

Activity checkpoints by thread ordered by time
Test worker:                                                                                                   |-startSpan/1-|-suspend/1-|----------|-----------|-------------|-----------|-------------|-------------|-----------|-----------|----------|-----------|----------|-----------|-------------|-----------|
kafka-producer-network-thread | producer-1:                                                                    |-------------|-----------|-resume/1-|-endSpan/1-|-------------|-----------|-------------|-------------|-----------|-----------|----------|-----------|----------|-----------|-------------|-----------|
test-application-278e57c6-a8cf-4522-8406-0cb56bebd042-StreamThread-1:                                          |-------------|-----------|----------|-----------|-startSpan/2-|-endSpan/2-|-startSpan/3-|-startSpan/4-|-suspend/4-|-endSpan/3-|----------|-----------|----------|-----------|-------------|-----------|
kafka-producer-network-thread | test-application-278e57c6-a8cf-4522-8406-0cb56bebd042-StreamThread-1-producer: |-------------|-----------|----------|-----------|-------------|-----------|-------------|-------------|-----------|-----------|-resume/4-|-endSpan/4-|-resume/3-|-endTask/3-|-------------|-----------|
-C-1:                                                                                                          |-------------|-----------|----------|-----------|-------------|-----------|-------------|-------------|-----------|-----------|----------|-----------|----------|-----------|-startSpan/5-|-endSpan/5-|

[richardstartin]

Span 3 should have been suspended on test-application-278e57c6-a8cf-4522-8406-0cb56bebd042-StreamThread-1 before being resumed on kafka-producer-network-thread | test-application-278e57c6-a8cf-4522-8406-0cb56bebd042-StreamThread-1-producer but this isn't a new problem.

[dougqh]

Just a question? Is this how we want to be handling this?

I'm concerned this opens us to overmatching if process is overloaded at a later time.
I think we'd be better off having a match for each signature that we want to instrument.

I'm interested to hear other opinions. I'm also not going to block this PR for this reason alone.

[mcculls]

There hasn't been much change to this method - previously it took no-arguments, now it takes a single long. Looking at the class and how this method is used I think it's unlikely that this method will be overloaded, but of course not impossible.

In other instrumentations we've generally gone for looser method matching if it's an internal method of an implementation class. If the advice was simple then I wouldn't be concerned - but this is one case where we're grabbing activeSpan / activeScope and trusting it is the right thing to finish/close. So if we did ever match overloaded (or 'bridge' methods) then this would risk closing both the scope we wanted to close as well as any surrounding scope...

So I would be happier to see more of an explicit match here - or alternatively an extra check in StopSpanAdvice that we're closing the right span/scope, even just a check of the type (note there is work planned to address this separately by adding methods to the scope manager, but that work still needs to be scheduled)

[tylerbenson]

Would you prefer if I changed it to an or that checks for either number of args?

[mcculls]

I'm ok with that since we're going to fix the StopSpanAdvice soon

[tylerbenson]

Ok, I've updated the matcher.