-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Change instrumentation to use otel-go for trace #1833
Conversation
Signed-off-by: Bogdan Drutu <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1833 +/- ##
==========================================
- Coverage 91.11% 91.07% -0.04%
==========================================
Files 263 263
Lines 16109 16139 +30
==========================================
+ Hits 14677 14698 +21
- Misses 1004 1015 +11
+ Partials 428 426 -2
Continue to review full report at Codecov.
|
tracer := tProvider.Tracer("grpc") | ||
opts := append(extraOpts, | ||
grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor(tracer)), | ||
grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor(tracer))) |
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.
Add comments to explains what does this do.
@@ -45,7 +48,8 @@ const ( | |||
) | |||
|
|||
var ( | |||
tagKeyExporter, _ = tag.NewKey(ExporterKey) | |||
defaultExportTracer = global.Tracer("") |
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.
nit: this is not consistent with the exporter helper that uses "go.opentelemetry.io/collector/exporter"
@@ -53,6 +56,7 @@ func New(instanceName string, nextConsumer consumer.TraceConsumer, opts ...Optio | |||
ocr := &Receiver{ | |||
nextConsumer: nextConsumer, | |||
instanceName: instanceName, | |||
tracer: global.Tracer("go.opentelemetry.io/collector/receiver/opencensus"), |
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.
At first it seems that the "opencensus" suffix is not consistent with the tracer for other receivers and components, ie.: they will be under "exporter", "receiver", and so on.
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
* add sqlquery receiver from contrib * pin testcontainers to 0.11.1 Co-authored-by: Ryan Fitzpatrick <[email protected]>
Some "breaking" changes: