Skip to content
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

Improve tracing support. #3238

Merged
merged 6 commits into from
Sep 16, 2019
Merged

Improve tracing support. #3238

merged 6 commits into from
Sep 16, 2019

Conversation

pgavlin
Copy link
Member

@pgavlin pgavlin commented Sep 16, 2019

This is a combination of two commits. The first commit fixes a number of
nagging issues with unparented spans; the second adds the ability to write
trace data to a file and view the result using the CLI.

Fix some tracing issues.

  • Add endpoints for startUpdate and postEngineEventsBatch so that
    spans for these invocations have proper names
  • Inject a tracing span when walking a plan so that resource operations
    are properly parented
  • When handling gRPC calls, inject a tracing span into the call's
    metadata if no span is already present so that resource monitor and
    engine spans are properly parented
  • Do not trace client gRPC invocations of the empty method so that these
    calls (which are used to determine server availability) do not muddy
    the trace. Note that I tried parenting these spans appropriately, but
    doing so broke the trace entirely.

With these changes, the only unparented span in a typical Pulumi
invocation is a single call to getUser. This span is unparented
because that call does not have a context available. Plumbing a context
into that particular call is surprisingly tricky, as it is often called
by other context-less functions.

Make tracing support more flexible.

  • Add support for writing trace data to a local file using Appdash
  • Add support for viewing Appdash traces via the CLI

- Add endpoints for `startUpdate` and `postEngineEventsBatch` so that
  spans for these invocations have proper names
- Inject a tracing span when walking a plan so that resource operations
  are properly parented
- When handling gRPC calls, inject a tracing span into the call's
  metadata if no span is already present so that resource monitor and
  engine spans are properly parented
- Do not trace client gRPC invocations of the empty method so that these
  calls (which are used to determine server availability) do not muddy
  the trace. Note that I tried parenting these spans appropriately, but
  doing so broke the trace entirely.

With these changes, the only unparented span in a typical Pulumi
invocation is a single call to `getUser`. This span is unparented
because that call does not have a context available. Plumbing a context
into that particular call is surprisingly tricky, as it is often called
by other context-less functions.
- Add support for writing trace data to a local file using Appdash
- Add support for viewing Appdash traces via the CLI
cmd/pulumi.go Outdated
@@ -175,6 +175,7 @@ func NewPulumiCmd() *cobra.Command {
cmd.AddCommand(newPluginCmd())
cmd.AddCommand(newVersionCmd())
cmd.AddCommand(newHistoryCmd())
cmd.AddCommand(newViewTraceCmd())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to hide this under the debug flag if that seems desirable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a grouping of commands like this that are "diagnostic" commands so that we could at least move this out of the top-level?

@pgavlin
Copy link
Member Author

pgavlin commented Sep 16, 2019

Here's a screenshot from the embedded trace viewer:

Screen Shot 2019-09-16 at 12 29 49 PM

Copy link
Member

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great to see the improvements. A couple comments, but nothing which has to hold up getting the improved tracing support in.

cmd/view-trace.go Outdated Show resolved Hide resolved
pkg/resource/deploy/source_eval.go Show resolved Hide resolved
pkg/resource/deploy/source_query.go Outdated Show resolved Hide resolved
pkg/util/rpcutil/interceptor.go Show resolved Hide resolved
cmd/pulumi.go Outdated
@@ -175,6 +175,7 @@ func NewPulumiCmd() *cobra.Command {
cmd.AddCommand(newPluginCmd())
cmd.AddCommand(newVersionCmd())
cmd.AddCommand(newHistoryCmd())
cmd.AddCommand(newViewTraceCmd())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a grouping of commands like this that are "diagnostic" commands so that we could at least move this out of the top-level?

@pgavlin
Copy link
Member Author

pgavlin commented Sep 16, 2019

I wonder if there is a grouping of commands like this that are "diagnostic" commands so that we could at least move this out of the top-level?

There might be. For now I'm going to move this under the debug flag so that it doesn't clutter the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants