Add franz-go #3113
Add franz-go #3113
Conversation
There was a problem hiding this comment.
Thank you for the contribution! I have left a few comments and questions. Let's also add some more tests in franz_test.go for some more coverage. We'd like to test the functions you created in option.go and franz.go, especially the more complex functions like Produce() or PollFetches().
|
|
||
| func init() { | ||
| telemetry.LoadIntegration(componentName) | ||
| tracer.MarkIntegrationImported("github.com/twmb/franz-go") |
There was a problem hiding this comment.
This should also come with a matching line in ddtrace/tracer/option.go so that it can be properly marked as imported.
| carrier := NewRecordHeadersCarrier(r.Headers) | ||
| if err := tracer.Inject(span.Context(), carrier); err != nil { | ||
| // We still want to continue even if injection failed | ||
| span.SetTag("error.msg", err.Error()) |
There was a problem hiding this comment.
If you would only like to save the error message, you can use ext.ErrorMsg as the key. But why not use span.SetTag(ext.Error, err)? Setting ext.Error will also set ext.ErrorMsg and other things. Using ext.Error is also typically our preferred method as well.
| } | ||
| carrier := NewRecordHeadersCarrier(headers) | ||
|
|
||
| // Test ForeachKey |
There was a problem hiding this comment.
For testing individual scenarios, use t.Run() to separate them out. This will make it easier to pinpoint what is going wrong if one of them were to fail.
| } = (*RecordHeadersCarrier)(nil) | ||
|
|
||
| // ForeachKey iterates over every header. | ||
| func (c RecordHeadersCarrier) ForeachKey(handler func(key, val string) error) error { |
There was a problem hiding this comment.
Any reason why ForeachKey is called on RecordHeadersCarrier instead of a pointer to it (*RecordHeadersCarrier)?
jontonsoup
force-pushed
the
add-franz-go
branch
from
2b7cb9e to
b6d4ec6
Compare
What does this PR do?
Motivation
Reviewer's Checklist
v2-devbranch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!