Add tracing instrumentation #3445
Conversation
|
a quick feedback: the first two commits should be one commit. Not vendoring the dependencies breaks the build and no commit should break the build if possible. |
|
I would also recommend to pull the You can rebase this one after that one is merged - which hopefully will be really fast. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3445 +/- ##
==========================================
- Coverage 73.35% 73.16% -0.20%
==========================================
Files 261 265 +4
Lines 19778 20083 +305
==========================================
+ Hits 14509 14694 +185
- Misses 4372 4471 +99
- Partials 897 918 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I will address these two points. Treat this PR as a draft currently. |
|
@ka3de could you also fill the PR checklist accordingly so as a reviewer, I have an idea of where we're at? 🙇🏻 |
|
Once #3446 is merged, I will force push rebasing the changes along the modifications discussed in #3445 (comment), and then set the PR up for review. |
This package provides APIs in order to interact with Open Telemetry tracing instrumentation.
This flag can be used to configure the traces output by setting the destination endpoint, protocol, extra headers, etc.
Initializes the global state's TracerProvider based on traces output cmdline flag.
Waits for tracer provider to stop within 5 seconds, otherwise an error message is logged. Because both methods, stopLoggers() and stopTracerProvider(), are bounded by a timeout, execute them concurrently and wait for them to finish.
Because traces will only be generated during a test run, move the tracer provider initialization and stop to the run command. Additionally, for this same reason, there is no need to wait for the tracer provider to shutdown unless the actual test run is executed, therefore wait for it concurrently with metrics processing wait.
Avoid a default initialization for TracerProvider in GlobalState and instead delegate that to the run command, if executed.
There was a problem hiding this comment.
LGTM in general - just move the flag configuration to the runtime options.
I have not really looked at the tracing parts - I expect @Blinkuu will be way more useful there.
I would also prefer that with/after the merge from k6-browser where this is used to get an integration test that mocks a tracing endpoint and checks that we get something useful.
Move initialization after loadAndConfigureTest call so we use the runtime options, which are parsed in that method.
cmd/run.go
Outdated
| return nil | ||
| } | ||
|
|
||
| tp, err := trace.TracerProviderFromConfigLine(c.gs.Ctx, ro.TracesOutput.String) |
There was a problem hiding this comment.
nit: you can provide the globalcCtx as is done in the cancelation timeout
There was a problem hiding this comment.
Right, that makes total sense. Fixed in 663c6c2.
|
Also please either squash the whole thing on merge or make it into 2(?) commits instead of 15 |
I don't have permissions to merge. I would say squash at this point if that works for you. Otherwise let me know and I'll try to organize it in a few commits that make sense. |
olegbespalov
added
the
documentation-needed
What?
This PR adds a base tracing instrumentation around opentelemetry-go library. Specifically initializes a
TracerProviderbased on a new traces ouput configuration option (--traces-output/K6_TRACES_OUTPUT). TheTracerProvideris exposed partially to modules through the VU.State() so these can request Tracers initialization that can be used to generate traces.Why?
There has been various requests to add more tracing functionality to k6, both from k6 users and from extension developers. This PR represents a first step towards promoting tracing as a first class citizen in k6.
Checklist
make lint) and all checks pass.make tests) and all tests pass.