-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Experimental support for tracing #16020
Conversation
I'm working on cleaning this up to be a reasonable PR (reasonable size, better factored). I started by splitting out #16021 which is mostly orthogonal (but supporting) changes to add context.Context to more methods in the vfs layer. |
6abfb3d
to
6199c31
Compare
cc @rifelpet |
dbfd339
to
58846d5
Compare
ea1c2d5
to
2d822b1
Compare
Added some basic docs in the last commit (including describing it as very early experimental support), and so removing WIP marker. |
/test pull-kops-e2e-k8s-gce-cilium (though it looks like it might be unrelated but legitimate?) |
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.
Nice work! Do you think we could either set that env var in the presubmit prow jobs, or push a temporary commit to this PR that sets it by default to a filepath within the prow artifacts path? Just to see an example file
2586155
to
cc203df
Compare
Good idea. In the last two commits I tweaked it to (1) write files to a directory and set up kubetest to default that directory if artifacts is set and (2) tweaked the reader to use the VFS layer. So now from this branch you can do:
http://127.0.0.1:16686/trace/5152af7cbbfc499316aba7df80ce1a76 is then the most interesting trace (I think). |
Very cool! for others, this is what the tracing looks like. The tranches as described here are very apparent. |
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.
This might be worthy of a release note. Even though it is only relevant to kops development, it might head off any concerns about introducing telemetry to the project, clarifying that it doesn't "phone home" or connect to any remote servers.
otherwise LGTM!
We initially support capturing to a file (in our own format, as it doesn't appear a suitable format exists). This means we don't need a server to capture the traces, and can start capturing through prow without a lot of infrastructure changes. Co-authored-by: Peter Rifel <rifelpet@users.noreply.github.com>
This allows us to explore a trace file in jaeger.
Co-authored-by: Peter Rifel <rifelpet@users.noreply.github.com>
If given a directory, we can construct a reasonable name based on the executable name, pid and timestamp. Then this is relatively easy to wire up from kubetest2, if we have an artifacts directory.
This lets us easily show the results from prow jobs, captured as artifacts.
In particular, highlighting that it is not "phone home" telemetry.
Good idea @rifelpet (and thanks for uploading the picture) - added a release note to highlight the fact that this is not telemetry in the "phone home" sense! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rifelpet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Implement simple otel tracing, starting with the kOps CLI.
Prompted by discussion in kOps office hours about how it would be understanding where we are spending time e.g. on node startup.