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

Read module dependencies from debug.BuildInfo instead of go.mod #199

Merged
merged 11 commits into from
Apr 17, 2020
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/getsentry/sentry-go

go 1.11
go 1.12

require (
github.com/ajg/form v1.5.1 // indirect
Expand Down
138 changes: 16 additions & 122 deletions integrations.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
package sentry

import (
"bufio"
"encoding/json"
"fmt"
"io/ioutil"
"os"
"regexp"
"runtime"
"runtime/debug"
"strings"
"sync"
)
Expand All @@ -32,133 +29,30 @@ func (mi *modulesIntegration) SetupOnce(client *Client) {
func (mi *modulesIntegration) processor(event *Event, hint *EventHint) *Event {
if len(event.Modules) == 0 {
mi.once.Do(func() {
mi.modules = extractModules()
info, ok := debug.ReadBuildInfo()
if !ok {
Logger.Print("The Modules integration is not available in binaries built without module support.")
return
}
mi.modules = extractModules(info)
})
}
event.Modules = mi.modules
return event
}

func extractModules() map[string]string {
extractedModules, err := getModules()
if err != nil {
Logger.Printf("ModuleIntegration wasn't able to extract modules: %v\n", err)
return nil
}
return extractedModules
}

func getModules() (map[string]string, error) {
if fileExists("go.mod") {
return getModulesFromMod()
func extractModules(info *debug.BuildInfo) map[string]string {
modules := map[string]string{
info.Main.Path: info.Main.Version,
}

if fileExists("vendor") {
// Priority given to vendor created by modules
if fileExists("vendor/modules.txt") {
return getModulesFromVendorTxt()
}

if fileExists("vendor/vendor.json") {
return getModulesFromVendorJSON()
for _, dep := range info.Deps {
ver := dep.Version
if dep.Replace != nil {
ver += fmt.Sprintf(" => %s %s", dep.Replace.Path, dep.Replace.Version)
}
modules[dep.Path] = strings.TrimSuffix(ver, " ")
}

return nil, fmt.Errorf("module integration failed")
}

func getModulesFromMod() (map[string]string, error) {
modules := make(map[string]string)

file, err := os.Open("go.mod")
if err != nil {
return nil, fmt.Errorf("unable to open mod file")
}

defer file.Close()

areModulesPresent := false

scanner := bufio.NewScanner(file)
for scanner.Scan() {
splits := strings.Split(scanner.Text(), " ")

if splits[0] == "require" {
areModulesPresent = true

// Mod file has only 1 dependency
if len(splits) > 2 {
modules[strings.TrimSpace(splits[1])] = splits[2]
return modules, nil
}
} else if areModulesPresent && splits[0] != ")" && splits[0] != "" {
modules[strings.TrimSpace(splits[0])] = splits[1]
}
}

if scannerErr := scanner.Err(); scannerErr != nil {
return nil, scannerErr
}

return modules, nil
}

func getModulesFromVendorTxt() (map[string]string, error) {
modules := make(map[string]string)

file, err := os.Open("vendor/modules.txt")
if err != nil {
return nil, fmt.Errorf("unable to open vendor/modules.txt")
}

defer file.Close()

scanner := bufio.NewScanner(file)
for scanner.Scan() {
splits := strings.Split(scanner.Text(), " ")

if splits[0] == "#" {
modules[splits[1]] = splits[2]
}
}

if scannerErr := scanner.Err(); scannerErr != nil {
return nil, scannerErr
}

return modules, nil
}

func getModulesFromVendorJSON() (map[string]string, error) {
modules := make(map[string]string)

file, err := ioutil.ReadFile("vendor/vendor.json")

if err != nil {
return nil, fmt.Errorf("unable to open vendor/vendor.json")
}

var vendor map[string]interface{}
if unmarshalErr := json.Unmarshal(file, &vendor); unmarshalErr != nil {
return nil, unmarshalErr
}

packages := vendor["package"].([]interface{})

// To avoid iterative dependencies, TODO: Change of default value
lastPath := "\n"

for _, value := range packages {
path := value.(map[string]interface{})["path"].(string)

if !strings.Contains(path, lastPath) {
// No versions are available through vendor.json
modules[path] = ""
lastPath = path
}
}

return modules, nil
return modules
}

// ================================
Expand Down
104 changes: 104 additions & 0 deletions integrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ package sentry
import (
"path/filepath"
"regexp"
"runtime/debug"
"testing"

"github.com/google/go-cmp/cmp"
)

func TestTransformStringsIntoRegexps(t *testing.T) {
Expand Down Expand Up @@ -238,3 +241,104 @@ func TestContextifyFramesNonexistingFilesShouldNotDropFrames(t *testing.T) {
contextifiedFrames := cfi.contextify(frames)
assertEqual(t, len(contextifiedFrames), len(frames))
}

func TestExtractModules(t *testing.T) {
tests := []struct {
name string
info *debug.BuildInfo
want map[string]string
}{
{
name: "no require modules",
info: &debug.BuildInfo{
Main: debug.Module{
Path: "my/module",
Version: "(devel)",
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized that at the moment the Version of the main module (in fact, the module that contains package main) is always "(devel)". That is unfortunate, but at least the module name will be in Sentry. Proper version can be added using the Releases feature.

golang/go#29228

},
Deps: []*debug.Module{},
},
want: map[string]string{
"my/module": "(devel)",
},
},
{
name: "have require modules",
info: &debug.BuildInfo{
Main: debug.Module{
Path: "my/module",
Version: "(devel)",
},
Deps: []*debug.Module{
{
Path: "github.com/getsentry/sentry-go",
Version: "v0.5.1",
},
{
Path: "github.com/gin-gonic/gin",
Version: "v1.4.0",
},
},
},
want: map[string]string{
"my/module": "(devel)",
"github.com/getsentry/sentry-go": "v0.5.1",
"github.com/gin-gonic/gin": "v1.4.0",
},
},
{
name: "replace module with local module",
info: &debug.BuildInfo{
Main: debug.Module{
Path: "my/module",
Version: "(devel)",
},
Deps: []*debug.Module{
{
Path: "github.com/getsentry/sentry-go",
Version: "v0.5.1",
Replace: &debug.Module{
Path: "pkg/sentry",
},
},
},
},
want: map[string]string{
"my/module": "(devel)",
"github.com/getsentry/sentry-go": "v0.5.1 => pkg/sentry",
},
},
{
name: "replace module with another remote module",
info: &debug.BuildInfo{
Main: debug.Module{
Path: "my/module",
Version: "(devel)",
},
Deps: []*debug.Module{
{
Path: "github.com/ugorji/go",
Version: "v1.1.4",
Replace: &debug.Module{
Path: "github.com/ugorji/go/codec",
Version: "v0.0.0-20190204201341-e444a5086c43",
},
},
},
},
want: map[string]string{
"my/module": "(devel)",
"github.com/ugorji/go": "v1.1.4 => github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43",
},
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got := extractModules(tt.info)
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("modules info mismatch (-want +got):\n%s", diff)
}
Comment on lines +339 to +341
Copy link
Contributor

Choose a reason for hiding this comment

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

I took the liberty to change this to use cmp.Diff to make it easier to spot problems when tests fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rhcarvalho I think we can turn assertEquals to uses cmp.Diff to display the diff when it's not equal. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the point, as it just limits the usability. Personally I prefer calling cmp.Diff directly, it is more clear and flexible.

assertEquals always calls t.Fatal*, even when t.Error* would be more appropriate. Moving cmp.Diff into assertEquals would also limit control of comparison options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. Thanks for your point.

})
}
}