Allow stacktrace path override for package sources

[bozaro]

What type of PR is this?

Feature

What does this PR do? Why is it needed?

We use go build for local development and bazel build for CI.
Our IDE has Analyze Stack Trace feature for navigation to code from log traces.

So, we want to have more friendly logfile stack traces like:

3 @ 0x447bb6 0x457a3c 0xc0ee95 0xc0f5a5 0xc18d8a 0x479161
#	0xc0ee94	google.golang.org/grpc/internal/transport.(*controlBuffer).get+0x114	go/pkg/mod/google.golang.org/grpc@v1.47.0/internal/transport/controlbuf.go:408
#	0xc0f5a4	google.golang.org/grpc/internal/transport.(*loopyWriter).run+0x84	go/pkg/mod/google.golang.org/grpc@v1.47.0/internal/transport/controlbuf.go:535
#	0xc18d89	google.golang.org/grpc/internal/transport.newHTTP2Client.func3+0x69	go/pkg/mod/google.golang.org/grpc@v1.47.0/internal/transport/http2_client.go:415

instead of:

1 @ 0x438d96 0x448812 0xeb7155 0xeb7865 0xec0fc5 0x468e81
#	0xeb7154	google.golang.org/grpc/internal/transport.(*controlBuffer).get+0x114	external/src-org_golang_google_grpc/internal/transport/controlbuf.go:408
#	0xeb7864	google.golang.org/grpc/internal/transport.(*loopyWriter).run+0x84	external/src-org_golang_google_grpc/internal/transport/controlbuf.go:535
#	0xec0fc4	google.golang.org/grpc/internal/transport.newHTTP2Client.func3+0x64	external/src-org_golang_google_grpc/internal/transport/http2_client.go:415

This PR adds ability to set package stack trace visible path with optional stackpath attribute.

Stack trace path override example:

go_library(
    name = "transport",
    srcs = [
        ...
    ],
    importpath = "google.golang.org/grpc/internal/transport",
    stackpath = "go/pkg/mod/google.golang.org/grpc@v1.47.0/internal/transport",
    visibility = ["//:__subpackages__"],
    deps = [
        ...
    ],
)

[sluongng]

Few thoughts:

1. A bit backward when we have to update rules_go to support a single IDE's feature. I think it's a nice change to have still, but it would be nice to show consideration on how this would work with other toolings in the same space: gopls, delve, ... OR observability vendors such as Datadog, Sentry etc...

2. It seems like there is an assumption that there would be a GOPATH or GOMODCACHE path in use. That assumption is not always true for a Bazel setup and could use some documentation suggesting how to use stackpath.

3. I was wondering what the correct "default" value of stackpath should be. For external dependencies, it seems like the external deps could be found at.

$(bazel info execution_root)/external/<dep-normalized-name>/<package-path/foo.go

In the PR description, you seem to be hinting toward external/src-org_golang_google_grpc with the src- prefix which I'm not quite sure if it's something specific to your setup.

[sluongng]

This test setup puzzled me a bit as I don't fully understand what's the differences between :stackpath_test and :stackpath_embed_test.

I am expecting 2-3 sets of tests to demonstrate:

• Default behaviors when stackpath is not set
• stackpath is set only in test
• stackpath is set in dep's lib
• stackpath is set in both dep's lib and test

I would suggest rename the test and add some docs into https://github.com/bazelbuild/rules_go/blob/master/tests/core/go_test/README.rst

[bozaro]

stackpath only affects the package itself. It does not affect or depend on deps. But he can come via embed (like: importpath).

[bozaro]

Few thoughts:

1. A bit backward when we have to update rules_go to support a single IDE's feature. I think it's a nice change to have still, but it would be nice to show consideration on how this would work with other toolings in the same space: gopls, delve, ... OR observability vendors such as Datadog, Sentry etc...

In our case, the pain was caused by the fact that we use go build locally, but for production build use bazel.
This causes problems with different source file paths in stack trace. I want to add stackpath option to control stack trace path behavior.

The option itself is not IDE specific.

Perhaps there will be different stack layouts. But they will be at the gazelle or similar utilities level.

2. It seems like there is an assumption that there would be a GOPATH or GOMODCACHE path in use. That assumption is not always true for a Bazel setup and could use some documentation suggesting how to use stackpath.

Yes. Our stack trace layout is not fully portable:

• https://youtrack.jetbrains.com/issue/GO-13568/What-should-portable-paths-look-like-in-StackTrace-for-GoLang

But it's good enough for us :)

I'm not saying the current stacktrace layout is bad. I just want the ability to change it.

3. I was wondering what the correct "default" value of stackpath should be. For external dependencies, it seems like the external deps could be found at.

It seems to me that the current layout is ideal as the default option: it is meaningful in the case of a bazel build.

I don't known about "universal" stacktrace layout. We try use go build-like style, but:

• we expect default GOMODCACHE on local developer workspace;
• we can't solve Go SDK reference issue;
• we need to download modules locally for these paths using the IDE.

But we are giving our developers the opportunity to live without knowing about BUILD.bazel files, as it was before the build via bazel.

$(bazel info execution_root)/external//<package-path/foo.go
In the PR description, you seem to be hinting toward external/src-org_golang_google_grpc with the src- prefix which I'm not quite sure if it's something specific to your setup.

We generate go_repository rules by our tool without gazelle. This prefix appeared due to multiple dependency graphs for different go.mod files.

[bozaro]

I was wondering what the correct "default" value of stackpath should be.

I wrote a small example to illustrate the standard stacktrace: https://github.com/bozaro/testify-example/tree/master/trace

It consists of two files:

go.mod

module github.com/bozaro/testify-example/trace

go 1.19

require github.com/pkg/errors v0.9.1

require (
	github.com/labstack/gommon v0.3.0 // indirect
	github.com/mattn/go-colorable v0.1.7 // indirect
	github.com/mattn/go-isatty v0.0.12 // indirect
	github.com/mkideal/cli v0.2.7 // indirect
	github.com/mkideal/expr v0.1.0 // indirect
	golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad // indirect
	golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd // indirect
	golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 // indirect
)

cmd/trace/main.go

package main

import (
	"fmt"
	"github.com/mkideal/cli"
	"github.com/pkg/errors"
	"os"
)

type argT struct {
	Name string `cli:"name" dft:"world" usage:"tell me your name"`
}

func main() {
	os.Exit(cli.Run(new(argT), func(ctx *cli.Context) error {
		argv := ctx.Argv().(*argT)
		fmt.Printf("%+v\n", errors.WithStack(errors.New(fmt.Sprintf("Hello, %s!\n", argv.Name))))
		return nil
	}))
}

And launched it with the command:

➜ go run -trimpath ./cmd/trace 
Hello, world!

main.main.func1
	github.com/bozaro/testify-example/trace/cmd/trace/main.go:17
github.com/mkideal/cli.(*Command).RunWith
	github.com/mkideal/cli@v0.2.7/command.go:219
github.com/mkideal/cli.(*Command).Run
	github.com/mkideal/cli@v0.2.7/command.go:176
github.com/mkideal/cli.RunWithArgs
	github.com/mkideal/cli@v0.2.7/cli.go:30
github.com/mkideal/cli.Run
	github.com/mkideal/cli@v0.2.7/cli.go:15
main.main
	github.com/bozaro/testify-example/trace/cmd/trace/main.go:15
runtime.main
	runtime/proc.go:250
runtime.goexit
	runtime/asm_amd64.s:1594
main.main.func1
	github.com/bozaro/testify-example/trace/cmd/trace/main.go:17
github.com/mkideal/cli.(*Command).RunWith
	github.com/mkideal/cli@v0.2.7/command.go:219
github.com/mkideal/cli.(*Command).Run
	github.com/mkideal/cli@v0.2.7/command.go:176
github.com/mkideal/cli.RunWithArgs
	github.com/mkideal/cli@v0.2.7/cli.go:30
github.com/mkideal/cli.Run
	github.com/mkideal/cli@v0.2.7/cli.go:15
main.main
	github.com/bozaro/testify-example/trace/cmd/trace/main.go:15
runtime.main
	runtime/proc.go:250
runtime.goexit
	runtime/asm_amd64.s:1594

There are three kinds of paths

• github.com/bozaro/testify-example/trace/cmd/trace/main.go:17 - path to local files (base directory replaced by full package name);
• github.com/mkideal/cli@v0.2.7/command.go:219 - path to third-party library files (base directory replaced by full package name with version);
• runtime/proc.go:250 - path to golang runtime files.

It seems to me that this can be considered as a standard representation.

Unfortunately, in my case, the IDE cannot recognize any file.

In any case, the stacktrace path layout should be outside of rules_go (for example, in gazelle). It only needs support for redefining the path in stacktrace for single package.

[bozaro]

Pull request is stuck. @sluongng, perhaps you are waiting something from me...

[sluongng]

I am not in a good position to review this PR. I don't use IntelliJ / GoLand enough in recent days to tell how this would impact the IDE. I also am not sure whether there is a more feasible approach (i.e. a wrapper that handles similar transition/conversion).

I will yield the review to @linzhp who spoke about IDE support at Uber. @achew22 might have some thoughts as well. 🙇

[linzhp]

I can review it later this week

[JamyDev]

Since this is mapping to go's native -trimpath flag. Is there a way you can use that instead if we were to map it through from the rule definition?

Staying closer to the native implementation is always better.

[sluongng]

@JamyDev Just for my own understanding, are you suggesting a rename of s/stackpath/trimpath/ in the rule attribute name?

[JamyDev]

I was suggesting we just expose the native functionality and use that. You can then have a custom rule outside of rules_go that provides stackpath and converts it into the native -trimpath.

[bozaro]

Perhaps the name stackpath is not a good one. Maybe something else, for example sourceprefix would be better.

But it seems to me that using trimpath for parameters with different format and semantics is a bad idea:

• trimpath in go build (boolean flag) - flag for using source file paths independent of the build location in debugging information;
• trimpath in $GOTOOLDIR/compile (string-to-string map) - mapping the original absolute path to the new prefix of the path of the source files in the debugging information;
• stackpath in go_library (string) - set prefix of the path of the source files in the debugging information.

It is very difficult to implement the format of the native parameter:

• It is impossible to make a boolean flag like in go build, since we have already lost information about the module, etc., which is also involved in the formation of the path;
• You will not be able to set the $GOTOOLDIR/compile trimpath parameter outside of rules_go as-is, since you need absolute paths to the source files (including those generated inside rules_go).

That is, in any case, the format of the parameter will differ from the native trimpath parameter and it will require some transformations inside rules_go.

[JamyDev]

Thanks for clarifying @bozaro

I think the main concerns I have here is:

A. manually having to define the full stackpath for each library + version causes a lot of duplication in your library rules. It should be automatic.
B. Your implementation is built around the way you have setup your repo (running go build during dev and bazel build during prod), meaning it's rather tailored to this specific scenario you're facing. I don't see this being used elsewhere a lot.

What I'm perceiving here is that you want go style output and support for go style builds, which doesn't fit with regards to how rules_go is built in my opinion.

Now let's say I'm wrong and we're okay with providing go-style support, a solution I see here is just putting this behind a "go_style_(packages|paths|stacks)" flag, which means we could put more functionality behind it.

The simpler solution IMO would be write a little tool that converts your stackpaths after the fact to the rules_go folder structure?

[bozaro]

A. manually having to define the full stackpath for each library + version causes a lot of duplication in your library rules. It should be automatic.

Yes. It should be automatic. But this automation can be implemented somewhere at the go_repository level.

For example: bazelbuild/bazel-gazelle#1379

B. Your implementation is built around the way you have setup your repo (running go build during dev and bazel build during prod), meaning it's rather tailored to this specific scenario you're facing. I don't see this being used elsewhere a lot.

I see only two options for the development process:

• Keep the BUILD files up to date and use bazel to build.
• Use locally go build as a shotcut from the permanent updating of BUILD files.

In our case, the ability to let developers use go build locally was the easiest way to use bazel painlessly.

In addition, you still need to support the ability to go build to manage dependencies via go.mod files (bazel does not solve the dependency management problem).

So I admit that this way of using bazel is used not only by us.

Now let's say I'm wrong and we're okay with providing go-style support, a solution I see here is just putting this behind a "go_style_(packages|paths|stacks)" flag, which means we could put more functionality behind it.

At the rules_go level, it is no longer possible to generate go-style stacks:

For example, a version from go.mod that has already been lost gets into the path:

github.com/mkideal/cli.Run
	github.com/mkideal/cli@v0.2.7/cli.go:15

I think go_style_stacks can only be implemented at the gazelle generator level (or similar tools).

The simpler solution IMO would be write a little tool that converts your stackpaths after the fact to the rules_go folder structure?

The first thing we tried to do was to change the format of the paths before writing stack trace to the log.

To do this, we sewed into the source file the correspondence of the prefix of paths from bazel and to the stack trace prefix. The code was very nasty, but it worked.

Unfortunately, to generate paths, you need:

• stack trace from the log;
• go.mod files from the original project;
• knowledge of how the rules that generate files arrange them into directories.

I see only one reliable way to implement an external utility: generate a file based on a BES file with matching paths for each source go file.

The presence of input data from the assembly and trace greatly complicates the use of such a utility.

But the most unpleasant thing is that this approach does not cover all the places where the source files appear. For example, go perf in this case was with bazel paths.

[linzhp]

I agree with @JamyDev.

Supporting two build systems for a language at the same time is not the right way to go for a developer experience team. I agree there are gaps in integrating Bazel+Go+IDEs, but the community should work together to improve IDE plugins for Bazel+Go. Uber will continue to make our fair share of contribution by upstreaming our internal improvements to IDE plugins, Go package driver for Bazel, and possibly gopls too.

In addition, you still need to support the ability to go build to manage dependencies via go.mod files (bazel does not solve the dependency management problem).

Go module system is still needed to populate go.mod file, but that doesn't mean we need the build system go build as well. After resolving the deps with go mod tidy, one can run gazelle update-repos in WORKSPACE mode or go_deps in bzlmod mode to import the deps for Bazel. However, both would require developers to use IDE plugins to resolve imports of external libraries.

IDE plugins also allow you to resolve generated code without having to checking them in, reducing a step in developer workflow to generate code and make sure it's up-to-date.

With a clear path forward, I don't think it's a good idea for rules_ to support such an invasive feature so that it produces the stack trace of another build system.

[bozaro]

Go module system is still needed to populate go.mod file, but that doesn't mean we need the build system go build as well.

In practice, this means that all packages (including the generated code) must exist in a working copy. In fact, it is synonymous with compilability.

IDE plugins also allow you to resolve generated code without having to checking them in, reducing a step in developer workflow to generate code and make sure it's up-to-date.

When reading articles about the integration of basel and GoLang, I have not found a seamless combination of tools and processes. Everything is limited to hello world examples, but I didn't see a complete solution.

Every time I use the plugin for basel in Idea I feel pain. I know we have developers who use vim as an IDE.

So I have no faith in integrating all IDEs with bazel. At least in the perspective of the next two years.

[bozaro]

Rebased to v0.41.0

[bozaro]

Rebased to v0.47.1