-
Notifications
You must be signed in to change notification settings - Fork 265
feat: introduce opentelemetry tracing #7172
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
Conversation
Jenkins BuildsClick to see older builds (103)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7172 +/- ##
============================================
+ Coverage 34.81% 59.85% +25.03%
============================================
Files 797 815 +18
Lines 111054 113558 +2504
============================================
+ Hits 38662 67967 +29305
+ Misses 67502 38713 -28789
- Partials 4890 6878 +1988
Flags with carried forward coverage won't be shown. Click here to find out more.
|
b0adc28 to
1c09af3
Compare
95f9fa0 to
eb110b2
Compare
eb110b2 to
807239d
Compare
|
Seems that this PR includes changes from #7177 ? |
igor-sirotin
left a comment
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.
Noice!
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 this and README should live in /tools/opentelemetry
And internal/instrumentation/trace -> internal/trace
https://github.com/golang-standards/project-layout/tree/master/tools
WDYT?
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.
Hm.. examples given there are standalone tools, makes sense. I believe we should move some of our cmds to tools directory as well, e.g. cmd/generate-db, cmd/library.
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.
Yup, absolutely we should move some 💯
807239d to
936cc6b
Compare
jrainville
left a comment
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.
Very cool!
Just check the comment Igor made about the insta span end. I feel like it might be an oopsie
6def979 to
897c2a2
Compare
Required to derive trace context when a message has been segmented.
897c2a2 to
b47f7d9
Compare
https://github.com/status-im/status-go/blob/1c09af3e026fe4da56d375252eb17e5a69d2fe95/internal/instrumentation/README.md