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

fix(goenv.GOMOD): never rely on the environment variable #384

Merged
merged 18 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 0 additions & 27 deletions instrument/instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,6 @@ import (
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
)

func getOpName(metadata ...any) string {
rank := map[string]int{
"verb": 1,
"function-name": 2,
}

var (
opname string
oprank = 10_000 // just a higher number than any key in the rank map.
)
for i := 0; i < len(metadata); i += 2 {
if i+1 >= len(metadata) {
break
}
if k, ok := metadata[i].(string); ok {
if r, ok := rank[k]; ok && r < oprank {
if on, ok := metadata[i+1].(string); ok {
opname = on
oprank = r
continue
}
}
}
}
return opname
}

func Report(ctx context.Context, e event.Event, metadata ...any) context.Context {
var span tracer.Span
if e == event.EventStart || e == event.EventCall {
Expand Down
43 changes: 6 additions & 37 deletions instrument/instrument_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023-present Datadog, Inc.

package instrument
package instrument_test

import (
"context"
"testing"

"github.com/DataDog/orchestrion/instrument"
"github.com/DataDog/orchestrion/instrument/event"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
Expand All @@ -17,65 +18,33 @@ import (
func TestReport(t *testing.T) {
t.Run("start", func(t *testing.T) {
ctx := context.Background()
ctx = Report(ctx, event.EventStart)
ctx = instrument.Report(ctx, event.EventStart)
if _, ok := tracer.SpanFromContext(ctx); !ok {
t.Errorf("Expected Report of StartEvent to generate a new ID.")
}
})

t.Run("call", func(t *testing.T) {
ctx := context.Background()
ctx = Report(ctx, event.EventCall)
ctx = instrument.Report(ctx, event.EventCall)
if _, ok := tracer.SpanFromContext(ctx); !ok {
t.Errorf("Expected Report of CallEvent to generate a new ID.")
}
})

t.Run("end", func(t *testing.T) {
ctx := context.Background()
ctx = Report(ctx, event.EventEnd)
ctx = instrument.Report(ctx, event.EventEnd)
if _, ok := tracer.SpanFromContext(ctx); ok {
t.Errorf("Expected Report of EndEvent not to generate a new ID.")
}
})

t.Run("return", func(t *testing.T) {
ctx := context.Background()
ctx = Report(ctx, event.EventReturn)
ctx = instrument.Report(ctx, event.EventReturn)
if _, ok := tracer.SpanFromContext(ctx); ok {
t.Errorf("Expected Report of ReturnEvent not to generate a new ID.")
}
})
}

func TestGetOpName(t *testing.T) {
for _, tt := range []struct {
metadata []any
opname string
}{
{
metadata: []any{"foo", "bar", "verb", "just-verb"},
opname: "just-verb",
},
{
metadata: []any{"foo", "bar", "function-name", "just-function-name"},
opname: "just-function-name",
},
{
metadata: []any{"foo", "bar", "verb", "verb-function-name", "function-name", "THIS IS WRONG"},
opname: "verb-function-name",
},
{
// Checking different order
metadata: []any{"foo", "bar", "function-name", "THIS IS WRONG", "verb", "verb-function-name"},
opname: "verb-function-name",
},
} {
t.Run(tt.opname, func(t *testing.T) {
n := getOpName(tt.metadata...)
if n != tt.opname {
t.Errorf("Expected %s, but got %s\n", tt.opname, n)
}
})
}
}
33 changes: 33 additions & 0 deletions instrument/opname.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023-present Datadog, Inc.

package instrument

func getOpName(metadata ...any) string {
rank := map[string]int{
"verb": 1,
"function-name": 2,
}

var (
opname string
oprank = 10_000 // just a higher number than any key in the rank map.
)
for i := 0; i < len(metadata); i += 2 {
if i+1 >= len(metadata) {
break

Check warning on line 20 in instrument/opname.go

View check run for this annotation

Codecov / codecov/patch

instrument/opname.go#L20

Added line #L20 was not covered by tests
}
if k, ok := metadata[i].(string); ok {
if r, ok := rank[k]; ok && r < oprank {
if on, ok := metadata[i+1].(string); ok {
opname = on
oprank = r
continue
}
}
}
}
return opname
}
40 changes: 40 additions & 0 deletions instrument/opname_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023-present Datadog, Inc.

package instrument

import "testing"

func TestGetOpName(t *testing.T) {
for _, tt := range []struct {
metadata []any
opname string
}{
{
metadata: []any{"foo", "bar", "verb", "just-verb"},
opname: "just-verb",
},
{
metadata: []any{"foo", "bar", "function-name", "just-function-name"},
opname: "just-function-name",
},
{
metadata: []any{"foo", "bar", "verb", "verb-function-name", "function-name", "THIS IS WRONG"},
opname: "verb-function-name",
},
{
// Checking different order
metadata: []any{"foo", "bar", "function-name", "THIS IS WRONG", "verb", "verb-function-name"},
opname: "verb-function-name",
},
} {
t.Run(tt.opname, func(t *testing.T) {
n := getOpName(tt.metadata...)
if n != tt.opname {
t.Errorf("Expected %s, but got %s\n", tt.opname, n)
}
})
}
}
13 changes: 13 additions & 0 deletions instrument/orchestrion.tool.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023-present Datadog, Inc.

//go:build tools

package instrument

// Ensures that `orchestrion.tool.go` files importing this package include all
// the necessary transitive dependencies for all possible integrations, and
// correctly discover the aspects to inject.
import _ "github.com/DataDog/orchestrion/internal/injector/builtin"
8 changes: 7 additions & 1 deletion internal/ensure/requiredversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"runtime"
"syscall"

"github.com/DataDog/orchestrion/internal/goenv"
"github.com/DataDog/orchestrion/internal/log"
"github.com/DataDog/orchestrion/internal/version"
"golang.org/x/tools/go/packages"
Expand Down Expand Up @@ -128,8 +129,13 @@
// working directory is used. The version may be blank if a replace directive is in effect; in which
// case the path value may indicate the location of the source code that is being used instead.
func goModVersion(dir string) (moduleVersion string, moduleDir string, err error) {
gomod, err := goenv.GOMOD(dir)
if err != nil {
return "", "", err
}

Check warning on line 135 in internal/ensure/requiredversion.go

View check run for this annotation

Codecov / codecov/patch

internal/ensure/requiredversion.go#L135

Added line #L135 was not covered by tests

cfg := &packages.Config{
Dir: dir,
Dir: filepath.Dir(gomod),
Mode: packages.NeedModule,
Logf: func(format string, args ...any) { log.Tracef(format+"\n", args...) },
}
Expand Down
8 changes: 3 additions & 5 deletions internal/ensure/requiredversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ func TestGoModVersion(t *testing.T) {
}

t.Run("no-go-mod", func(t *testing.T) {
tmp, err := os.MkdirTemp("", "ensure-*")
require.NoError(t, err, "failed to create temporary directory")
defer os.RemoveAll(tmp)
tmp := t.TempDir()

os.WriteFile(filepath.Join(tmp, "main.go"), []byte(`
package main
Expand All @@ -98,9 +96,9 @@ func TestGoModVersion(t *testing.T) {
`), 0o644)

require.NotPanics(t, func() {
_, _, err = goModVersion(tmp)
_, _, err := goModVersion(tmp)
require.ErrorIs(t, err, goenv.ErrNoGoMod)
})
require.ErrorContains(t, err, "go.mod file not found in current directory")
})
}

Expand Down
10 changes: 4 additions & 6 deletions internal/goenv/goenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ import (

var (
// ErrNoGoMod is returned when no GOMOD value could be identified.
ErrNoGoMod = errors.New("`go mod GOMOD` returned a blank string")
ErrNoGoMod = errors.New("`go env GOMOD` returned a blank string")
)

// GOMOD returns the current GOMOD environment variable (possibly from running `go env GOMOD`).
func GOMOD() (string, error) {
if goMod := os.Getenv("GOMOD"); goMod != "" {
return goMod, nil
}
// GOMOD returns the current GOMOD environment variable (from running `go env GOMOD`).
func GOMOD(dir string) (string, error) {
cmd := exec.Command("go", "env", "GOMOD")
cmd.Dir = dir
var stdout bytes.Buffer
cmd.Stdout = &stdout
if err := cmd.Run(); err != nil {
Expand Down
22 changes: 2 additions & 20 deletions internal/goenv/goenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,21 @@
package goenv

import (
"os"
"testing"

"github.com/stretchr/testify/require"
)

func TestGOMOD(t *testing.T) {
t.Run("without GOMOD environment variable", func(t *testing.T) {
t.Setenv("GOMOD", "")

gomod, err := GOMOD()
gomod, err := GOMOD("")
require.NoError(t, err)
require.NotEmpty(t, gomod)
})

t.Run("no GOMOD can be found", func(t *testing.T) {
t.Setenv("GOMOD", "")

wd, _ := os.Getwd()
defer os.Chdir(wd)
os.Chdir(os.TempDir())

val, err := GOMOD()
val, err := GOMOD(t.TempDir())
require.Empty(t, val)
require.ErrorIs(t, err, ErrNoGoMod)
})

t.Run("with GOMOD environment variable", func(t *testing.T) {
expected := "/fake/path/to/go.mod"
t.Setenv("GOMOD", expected)

gomod, err := GOMOD()
require.NoError(t, err)
require.EqualValues(t, expected, gomod)
})
}
2 changes: 1 addition & 1 deletion internal/pin/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func defaultOrchestrionToolGo() *dst.File {
// PinOrchestrion applies or update the orchestrion pin file in the current
// working directory, according to the supplied [Options].
func PinOrchestrion(opts Options) error {
goMod, err := goenv.GOMOD()
goMod, err := goenv.GOMOD("")
if err != nil {
return fmt.Errorf("getting GOMOD: %w", err)
}
Expand Down