From 0f72b63fc531c26efcc324ae8e6f52aaa40fe07c Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Tue, 28 Jan 2025 17:49:51 +0100 Subject: [PATCH] fix: load built-in aspects from binary instead of resolving YAML (#517) The built-in orchestrion aspects must be loaded from the running binary instead of being loaded from disk. This ensures the correct version of aspects is being used even if the running version is not the one declared in `go.mod` (which cannot always be the case anyway). In addition to this, `orchestrion pin` should upgrade `go.mod` to the running version of `orchestrion` if it's newer than what's already in there; and we default-install `dd-trace-go.v1@main` for now (pending the next release). This change also changes the `go` directive in the `go.mod` files to be pinned to the `.0` patch level of the current lowest supported go minor, instead of pinning it to the latest available one (which is unnessary and can be inconvenient to users). --- .github/actions/codecov-cli/action.yml | 2 +- .github/dependabot.yml | 2 + .github/workflows/deps-update.yml | 5 +- .github/workflows/validate.yml | 4 +- _docs/go.mod | 2 +- _tools/go.mod | 2 +- go.mod | 2 +- instrument/go.mod | 2 +- internal/injector/config/builtin.go | 48 +++++++ internal/injector/config/config-go.go | 6 + internal/injector/config/config-yml.go | 15 ++- .../config/testdata/required.snap.yml | 4 +- internal/injector/config/validate_test.go | 122 +++++------------- internal/pin/gomod.go | 6 +- internal/pin/pin.go | 14 +- internal/version/version.go | 9 +- orchestrion.yml | 27 ---- samples/go.mod | 4 +- samples/go.sum | 4 +- 19 files changed, 127 insertions(+), 153 deletions(-) create mode 100644 internal/injector/config/builtin.go delete mode 100644 orchestrion.yml diff --git a/.github/actions/codecov-cli/action.yml b/.github/actions/codecov-cli/action.yml index e2316ea3e..66ce1e123 100644 --- a/.github/actions/codecov-cli/action.yml +++ b/.github/actions/codecov-cli/action.yml @@ -11,7 +11,7 @@ runs: - name: Setup Python uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5 with: - python-version: '3.x' + python-version: '3.12' cache-dependency-path: ${{ github.action_path }}/requirements-dev.txt - name: Setup Rust diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 0f7d1c0f8..4bdce3387 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -9,6 +9,7 @@ updates: groups: GitHub Actions: applies-to: version-updates + dependency-type: production - package-ecosystem: pip directory: /.github/actions/codecov-cli @@ -18,3 +19,4 @@ updates: groups: Python Dependencies: applies-to: version-updates + dependency-type: development diff --git a/.github/workflows/deps-update.yml b/.github/workflows/deps-update.yml index 8d2bda209..aa8255513 100644 --- a/.github/workflows/deps-update.yml +++ b/.github/workflows/deps-update.yml @@ -20,6 +20,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 - name: Set up Go + id: setup-go uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5 with: go-version: oldstable # Minimum supported go release @@ -37,9 +38,9 @@ jobs: # Passing "go@" to "go get -u" ensures no dependencies get upgraded to a release that # does not support that specific go release. - name: Update dependencies - run: find . -name go.mod -execdir go get -t -u go@1.22 gopkg.in/DataDog/dd-trace-go.v1@${{ steps.dd-trace-go.outputs.version }} ./... \; + run: find . -name go.mod -execdir go get -t -u "go@$(go mod edit -json | jq .Go)" gopkg.in/DataDog/dd-trace-go.v1@${{ steps.dd-trace-go.outputs.version }} ./... \; - name: Run go mod tidy - run: find . -name go.mod -execdir go mod tidy \; + run: find . -name go.mod -execdir go mod tidy -go="$(go mod edit -json | jq .Go)" \; - name: Ensure no toolchain directive run: find . -name go.mod -execdir go mod edit -toolchain=none \; diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 275f7f57e..1340f2d5b 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -33,7 +33,7 @@ jobs: id: setup-go uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5 with: - go-version: stable + go-version: oldstable cache-dependency-path: '**/go.mod' - name: Run 'go generate ./...' @@ -61,7 +61,7 @@ jobs: - name: Run 'go mod tidy' # Don't run for push, it's not necessary if: github.event_name != 'push' - run: find . -name go.mod -execdir go mod tidy \; + run: find . -name go.mod -execdir go mod tidy -go="$(go mod edit -json | jq .Go)" \; - name: Refresh LICENSE-3rdparty.csv run: ./_tools/make-licenses.sh diff --git a/_docs/go.mod b/_docs/go.mod index 41b4cf1a1..93135981f 100644 --- a/_docs/go.mod +++ b/_docs/go.mod @@ -1,6 +1,6 @@ module github.com/DataDog/orchestrion/_docs -go 1.22.10 +go 1.22.0 replace github.com/DataDog/orchestrion => .. diff --git a/_tools/go.mod b/_tools/go.mod index 06e6aa9f8..5c4471166 100644 --- a/_tools/go.mod +++ b/_tools/go.mod @@ -1,6 +1,6 @@ module github.com/DataDog/orchestrion/_tools -go 1.22.10 +go 1.22.0 require ( github.com/google/go-licenses/v2 v2.0.0-alpha.1 diff --git a/go.mod b/go.mod index cda55d70e..c897febf3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/DataDog/orchestrion -go 1.22.10 +go 1.22.0 require ( github.com/charmbracelet/lipgloss v1.0.0 diff --git a/instrument/go.mod b/instrument/go.mod index 7aacdc3ca..4b7711990 100644 --- a/instrument/go.mod +++ b/instrument/go.mod @@ -1,6 +1,6 @@ module github.com/DataDog/orchestrion/instrument -go 1.22.10 +go 1.22.5 replace github.com/DataDog/orchestrion => .. diff --git a/internal/injector/config/builtin.go b/internal/injector/config/builtin.go new file mode 100644 index 000000000..2a1d93329 --- /dev/null +++ b/internal/injector/config/builtin.go @@ -0,0 +1,48 @@ +// 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 config + +import ( + "github.com/DataDog/orchestrion/internal/injector/aspect" + "github.com/DataDog/orchestrion/internal/injector/aspect/advice" + "github.com/DataDog/orchestrion/internal/injector/aspect/advice/code" + "github.com/DataDog/orchestrion/internal/injector/aspect/context" + "github.com/DataDog/orchestrion/internal/injector/aspect/join" +) + +var builtIn = configGo{ + pkgPath: "github.com/DataDog/orchestrion", + yaml: &configYML{ + aspects: []*aspect.Aspect{ + { + ID: "built.WithOrchestrion", + TracerInternal: true, // This is safe to apply in the tracer itself + JoinPoint: join.AllOf( + join.ValueDeclaration(join.MustTypeName("bool")), + join.OneOf( + join.DeclarationOf("github.com/DataDog/orchestrion/runtime/built", "WithOrchestrion"), + join.Directive("dd:orchestrion-enabled"), + ), + ), + Advice: []advice.Advice{ + advice.AssignValue( + code.MustTemplate("true", nil, context.GoLangVersion{}), + ), + }, + }, + }, + name: "", + meta: configYMLMeta{ + name: "built.WithOrchestrion & //dd:orchestrion-enabled", + description: "Flip a boolean to true if Orchestrion is enabled.", + icon: "cog", + caveats: "This aspect allows introducing conditional logic based on whether" + + "Orchestrion has been used to instrument an application or not. This should" + + "generally be avoided, but can be useful to ensure the application (or tests)" + + "is running with instrumentation.", + }, + }, +} diff --git a/internal/injector/config/config-go.go b/internal/injector/config/config-go.go index 5cc87369d..b420cc568 100644 --- a/internal/injector/config/config-go.go +++ b/internal/injector/config/config-go.go @@ -25,6 +25,12 @@ var ErrInvalidGoPackage = errors.New("no .go files in package") // loadGoPackage loads configuration from the specified go package. func (l *Loader) loadGoPackage(pkg *packages.Package) (*configGo, error) { + // Special-case the `github.com/DataDog/orchestrion` package, we need not + // parse this one, and should always use the built-in object. + if pkg.PkgPath == builtIn.pkgPath { + return &builtIn, nil + } + root := packageRoot(pkg) if root == "" { // This might be explained by a package-level loading error... We only check diff --git a/internal/injector/config/config-yml.go b/internal/injector/config/config-yml.go index e5753b172..0b3b3513e 100644 --- a/internal/injector/config/config-yml.go +++ b/internal/injector/config/config-yml.go @@ -83,17 +83,20 @@ func (l *Loader) loadYMLFile(dir string, name string) (*configYML, error) { return cfg, nil } -type configYML struct { - extends []Config - aspects []*aspect.Aspect - name string - meta struct { +type ( + configYML struct { + extends []Config + aspects []*aspect.Aspect + name string + meta configYMLMeta + } + configYMLMeta struct { name string description string icon string caveats string } -} +) func (c *configYML) Aspects() []*aspect.Aspect { if c == nil { diff --git a/internal/injector/config/testdata/required.snap.yml b/internal/injector/config/testdata/required.snap.yml index 575c0c040..3e6093a0a 100644 --- a/internal/injector/config/testdata/required.snap.yml +++ b/internal/injector/config/testdata/required.snap.yml @@ -1,5 +1,5 @@ pkgpath: github.com/DataDog/orchestrion yaml: - name: orchestrion.yml + name: aspects: - - Initialize to true + - built.WithOrchestrion diff --git a/internal/injector/config/validate_test.go b/internal/injector/config/validate_test.go index a57471def..389560587 100644 --- a/internal/injector/config/validate_test.go +++ b/internal/injector/config/validate_test.go @@ -9,104 +9,13 @@ import ( "bytes" _ "embed" "fmt" - "os" - "path/filepath" - "runtime" - "strings" "sync" "testing" - "github.com/DataDog/orchestrion/internal/injector/config" "github.com/santhosh-tekuri/jsonschema/v6" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v3" ) -func TestBuiltinYAML(t *testing.T) { - _, thisFile, _, _ := runtime.Caller(0) - rootDir := filepath.Join(thisFile, "..", "..", "..", "..") - - yamlSegment := fmt.Sprintf("%[1]cyaml%[1]c", filepath.Separator) - - count := 0 - filepath.WalkDir(rootDir, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - - if d.IsDir() { - // Ignore `testdata` directories... - if d.Name() == "testdata" { - return filepath.SkipDir - } - // Ignores `.git` and other hidden directories... - if strings.HasPrefix(d.Name(), ".") { - return filepath.SkipDir - } - return nil - } - - if filepath.Ext(path) != ".yml" { - // Only interested in .yml files that aren't hidden files... - return nil - } - - if d.Name() != config.FilenameOrchestrionYML && !strings.Contains(path, yamlSegment) { - // Only look at `.yml` files that aren't `orchestrion.yml` if they're - // under a `yaml` directory... - return nil - } - - count++ - rel, err := filepath.Rel(rootDir, path) - require.NoError(t, err) - t.Run(rel, func(t *testing.T) { - file, err := os.Open(path) - require.NoError(t, err) - defer func() { require.NoError(t, file.Close()) }() - - var raw map[string]any - require.NoError(t, yaml.NewDecoder(file).Decode(&raw)) - require.NoError(t, config.ValidateObject(raw)) - }) - - return nil - }) - - require.Positive(t, count) -} - -var ( - //go:embed "schema.json" - schemaBytes []byte - schema *jsonschema.Schema - schemaOnce sync.Once -) - -func getSchema() *jsonschema.Schema { - schemaOnce.Do(compileSchema) - return schema -} - -func compileSchema() { - rawSchema, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaBytes)) - if err != nil { - panic(fmt.Errorf("parsing JSON schema: %w", err)) - } - mapSchema, _ := rawSchema.(map[string]any) - schemaURL, _ := mapSchema["$id"].(string) - - compiler := jsonschema.NewCompiler() - if err := compiler.AddResource(schemaURL, rawSchema); err != nil { - panic(fmt.Errorf("preparing JSON schema compiler: %w", err)) - } - - schema, err = compiler.Compile(schemaURL) - if err != nil { - panic(fmt.Errorf("compiling JSON schema: %w", err)) - } -} - func TestSchemaValidity(t *testing.T) { count := validateExamples(t, getSchema(), "", nil) // Make sure we verified some examples... @@ -172,3 +81,34 @@ func validateExamplesMap[K comparable](t *testing.T, schemas map[K]*jsonschema.S } return count } + +var ( + //go:embed "schema.json" + schemaBytes []byte + schema *jsonschema.Schema + schemaOnce sync.Once +) + +func getSchema() *jsonschema.Schema { + schemaOnce.Do(compileSchema) + return schema +} + +func compileSchema() { + rawSchema, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaBytes)) + if err != nil { + panic(fmt.Errorf("parsing JSON schema: %w", err)) + } + mapSchema, _ := rawSchema.(map[string]any) + schemaURL, _ := mapSchema["$id"].(string) + + compiler := jsonschema.NewCompiler() + if err := compiler.AddResource(schemaURL, rawSchema); err != nil { + panic(fmt.Errorf("preparing JSON schema compiler: %w", err)) + } + + schema, err = compiler.Compile(schemaURL) + if err != nil { + panic(fmt.Errorf("compiling JSON schema: %w", err)) + } +} diff --git a/internal/pin/gomod.go b/internal/pin/gomod.go index 25de374c3..eeab8a575 100644 --- a/internal/pin/gomod.go +++ b/internal/pin/gomod.go @@ -63,13 +63,13 @@ func parseGoMod(modfile string) (goMod, error) { // requires returns true if the `go.mod` file contains a require directive for // the designated module path. -func (m *goMod) requires(path string) bool { +func (m *goMod) requires(path string) (string, bool) { for _, r := range m.Require { if r.Path == path { - return true + return r.Version, true } } - return false + return "", false } // runGoGet executes the `go get ` subcommand with the provided diff --git a/internal/pin/pin.go b/internal/pin/pin.go index 6c5873cc1..9d7266aaf 100644 --- a/internal/pin/pin.go +++ b/internal/pin/pin.go @@ -24,6 +24,7 @@ import ( "github.com/dave/dst" "github.com/dave/dst/decorator" "github.com/rs/zerolog" + "golang.org/x/mod/semver" "golang.org/x/tools/go/packages" ) @@ -91,10 +92,10 @@ func PinOrchestrion(ctx context.Context, opts Options) error { return fmt.Errorf("parsing %q: %w", goMod, err) } - if !curMod.requires(datadogTracerV1) { - // TODO: Replace `@hash` with `@latest`. + if _, found := curMod.requires(datadogTracerV1); !found { log.Info().Msg("Installing " + datadogTracerV1) - if err := runGoGet(goMod, datadogTracerV1+"@02f0df46da0f7a3511d879ba11f680f06b9aa8fa"); err != nil { + // TODO: Replace `@main` with `@latest`. + if err := runGoGet(goMod, datadogTracerV1+"@main"); err != nil { return fmt.Errorf("go get "+datadogTracerV1+": %w", err) } } @@ -104,9 +105,10 @@ func PinOrchestrion(ctx context.Context, opts Options) error { edits = append(edits, goModVersion("1.22.0")) } - log.Info().Msg("Adding/updating require entry for " + orchestrionImportPath) - rawTag, _ := version.TagInfo() - edits = append(edits, goModRequire{Path: orchestrionImportPath, Version: rawTag}) + if ver, found := curMod.requires(orchestrionImportPath); !found || semver.Compare(ver, version.Tag()) < 0 { + log.Info().Msg("Adding/updating require entry for " + orchestrionImportPath) + edits = append(edits, goModRequire{Path: orchestrionImportPath, Version: version.Tag()}) + } if err := runGoModEdit(goMod, edits...); err != nil { return fmt.Errorf("editing %q: %w", goMod, err) diff --git a/internal/version/version.go b/internal/version/version.go index 3e8168106..97266dd67 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -38,13 +38,12 @@ func init() { return } + version := bi.Main.Version if bi.Main.Replace != nil { - if bi.Main.Replace.Version != "" { - buildInfoVersion = bi.Main.Replace.Version - } - return + version = bi.Main.Replace.Version } - switch bi.Main.Version { + + switch version { case "", "(devel)": // In tests, the [debug.BuildInfo.Main] has an empty version; and in builds // of the command, it has a "(devel)" version. diff --git a/orchestrion.yml b/orchestrion.yml deleted file mode 100644 index 8850ca7bc..000000000 --- a/orchestrion.yml +++ /dev/null @@ -1,27 +0,0 @@ -# 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. ---- -# yaml-language-server: $schema=./internal/injector/config/schema.json -meta: - name: built.WithOrchestrion & //dd:orchestrion-enabled - description: Flip a boolean to true if Orchestrion is enabled. - icon: cog - caveats: |- - This aspect allows introducing conditional logic based on whether - Orchestrion has been used to instrument an application or not. This should - generally be avoided, but can be useful to ensure the application (or tests) - is running with instrumentation. -aspects: - - id: Initialize to true - tracer-internal: true # This is safe to apply in the tracer itself. - join-point: - all-of: - - value-declaration: bool - - one-of: - - declaration-of: github.com/DataDog/orchestrion/runtime/built.WithOrchestrion - - directive: dd:orchestrion-enabled - advice: - - assign-value: - template: 'true' diff --git a/samples/go.mod b/samples/go.mod index 162fbc41a..ab54a42ed 100644 --- a/samples/go.mod +++ b/samples/go.mod @@ -1,6 +1,6 @@ module github.com/DataDog/orchestrion/samples -go 1.22.10 +go 1.22.5 replace ( github.com/DataDog/orchestrion => .. @@ -8,7 +8,7 @@ replace ( ) require ( - github.com/99designs/gqlgen v0.17.63 + github.com/99designs/gqlgen v0.17.62 github.com/DataDog/orchestrion v0.0.0-00010101000000-000000000000 github.com/DataDog/orchestrion/instrument v0.0.0-00010101000000-000000000000 github.com/IBM/sarama v1.45.0 diff --git a/samples/go.sum b/samples/go.sum index ead41b403..7982d78e7 100644 --- a/samples/go.sum +++ b/samples/go.sum @@ -18,8 +18,8 @@ cloud.google.com/go/pubsub v1.45.1 h1:ZC/UzYcrmK12THWn1P72z+Pnp2vu/zCZRXyhAfP1hJ cloud.google.com/go/pubsub v1.45.1/go.mod h1:3bn7fTmzZFwaUjllitv1WlsNMkqBgGUb3UdMhI54eCc= dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= -github.com/99designs/gqlgen v0.17.63 h1:HCdaYDPd9HqUXRchEvmE3EFzELRwLlaJ8DBuyC8Cqto= -github.com/99designs/gqlgen v0.17.63/go.mod h1:sVCM2iwIZisJjTI/DEC3fpH+HFgxY1496ZJ+jbT9IjA= +github.com/99designs/gqlgen v0.17.62 h1:Wovt1+XJN9dTWYh92537Y9a5FuMVSkrQL4bn0a8v5Rg= +github.com/99designs/gqlgen v0.17.62/go.mod h1:sVCM2iwIZisJjTI/DEC3fpH+HFgxY1496ZJ+jbT9IjA= github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 h1:bvDV9vkmnHYOMsOr4WLk+Vo07yKIzd94sVoIqshQ4bU= github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= github.com/AlecAivazis/survey/v2 v2.3.7 h1:6I/u8FvytdGsgonrYsVn2t8t4QiRnh6QSTqkkhIiSjQ=