1
0
mirror of https://github.com/golang/go synced 2024-11-18 14:34:39 -07:00
Commit Graph

74 Commits

Author SHA1 Message Date
Ian Cottrell
c9942794f0 internal/telemetry: render trace tags using typed keys
Type switch on the key and use it to get the value and decide how
to render it.
Use the same render for all debug tag printing.

Change-Id: Ia305fded7dcf05b57c5805f48bb5c22fa7def71f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225380
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:46:26 +00:00
Ian Cottrell
1386b938c6 internal/telemetry: convert attributes using the key type
This uses the strongly typed key to get the value rather than
type switching on the value itself.

Change-Id: I8f72d1d9cac0191b0565c14d8a1108459ee6df36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225379
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:46:10 +00:00
Ian Cottrell
1249273038 internal/telemetry: make metrics take a strongly typed key
Now that keys are solidly typed, we can use them for the metrics.
This prevents accidentally using the wrong type of key, and
allows us to use the typed accesorrs rather than the raw value.

Change-Id: I553bd8e12128d3f00a3e926dbd3bfd420cd3f135
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225378
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:58 +00:00
Ian Cottrell
063d392fe0 internal/telemetry: normalize the event reciever names to all use ev
Change-Id: Ie622d8d5c4f1c7c272400e549981cd182fe0ab67
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225377
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:47 +00:00
Ian Cottrell
8f75b710dd internal/telemetry: improve the telemetry benchmark
This changes the benchmarks to be less heavy weight.
It allows the baseline to have no allocations and be
very lightweight. This makes it easier to see
realistic numbers for the costs of the various
operations, making it easier to track and optimize.

Change-Id: I07699881d6976c3ab3432373f057b563752509b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225337
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:41 +00:00
Ian Cottrell
a49f79bcc2 internal/telemetry: hide event.Type
Change-Id: I390102bbffaa242051cc131ef0659a6544aa89c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224938
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-25 01:02:19 +00:00
Ian Cottrell
f207553f3c internal/telemetry: remove the ProcessEvent function
There is no reason for it to be public now, it is no longer possible to build an
event
without using the helpers.
This also allows us to delay setting the time until after the nil exporter
check, for
a significant time saving in the no exporter case.

name                old time/op    new time/op    delta
/Baseline-8            588ns ± 4%     575ns ± 4%   -2.24%  (p=0.000 n=18+18)
/StdLog-8             9.61µs ± 3%    9.58µs ± 3%     ~     (p=0.384 n=18+18)
/LogNoExporter-8      3.94µs ± 2%    2.26µs ± 2%  -42.50%  (p=0.000 n=17+18)
/TraceNoExporter-8    2.77µs ± 4%    1.11µs ± 2%  -59.82%  (p=0.000 n=18+18)
/StatsNoExporter-8    3.83µs ± 5%    2.15µs ± 3%  -43.70%  (p=0.000 n=18+17)
/Log-8                23.3µs ± 6%    23.0µs ± 1%     ~     (p=0.245 n=18+17)
/Trace-8              26.4µs ± 3%    26.5µs ± 4%     ~     (p=0.269 n=18+17)
/Stats-8              5.36µs ± 2%    5.45µs ± 3%   +1.68%  (p=0.000 n=17+18)

Change-Id: Ibde0e20eaf99d03f786cd1436f05eab7b2a17b20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224657
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-25 01:01:39 +00:00
Ian Cottrell
224c947ce5 internal/telemetry: replace TagSet with TagMap and TagPointer
This separates the concerns of tag collections that have to be iterated
and tag collections that need lookup by key.
Also make it so that events just carry a plain slice of tags.
We pass a TagMap down through the exporters and allow it to be
extended on the way.
We no longer need the event.Query method (or the event type)
We now exclusivley use Key as the identity, and no longer have a
common core implementation but just implement it directly in each
type.
This removes some confusion that was causing the same key through
different paths to end up with a different identity.

Change-Id: I61e47adcb397f4ca83dd90342b021dd8e9571ed3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-25 01:00:44 +00:00
Ian Cottrell
62abcc1da2 internal/telemetry: change Exporter to be a function type.
Change-Id: Id410da20310baf4da6875de08e4449c7a6fb0250
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224277
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-25 00:42:15 +00:00
Nathan Dias
5c746ccfa2 internal/telemetry/export/ocagent: add traces to tutorial
This change updates the metrics tutorial to include example code for
exporting traces from go tools.

Change-Id: Ie1d3c373ed4308ef0160f6389b74c642b348bed6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225061
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-24 05:36:59 +00:00
Nathan Dias
dbf25ea225 internal/telemetry/export/ocagent: update metrics tutorial to use the event system
This change updates the metrics tutorial to be compatible with the new event system.

Change-Id: I8b75f6b02d241bc9dede01cfac167a982f1df67c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225058
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-24 03:55:26 +00:00
Ian Cottrell
8dcfad9e01 internal/telemetry: switch metrics to use the event system
Change-Id: If036530ffe47e7df925bcf6592f664d6020a3d65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223997
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:44:30 +00:00
Ian Cottrell
326edff2a4 internal/telemetry: switch metrics to use only the public API
This also modifies the test data based on a comment in
https://go-review.googlesource.com/c/tools/+/222849

Change-Id: Ib0db60846566b40408b12f84240b58356065d319
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223928
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-23 14:44:17 +00:00
Ian Cottrell
b378960d5b internal/telemetry: replace event.TagList with event.TagSet
This allows us to hide the implementation details of how tags are stored on a
context from the normal interface, to allow us to explore more efficient
mechanisms.
The current storage is not intended as the most efficient choice, this cl is
about isolating the API so we can experiment with benchmarks in the future.

Change-Id: Ib101416bccd8ecdee269cee636b1564d51e1da8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222854
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:43:48 +00:00
Nathan Dias
268ba720d3 internal/telemetry/export/ocagent: update metric tutorial to use oragent
This change updates the metric exporting tutorial to use oragent to spin
up OpenCensus, Prometheus, and Zipkin all at once using docker-compose
rather than manually setting each one up. This allows developers to set
up an environment for testing metrics and traces with minimal effort.

While oragent also spins up Zipkin for traces, the tutorial does not yet
have a section outlining how to export traces from Go tools. A section
for traces will added in a later CL.

Change-Id: I07f49977f7ab49995853ff8ee8eb6ccdf6ef1642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224258
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-21 01:49:04 +00:00
Ian Cottrell
540150da73 internal/telemetry: add type safe tag keys
This changes the way keys work, there is still a single internal key
implementation for performance reasons, but the public interface is a set of key
implementations that have type safe Of and Get methods.
This also hides the implemenation of Tag so that we can modify the storage form
and find a more efficient storage if needed.

Change-Id: I6a39cc75c2824c6a92e52d59f16e82e876f16e9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223137
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:29:43 +00:00
Ian Cottrell
d7fc2cf50e internal/telemetry: delete the event.TagOf method
It encourages poor performing log lines, and also reduces the readability of
those lines.
Also delete the Key.With method which was unused, and should never be used
instead of the event.Label function anyway.

Change-Id: I9b55102864ee49a7d03e60af022a2002170c0fb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222851
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:23:15 +00:00
Ian Cottrell
9dec35b5f8 internal/telemetry: change ocagent test to use the standard telemetry methods
Rather than building cusom events and driving the exporter, the test now
registers a custom exporter and then uses the normal higher level methods to
deliver events to it.
This means we are testing the actual information that would arise in a user
program, and also means we no longer have to expose internal features just for
tests.
Metrics are not fully converted yet.

Change-Id: I63248a480fb1b3e6274dce0c2e1d66904d055978
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222849
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:16 +00:00
Ian Cottrell
cb106d260e internal/telemetry: allow ProcessEvent to modify the event
This allows early exporters to adjust the event for later ones.
This is used to lookup key values from the context if needed.
Also add a Query type event which is intended to perform all event
modifications but nothing else, and is used to lookup values from
the context. This cleans up a weirdness where the current lookup
presumes there will be an exporter with a matching mechanism.

Change-Id: I835d1e0b2511553c30f94b7becfe7b7b5462c111
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223657
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:01 +00:00
Ian Cottrell
fbeba2149c internal/telemetry: split the ocagent tests from the support functions
This moves the actual trace test into its own file so that it can easily
be seen separately from the functions that support all tests.

Change-Id: I1d30cf8a712377ca79a5de77b85412c881177f43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223397
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-17 03:16:54 +00:00
Ian Cottrell
eff45d17df internal/telemetry: convert key from string to struct
This makes keys a pointer, which reduces the allocations during logging, and has
a significant improvement in memory and cpu cost when there is no exporter.
Packing a string into an interface requires a 16 byte allocation, packing a pointer
does not.

name                old time/op    new time/op    delta
/LogNoExporter-8      4.39µs ± 2%    3.83µs ± 2%  -12.74%  (p=0.000 n=18+20)
/Log-8                23.5µs ± 2%    22.6µs ± 1%   -4.20%  (p=0.000 n=19+18)

name                old alloc/op   new alloc/op   delta
/LogNoExporter-8      1.70kB ± 0%    1.38kB ± 0%  -18.78%  (p=0.000 n=20+20)
/Log-8                4.91kB ± 0%    4.91kB ± 0%     ~     (p=1.000 n=20+20)

name                old allocs/op  new allocs/op  delta
/LogNoExporter-8        83.0 ± 0%      63.0 ± 0%  -24.10%  (p=0.000 n=20+20)
/Log-8                   163 ± 0%       163 ± 0%     ~     (all equal)

Change-Id: Iec127f1bff8d5c8f4bd0a6d9f6d8fc4b8bc740b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222599
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 04:17:52 +00:00
Ian Cottrell
a9aaa4e906 internal/telemetry: use keys properly for benchmarks
Change-Id: I571628db26a5b89fd567077d90d325fd76301956
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222598
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 04:17:28 +00:00
Ian Cottrell
68dc0f3515 internal/telemetry: remove all the event aliases
Change-Id: I6d9a25be2b9dbba2385e13810e3c9e79c5f67aa5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222559
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 04:17:16 +00:00
Ian Cottrell
d780ff7bdd internal/telemetry: unify the event handling to an event package
This is now the only package that is exposed to normal use, and should
be the only thing to appear in libraries.

Change-Id: I90ee47c6519f30db16ff5d5d2910be86e91e5df2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222557
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 03:58:56 +00:00
Ian Cottrell
f30ed8521d internal/telemetry: change detach to be an event
deliver detach to the exporter as an event rather than hard coding the
span detach behavior.

Change-Id: I87b6e2a3596fea338908c11ba0b219176b6305bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222542
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-11 02:43:08 +00:00
Ian Cottrell
e8dd451b4b internal/telemetry: add a stats benchmark
Change-Id: I9cb1459a8589cec75ae29d0e53f044c6489b1ea2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222537
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-11 02:40:42 +00:00
Ian Cottrell
ca9608b5cd internal/telemetry: refactor the benchmark framework
Change-Id: I566ed7cd3a4d755cf06504e749ba54d0a309e53f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221921
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-06 18:16:20 +00:00
Rob Findley
8287d625a0 internal/lsp/debug: guard rpc stats with a Mutex
This type was previously guarded implicitly by the global exporter
mutex. With that gone, we've started seeing panics due to races in
(*rpcs).Metric.

Guard it with a mutex.

Change-Id: I2a8c670ecfbaee9422e1f1d282b1fedb86b6a5e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222300
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-03-06 16:15:13 +00:00
Ian Cottrell
32f14692fc internal/lsp: use standardised events for tagging
This means that tags also become cheap if there is no exporter and cleans up the
mess with how spans, tags and logs were related.
Also fixes the currently broken metrics that relied on the span tags.

Change-Id: I8e56b6218a60fd31a1f6c8d329dbb2cab1b9254d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222065
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-05 14:00:10 +00:00
Ian Cottrell
c4206d458c internal/telemetry: change tracing to be event based
We no longer use the span as the core type of tracing, instead that is an
artifact of the exporter, and start and end tracing is just event based.
This both makes the interface normalized, and also means the null exporter case
is considerably cheaper in memory and cpu.
See below for benchstat changes

name                 old time/op    new time/op    delta
TracingNoExporter-8    4.19µs ±12%    2.71µs ±11%  -35.33%  (p=0.000 n=20+20)
Tracing-8              24.1µs ± 3%     5.1µs ±17%  -78.66%  (p=0.000 n=16+20)

name                 old alloc/op   new alloc/op   delta
TracingNoExporter-8    2.32kB ± 0%    0.40kB ± 0%  -82.76%  (p=0.000 n=20+20)
Tracing-8              6.32kB ± 0%    2.32kB ± 0%  -63.30%  (p=0.000 n=20+20)

name                 old allocs/op  new allocs/op  delta
TracingNoExporter-8      35.0 ± 0%      15.0 ± 0%  -57.14%  (p=0.000 n=20+20)
Tracing-8                 215 ± 0%        35 ± 0%  -83.72%  (p=0.000 n=20+20)

Change-Id: I3cf25871fa49584819504b5c19aa580e5dd03395
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221740
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-04 02:41:40 +00:00
Ian Cottrell
c5a1414753 internal/telemetry: add simple tracing benchmarks
These are just like the logging benchmarks but for tracing instead.
Also makes the log writer write out tracing events as well if it is
not in only errors mode

Change-Id: Ie00d7c80f7e2b9433787603261950f70ab1c1e9d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221739
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:57:24 +00:00
Ian Cottrell
3e346efd93 internal/telemetry: move the benchmarks to the main package
Change-Id: I9aabed798951ffba775c2255c8baafd56b009636
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221738
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:57:05 +00:00
Ian Cottrell
046aa1cdaf internal/telemetry: moving towards a unified event based exporter
This adds a type to events and makes the logging calls use it.

Change-Id: Iaa50fe2e332caae611b1e00424c878a3bc479feb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221741
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:56:52 +00:00
Ian Cottrell
a1c56757aa internal/telemetry: remove the concept of a Tagger
The Tagger interface allowed for specifying a key in a tag list and having the
value be aquired from the context automatically. It was almost never used, and
the alternative (using the key to get the value) is not that much more long
winded, so it was not holding it's own weight from a complexity persective alone
but the performance cost of having to use a list of interface rather than a list
of tags was very large. See the benchstat improvements below for the difference
it makes to both speed and memory usage, especially in the no exporter case.

name                 old time/op    new time/op    delta
Baseline-8              341ns ± 2%     342ns ± 1%     ~     (p=0.139 n=19+18)
LoggingNoExporter-8    2.46µs ± 5%    1.97µs ± 3%  -19.88%  (p=0.000 n=19+20)
Logging-8              13.3µs ± 2%    12.8µs ± 2%   -3.42%  (p=0.000 n=17+20)
LoggingStdlib-8        5.39µs ± 6%    5.34µs ± 3%     ~     (p=0.692 n=20+19)

name                 old alloc/op   new alloc/op   delta
Baseline-8              80.0B ± 0%     80.0B ± 0%     ~     (all equal)
LoggingNoExporter-8      728B ± 0%      440B ± 0%  -39.56%  (p=0.000 n=20+20)
Logging-8              2.75kB ± 0%    2.46kB ± 0%  -10.53%  (p=0.000 n=17+20)
LoggingStdlib-8          568B ± 0%      568B ± 0%     ~     (all equal)

name                 old allocs/op  new allocs/op  delta
Baseline-8               5.00 ± 0%      5.00 ± 0%     ~     (all equal)
LoggingNoExporter-8      32.0 ± 0%      23.0 ± 0%  -28.12%  (p=0.000 n=20+20)
Logging-8                88.0 ± 0%      79.0 ± 0%  -10.23%  (p=0.000 n=20+20)
LoggingStdlib-8          28.0 ± 0%      28.0 ± 0%     ~     (all equal)

Change-Id: Ic203ad0c5de7451348976b999a0d038ac532dc39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:56:24 +00:00
Ian Cottrell
ae19ec7143 internal/telemetry: use atomics to get the exporter
We change the main exporter to be stored and fetched using atomics rather
than aquiring a mutex for a mild (but significant in the disabled case) speedup.
Also has the benefit of not holding a global lock over all telemetry operations.

benchstat of logging benchmatks before and after:

name                 old time/op    new time/op    delta
Baseline-8              329ns ± 2%     327ns ± 2%     ~     (p=0.181 n=19+17)
LoggingNoExporter-8    3.08µs ± 3%    2.42µs ± 2%  -21.42%  (p=0.000 n=20+19)
Logging-8              13.7µs ± 2%    13.2µs ± 1%   -3.49%  (p=0.000 n=19+19)
LoggingStdlib-8        5.39µs ± 3%    5.41µs ± 2%     ~     (p=0.177 n=19+20)

This is a replacement for https://go-review.googlesource.com/c/tools/+/212244
but built on the single exporter principle rather than the exporter list.

Change-Id: Icc99319c4357e0bcb63386c64372a733e8a76796
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221218
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:56:03 +00:00
Ian Cottrell
71482053b8 internal/telemetry: remove Flush method from exporter
It is never invoked anyway, and it provices no useful benefit while complicating
the implementation, as it is the only non context aware method.

Change-Id: Id5a99439fedafdf4d71285e36103b4854cf3635a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221540
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-02-28 22:46:39 +00:00
Ian Cottrell
2e887e3d13 internal/telemetry: removing the concept of exporter lists
Instead we only have a single exporter, and it must delegate behaviour
to any other exporters it wants to include.
This removes a whole collection of suprises caused by init functions adding
new exporters to a list, as well as generally making things faster, at the small
expense of needing to implement a custom exporter if you want to combine the
features of a few other exporters.

This is essentially the opposite of https://go-review.googlesource.com/c/tools/+/212243
which will now be abandoned in favor of this approach.

Change-Id: Icacb4c1f0f40f99ddd1d82c73d4f25a3486e56ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220857
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-26 21:51:01 +00:00
Ian Cottrell
a0ec867d51 internal/telemetry: add a benchmark with a null writer for comparison
Make all the benchmarks cleanly pass around the context to remove a non logging
difference.
Rename the non logging calls benchmark to Baseline

Change-Id: I24a33b0a817df403fb43c53b5f097bc1e9418af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220520
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-02-25 02:20:59 +00:00
Rebecca Stambler
207d3de1fa all: fix some staticcheck errors
Updates golang/go#35718

Change-Id: I10bfd5421cd44bb58b8bcaa6e9205040c25f51be
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208257
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-29 04:53:41 +00:00
Rob Findley
29f64efd55 internal/telemetry/log: correct the docstring for Error
Change-Id: I8077bbdd6f011087f7988fbf04c97c13575afec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215741
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-21 22:25:47 +00:00
Rob Findley
df87866820 internal/lsp,internal/telemetry: correct stale docstrings
Several docstrings reference earlier names for the symbols they
document. This CL corrects those that I noticed while reading the lsp
code.

Change-Id: I1968459feff7011e070333c99eb149e72d3302de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214801
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-15 14:25:42 +00:00
Ian Cottrell
f13409bbeb internal/telemetry: clean up test data
the current implementation likes to sort maps, so we make sure
tag lists are in sorted order already so that a stable encoder
produces the same result

Change-Id: Ia7ce05f35edb636817c354d9df02de753a48fe1d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210216
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 23:47:30 +00:00
Ian Cottrell
c7346ecdc6 internal/telemetry: remove an extraneous comment
There was a fragment of a sentence that must have been from a previous version
(as it talks about a return value for a function that does not have one).

Change-Id: I9d154fe10711344f93e1d49b68a811dbc9772710
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212241
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 23:47:11 +00:00
Ian Cottrell
814139985e internal/telemetry: obey the onlyErrors flag in the log writer
has no impact because there are no use cases that don't set it to true right now

Change-Id: I2bc485226078c710bdc36397b96755cdce82d9cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212242
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 23:46:55 +00:00
Ian Cottrell
2f6d8bf0ad internal/lsp: change tests to use the main exporter
Now the tests are at a high enough level, we can
switch them to using the full exporter by bulding
a fake http client to bind it to. This lets us
use the true public interface and also excercise
more of the functionality in the tests.
With this we are now ready to replace the entire
implementation safely.

Change-Id: Ifdbf6230de3ec7c7c5381c840b135cb7a0bc1e55
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209161
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-12-20 19:55:05 +00:00
Emmanuel T Odeke
2ad5dca7a5 telemetry/log: sample reference for benchmarking harness
Serves as a reference harness to get benchmarking started.
Steps to run benchmark:

$ go test -run=^$ -bench=. -count=10

After you grab the output, put the various comparisons
into files before.txt and after.txt then edit those
result names to have a common name e.g.
    s/BenchmarkLoggingNoExporter/BenchmarkIt/g
    s/BenchmarkNoTracingNoMetricsNoLogging/BenchmarkIt/g
    s/BenchmarkLoggingStdlib/BenchmarkIt/g

Now run benchstat:

* All compared
$ benchstat no_log.txt tellog.txt stdlog.txt
name \ time/op    no_log.txt  tellog.txt   stdlog.txt
It-8              289ns ± 2%  2780ns ± 3%  5100ns ± 3%

name \ alloc/op   no_log.txt  tellog.txt   stdlog.txt
It-8              80.0B ± 0%  728.0B ± 0%  568.0B ± 0%

name \ allocs/op  no_log.txt  tellog.txt   stdlog.txt
It-8               5.00 ± 0%   32.00 ± 0%   28.00 ± 0%

* No logging vs telemetry log
$ benchstat no_log.txt tellog.txt
name  old time/op    new time/op    delta
It-8     289ns ± 2%    2780ns ± 3%  +862.31%  (p=0.000 n=10+9)

name  old alloc/op   new alloc/op   delta
It-8     80.0B ± 0%    728.0B ± 0%  +810.00%  (p=0.000 n=10+10)

name  old allocs/op  new allocs/op  delta
It-8      5.00 ± 0%     32.00 ± 0%  +540.00%  (p=0.000 n=10+10)

* No logging vs Standard library "log"
$ benchstat no_log.txt stdlog.txt
name  old time/op    new time/op    delta
It-8     289ns ± 2%    5100ns ± 3%  +1665.16%  (p=0.000 n=10+9)

name  old alloc/op   new alloc/op   delta
It-8     80.0B ± 0%    568.0B ± 0%   +610.00%  (p=0.000 n=10+10)

name  old allocs/op  new allocs/op  delta
It-8      5.00 ± 0%     28.00 ± 0%   +460.00%  (p=0.000 n=10+10)

* telemetry log vs Standard library "log"
$ benchstat tellog.txt stdlog.txt
name  old time/op    new time/op    delta
It-8    2.78µs ± 3%    5.10µs ± 3%  +83.43%  (p=0.000 n=9+9)

name  old alloc/op   new alloc/op   delta
It-8      728B ± 0%      568B ± 0%  -21.98%  (p=0.000 n=10+10)

name  old allocs/op  new allocs/op  delta
It-8      32.0 ± 0%      28.0 ± 0%  -12.50%  (p=0.000 n=10+10)

Change-Id: I53b15e9da315615278c576f3a60108435417a9f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212078
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2019-12-19 18:19:13 +00:00
Ian Cottrell
99399511c1 internal/telemetry: lift the tests up to the request level
rather than testing events or metrics directly, we
test them in the context of a full message that would
get sent to the agent

Change-Id: I238a7f9ab7b1456d1b4b2bac2519d814928f2283
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209098
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-11-27 15:41:43 +00:00
Ian Cottrell
7360bd5c0f internal/telemtry: changed to a simpler threading model for stats
this is not a final solution, but it makes it easier to debug
and reason about, and does not require a go routine or buffered
channel

Change-Id: I758758ac80fcd525ab5264e34c48941766a8db11
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208664
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-11-26 22:52:16 +00:00
Ian Cottrell
9fe613bd66 internal/lsp: build the wire.Node lazily
this is needed to move to a model where we do not need to have
the wire form at all

Change-Id: I3b3693e027b568de4e8b4d10f7d5dd022a616e2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208958
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-11-26 22:51:56 +00:00
Ian Cottrell
fc82735a89 internal/telemetry: delay the conversion of metrics
convert to wire form only when we need to encode it

Change-Id: Ib36ee2fbef773241a30b9aa707ebf311119b8705
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208658
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Nathan Dias <nathan.dias@orijtech.com>
2019-11-26 18:15:43 +00:00