-
Notifications
You must be signed in to change notification settings - Fork 2.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
Replace Jaeger SDK with OTEL SDK + OT Bridge #4574
Replace Jaeger SDK with OTEL SDK + OT Bridge #4574
Conversation
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4574 +/- ##
==========================================
+ Coverage 97.03% 97.07% +0.03%
==========================================
Files 301 301
Lines 17839 17857 +18
==========================================
+ Hits 17310 17334 +24
+ Misses 424 419 -5
+ Partials 105 104 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal <[email protected]>
Signed-off-by: Afzal <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
1f8f7f2
to
d429861
Compare
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
OTEL tracerProvider
to jtracer pkg
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
tracerCloser := initTracer(svc) | ||
tracer, err := jtracer.New("jaeger-all-in-one") | ||
if err != nil { | ||
logger.Fatal("Failed to initialize tracer", zap.Error(err)) |
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.
Hmmm... I wonder why we don't annotate and return the error here instead of logging it.
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.
I think mostly because if we return an error, it is intercepted at the top level of main like this:
if err := command.Execute(); err != nil {
log.Fatal(err)
}
Here the zapp logger is not available so the stdlib log
is used, which is formatted differently. E.g.:
$ go run ./cmd/all-in-one --admin.http.tls.enabled=true --admin.http.tls.key=invalid
. . .
2023/07/20 11:43:08 cannot initialize admin server: failed to watch key pair invalid and : open invalid: no such file or directory
exit status 1
With zapp logs you get line number and other nice things. E.g.
$ go run ./cmd/all-in-one --reporter.grpc.tls.enabled=true --reporter.grpc.tls.key=nopath
. . .
{"level":"fatal","ts":1689868046.063343,"caller":"all-in-one/main.go:192","msg":"Could not create collector proxy","error":"failed to load TLS config: failed to watch key pair nopath and : open nopath: no such file or directory","stacktrace":"main.main.func1\n\t/Users/ysh/dev/jaegertracing/jaeger/cmd/all-in-one/main.go:192\ngithub.com/spf13/cobra.(*Command).execute\n\t/Users/ysh/golang/pkg/mod/github.com/spf13/[email protected]/command.go:940\ngithub.com/spf13/cobra.(*Command).ExecuteC\n\t/Users/ysh/golang/pkg/mod/github.com/spf13/[email protected]/command.go:1068\ngithub.com/spf13/cobra.(*Command).Execute\n\t/Users/ysh/golang/pkg/mod/github.com/spf13/[email protected]/command.go:992\nmain.main\n\t/Users/ysh/dev/jaegertracing/jaeger/cmd/all-in-one/main.go:243\nruntime.main\n\t/Users/ysh/homebrew/Cellar/go/1.20.5/libexec/src/runtime/proc.go:250"}
exit status 1
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.
That's cool thing I noticed here. 🏷️
Is it a good time to remove the [WIP] from the PR title? |
Signed-off-by: Afzal Ansari <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>
Which problem is this PR solving?
Short description of the changes
otel tracer