This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
trace: several tracing improvements #9153
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
keegancsmith
requested review from
beyang,
ryan-blunden,
slimsag and
a team
as code owners
keegancsmith
force-pushed
the
core/trace-lazy-log
branch
from
0db4de4
to
b3cb5dd
Compare
unknwon
approved these changes
Mar 19, 2020
tsenart
approved these changes
Mar 19, 2020
keegancsmith
force-pushed
the
core/trace-lazy-log
branch
from
c71e47d
to
ed7dd02
Compare
Codecov Report
@@ Coverage Diff @@
## master #9153 +/- ##
==========================================
+ Coverage 41.51% 41.52% +<.01%
==========================================
Files 1312 1312
Lines 71393 71400 +7
Branches 6581 6581
==========================================
+ Hits 29642 29650 +8
+ Misses 39018 39017 -1
Partials 2733 2733
|
LazyLog wouldn't log to opentracing. So we remove it and add a LazyLogger for fmt.Stringers such that we can update call sites of LazyLog.
LazyPrintf is used quite extensively in our codebase. I wanted to update all to LogFields, but it is too many callsites. Instead I have made LazyPrintf call LogFields, but still preserve the lazy behaviour. Follow-up commits will clean up call sites of LazyPrintf.
One callsite updated to use LogFields instead. I wanted to update lots, but stopped when I realised how much effort that would be. Going forward the updates will be much more targetted.
This adds a new opentracing Field which emits much nicer SQL logs.
I found this quite convenient to just click on, instead of assuming the admin knows to check localhost.
Only logged to opentracing currently. This will allow us to also log to net/trace.
go lint now knows to check the printf args which is cool, so it picked up this misuse.
otlog.Field.String() just marshals the function for Lazy fields, instead of actually calling the function to emit its values. This lead to the net/trace logs to just log the function pointer numbers, instead of the values you want.
keegancsmith
force-pushed
the
core/trace-lazy-log
branch
from
ed7dd02
to
26069d5
Compare
keegancsmith
added a commit
that referenced
this pull request
Mar 24, 2020
* trace: replace LazyLog with LogFields LazyLog wouldn't log to opentracing. So we remove it and add a LazyLogger for fmt.Stringers such that we can update call sites of LazyLog. * trace: LazyPrintf logs to opentracing LazyPrintf is used quite extensively in our codebase. I wanted to update all to LogFields, but it is too many callsites. Instead I have made LazyPrintf call LogFields, but still preserve the lazy behaviour. Follow-up commits will clean up call sites of LazyPrintf. * search: use LogFields for searchSymbols One callsite updated to use LogFields instead. I wanted to update lots, but stopped when I realised how much effort that would be. Going forward the updates will be much more targetted. * trace: add SQL for better lazy logging This adds a new opentracing Field which emits much nicer SQL logs. * doc: mention jaeger URL when port forwarding I found this quite convenient to just click on, instead of assuming the admin knows to check localhost. * db: use internal/trace for sql tracing Only logged to opentracing currently. This will allow us to also log to net/trace. * fix lint warning go lint now knows to check the printf args which is cool, so it picked up this misuse. * trace: include sql query in title * trace: correctly marshal lazy otlog.Fields otlog.Field.String() just marshals the function for Lazy fields, instead of actually calling the function to emit its values. This lead to the net/trace logs to just log the function pointer numbers, instead of the values you want.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits.