feat: add support for github.com/confluentinc/confluent-kafka-go/kafka (& its v2 counterpart) #273
Conversation
| if err != nil { | ||
| t.Skipf("Failed to start kafka test container: %v\n", err) | ||
| } |
There was a problem hiding this comment.
| if err != nil { | |
| t.Skipf("Failed to start kafka test container: %v\n", err) | |
| } | |
| utils.AssertTestContainersError(t, err) |
this function already exists to check whether the error should mark the test as skipped or not
| if err != nil { | ||
| t.Skipf("Failed to start kafka test container: %v\n", err) | ||
| } |
There was a problem hiding this comment.
| if err != nil { | |
| t.Skipf("Failed to start kafka test container: %v\n", err) | |
| } | |
| utils.AssertTestContainersError(t, err) |
same here
| kafkatrace: gopkg.in/DataDog/dd-trace-go.v1/contrib/confluentinc/confluent-kafka-go/kafka.v2 | ||
| kafka: github.com/confluentinc/confluent-kafka-go/v2/kafka | ||
| template: |- | ||
| func(p *kafka.Producer, err error) (*kafkatrace.Producer, error) { |
There was a problem hiding this comment.
I don't think this is a safe replacement to do, as we originally have a *kafka.Producer and we return a *kafkatrace.Producer
if the user has code like this, it would break:
var (
producer *kafka.Producer
err error
)
producer, err = kafka.NewProducer() // replacement happens here
Also, if there are any type assertions in the users code, these would break as well.
I think the only safe approach here is to instrument the library code itself, similar as I did here: #266
cc @RomainMuller @eliottness please confirm if this is correct 🙇
There was a problem hiding this comment.
I can confirm. There is the same issue with gorilla/mux
There was a problem hiding this comment.
Correct - Actually, assignment to a struct field is more likely to be where the problem is than an explicitly typed var, but same thing really.
Change the behavior of the code generation from `_integration-tests` to generate individual test files in each test package. This speeds up significantly when running only one test suite, which is particularly useful when developing a new integration. Also, this change is required for #273, where we are testing both `confluent-kafka-go` v1 and v2. Since both of them depend on CGO `librdkafka`, they cannot coexist in the same package.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #273 +/- ##
==========================================
- Coverage 78.53% 71.25% -7.28%
==========================================
Files 140 144 +4
Lines 6359 7780 +1421
==========================================
+ Hits 4994 5544 +550
- Misses 955 1828 +873
+ Partials 410 408 -2
|
Adds support for instrumenting
github.com/confluentinc/confluent-kafka-go/kafkaandgithub.com/confluentinc/confluent-kafka-go/v2/kafka.Tests for v2 library are commented because both libraries cannot coexist in the same package. An upcoming PR from @rarguelloF will separate the tests in a way that will keep them apart, so we'll be able to run both test suites. The v2 tests have been tested locally successfully.