Skip to content

Commit

Permalink
fix: load built-in aspects from binary instead of resolving YAML (#517)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
RomainMuller authored Jan 28, 2025
1 parent 30ee622 commit 0f72b63
Show file tree
Hide file tree
Showing 19 changed files with 127 additions and 153 deletions.
2 changes: 1 addition & 1 deletion .github/actions/codecov-cli/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ updates:
groups:
GitHub Actions:
applies-to: version-updates
dependency-type: production

- package-ecosystem: pip
directory: /.github/actions/codecov-cli
Expand All @@ -18,3 +19,4 @@ updates:
groups:
Python Dependencies:
applies-to: version-updates
dependency-type: development
5 changes: 3 additions & 2 deletions .github/workflows/deps-update.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -37,9 +38,9 @@ jobs:
# Passing "go@<version>" 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 \;

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ./...'
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion _docs/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/DataDog/orchestrion/_docs

go 1.22.10
go 1.22.0

replace github.com/DataDog/orchestrion => ..

Expand Down
2 changes: 1 addition & 1 deletion _tools/go.mod
Original file line number Diff line number Diff line change
@@ -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
Expand Down
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/DataDog/orchestrion

go 1.22.10
go 1.22.0

require (
github.com/charmbracelet/lipgloss v1.0.0
Expand Down
2 changes: 1 addition & 1 deletion instrument/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/DataDog/orchestrion/instrument

go 1.22.10
go 1.22.5

replace github.com/DataDog/orchestrion => ..

Expand Down
48 changes: 48 additions & 0 deletions internal/injector/config/builtin.go
Original file line number Diff line number Diff line change
@@ -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: "<built-in>",
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.",
},
},
}
6 changes: 6 additions & 0 deletions internal/injector/config/config-go.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 9 additions & 6 deletions internal/injector/config/config-yml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions internal/injector/config/testdata/required.snap.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
pkgpath: github.com/DataDog/orchestrion
yaml:
name: orchestrion.yml
name: <built-in>
aspects:
- Initialize to true
- built.WithOrchestrion
122 changes: 31 additions & 91 deletions internal/injector/config/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down Expand Up @@ -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))
}
}
6 changes: 3 additions & 3 deletions internal/pin/gomod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <modSpecs...>` subcommand with the provided
Expand Down
14 changes: 8 additions & 6 deletions internal/pin/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions internal/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 0f72b63

Please sign in to comment.