-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding WithCustomTag
to a few libs in contrib/
: gorm/gorm.v1
, confluentinc/confluent-kafka-go/kafka
, database/sql
, google.golang.org/grpc
#1309
Conversation
support custom tags for gorm.v1 instrumentation Fixes DataDog#1308
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi duxing! Thanks for this contribution, it looks like a good idea to bring both these versions to feature parity. Just one small comment I've attached, and also can you add a unit test for this change? You may be able to borrow nearly all the test from the previous version which has a test TestCustomTags
that covers this.
Thanks again!
support custom tags for confluent-kafka-go/kafka instrumentation
support custom tags for confluent-kafka-go/kafka instrumentation
support custom tags for google.golang.org/grpc instrumentation
WithCustomTag
to a few libs in contrib/
: gorm.io/gorm.v1
, confluentinc/confluent-kafka-go/kafka
, database/sql
, google.golang.org/grpc
WithCustomTag
to a few libs in contrib/
: gorm.io/gorm.v1
, confluentinc/confluent-kafka-go/kafka
, database/sql
, google.golang.org/grpc
WithCustomTag
to a few libs in contrib/
: jinzhu/gorm
, confluentinc/confluent-kafka-go/kafka
, database/sql
, google.golang.org/grpc
WithCustomTag
to a few libs in contrib/
: jinzhu/gorm
, confluentinc/confluent-kafka-go/kafka
, database/sql
, google.golang.org/grpc
WithCustomTag
to a few libs in contrib/
: gorm/gorm.v1
, confluentinc/confluent-kafka-go/kafka
, database/sql
, google.golang.org/grpc
hi @ajgajg1134 sorry about the delay. I was able to get to this last night and added test coverage for everything I introduced. can you take another look when you have a chance? I noticed the CI fails complaining about |
hi @ajgajg1134 can you help me understand why my PR fails due to:
thanks in advance! |
Hello @duxing! Sorry, the system-test failures you're seeing are due to this PR running a branch from an external fork of the repository. Unfortunately it means these tests can't be run unless the branch is coming from within the repository, although we can run the other tests still and should be able to merge after review is complete. (This is a limitation of github actions that hopefully is resolved at some point) |
@ajgajg1134 thx for explaining, i figured it has something to do with the github action settings. I've disabled my own fork since it's complaining about the same thing. is it possible to cherry-pick my change to a branch for this repo and proceed? |
Closing this PR since the work was merged with #1359 |
support custom tags for a few libs in
contrib/
:gorm.io/gorm.v1
confluentinc/confluent-kafka-go/kafka
database/sql
google.golang.org/grpc
Fixes #1308