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.

Sign up for GitHub

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

[Backport 7.64.x] Revert "autoinstrumentation: pin minor library versions" #34456

Merged
merged 1 commit into from
Feb 26, 2025
+110 −166
Merged

[Backport 7.64.x] Revert "autoinstrumentation: pin minor library versions" #34456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 54 additions & 66 deletions pkg/clusteragent/admission/mutate/autoinstrumentation/auto_instrumentation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ const commonRegistry = "gcr.io/datadoghq"

var (
defaultLibraries = map[string]string{
"java": "v1.46",
"python": "v2.21",
"ruby": "v2.10",
"dotnet": "v3.10",
"js": "v5.37",
"java": "v1",
"python": "v2",
"ruby": "v2",
"dotnet": "v3",
"js": "v5",
}

// TODO: Add new entry when a new language is supported
Expand All @@ -67,52 +67,6 @@ func defaultLibrariesFor(languages ...string) map[string]string {
return out
}

func TestDefaultLibVersions(t *testing.T) {
// N.B. This test uses js since there are many major versions.
tests := []struct {
in, out string
}{
{
in: "v5",
out: defaultLibImageVersions[js],
},
{
in: "5",
out: defaultLibImageVersions[js],
},
{
in: "default",
out: defaultLibImageVersions[js],
},
{
in: "v1",
out: "registry/dd-lib-js-init:v1",
},
{
in: "v1.0",
out: "registry/dd-lib-js-init:v1.0",
},
{
in: "5.0",
out: "registry/dd-lib-js-init:5.0",
},
{
in: "v5.0",
out: "registry/dd-lib-js-init:v5.0",
},
{
in: "latest",
out: "registry/dd-lib-js-init:latest",
},
}

for _, tt := range tests {
t.Run(fmt.Sprintf("%s->%s", tt.in, tt.out), func(t *testing.T) {
require.Equal(t, tt.out, js.libImageName("registry", tt.in))
})
}
}

func TestInjectAutoInstruConfigV2(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -498,12 +452,36 @@ func TestInjectAutoInstruConfigV2(t *testing.T) {
}

func TestExtractLibInfo(t *testing.T) {
var allLatestDefaultLibs []libInfo
for k, v := range defaultLibImageVersions {
allLatestDefaultLibs = append(allLatestDefaultLibs, libInfo{
lang: k,
image: v,
})
defaultLibImageVersions := map[string]string{
"java": "registry/dd-lib-java-init:v1",
"js": "registry/dd-lib-js-init:v5",
"python": "registry/dd-lib-python-init:v2",
"dotnet": "registry/dd-lib-dotnet-init:v3",
"ruby": "registry/dd-lib-ruby-init:v2",
}

// TODO: Add new entry when a new language is supported
allLatestDefaultLibs := []libInfo{
{
lang: "java",
image: defaultLibImageVersions["java"],
},
{
lang: "js",
image: defaultLibImageVersions["js"],
},
{
lang: "python",
image: defaultLibImageVersions["python"],
},
{
lang: "dotnet",
image: defaultLibImageVersions["dotnet"],
},
{
lang: "ruby",
image: defaultLibImageVersions["ruby"],
},
}

var mockConfig model.Config
Expand All @@ -522,7 +500,7 @@ func TestExtractLibInfo(t *testing.T) {
expectedLibsToInject: []libInfo{
{
lang: "java",
image: "registry/dd-lib-java-init:v1.46",
image: "registry/dd-lib-java-init:v1",
},
},
},
Expand All @@ -533,7 +511,7 @@ func TestExtractLibInfo(t *testing.T) {
expectedLibsToInject: []libInfo{
{
lang: "java",
image: "registry/dd-lib-java-init:v1.46",
image: "registry/dd-lib-java-init:v1",
},
},
},
Expand All @@ -544,7 +522,7 @@ func TestExtractLibInfo(t *testing.T) {
expectedLibsToInject: []libInfo{
{
lang: "java",
image: fmt.Sprintf("%s/dd-lib-java-init:v1.46", commonRegistry),
image: fmt.Sprintf("%s/dd-lib-java-init:v1", commonRegistry),
},
},
},
Expand Down Expand Up @@ -606,7 +584,10 @@ func TestExtractLibInfo(t *testing.T) {
},
containerRegistry: "registry",
expectedLibsToInject: []libInfo{
java.libInfo("", defaultLibImageVersions[java]),
{
lang: "java",
image: "registry/dd-lib-java-init:v1",
},
{
lang: "js",
image: "registry/dd-lib-js-init:v1",
Expand Down Expand Up @@ -635,7 +616,11 @@ func TestExtractLibInfo(t *testing.T) {
},
containerRegistry: "registry",
expectedLibsToInject: []libInfo{
java.libInfo("java-app", defaultLibImageVersions[java]),
{
ctrName: "java-app",
lang: "java",
image: "registry/dd-lib-java-init:v1",
},
{
ctrName: "node-app",
lang: "js",
Expand Down Expand Up @@ -762,7 +747,7 @@ func TestExtractLibInfo(t *testing.T) {
expectedLibsToInject: []libInfo{
{
lang: "java",
image: "registry/dd-lib-java-init:v1.46",
image: "registry/dd-lib-java-init:v1",
},
},
},
Expand Down Expand Up @@ -834,7 +819,10 @@ func TestExtractLibInfo(t *testing.T) {
pod: common.FakePodWithAnnotation("admission.datadoghq.com/java-lib.version", "v1"),
containerRegistry: "registry",
expectedLibsToInject: []libInfo{
java.libInfo("", defaultLibImageVersions[java]),
{
lang: "java",
image: "registry/dd-lib-java-init:v1",
},
},
setupConfig: func() {
mockConfig.SetWithoutSource("apm_config.instrumentation.enabled", true)
Expand All @@ -848,8 +836,8 @@ func TestExtractLibInfo(t *testing.T) {
containerRegistry: "registry",
expectedLibsToInject: []libInfo{
{
lang: php,
image: "registry/dd-lib-php-init:v1.6",
lang: "php",
image: "registry/dd-lib-php-init:v1",
},
},
},
Expand Down
98 changes: 9 additions & 89 deletions pkg/clusteragent/admission/mutate/autoinstrumentation/language_versions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ package autoinstrumentation
import (
"fmt"
"slices"
"strconv"
"strings"

corev1 "k8s.io/api/core/v1"

Expand All @@ -35,47 +33,13 @@ func (l language) defaultLibInfo(registry, ctrName string) libInfo {
}

func (l language) libImageName(registry, tag string) string {
switch tag {
case "latest":
// do nothing with this as a shortcut well known valid tag.
case defaultVersionMagicString:
if tag == defaultVersionMagicString {
tag = l.defaultLibVersion()
default:
if l.isDefaultVersionMoreSpecificThan(tag) {
tag = l.defaultLibVersion()
}
}

return fmt.Sprintf("%s/dd-lib-%s-init:%s", registry, l, tag)
}

func (l language) isDefaultVersionMoreSpecificThan(input string) bool {
v, ok := languageVersions[l]
if !ok {
return false
}

in, err := newLibraryVersion(input)
if err != nil {
return false
}

return isDefaultVersionMoreSpecific(v, in)
}

// isDefaultVersionMoreSpecific tells us whether or not we should use
// the default version even though a user-specified one might be set.
//
// For example if we have `v1.46` as the defaultVersion, a user setting
// `v1` should have us using `v1.46` but them specifying a more specific
// point release would leave that alone.
func isDefaultVersionMoreSpecific(s1, s2 *libraryVersion) bool {
// we only care if there is a minor version specified
// in the `inputVersion` and the majors match since our `languageVersions`
// will only be as specific as the minor.
return s1.major == s2.major && s2.segmentsSpecified == 1
}

func (l language) libInfo(ctrName, image string) libInfo {
return libInfo{
lang: l,
Expand Down Expand Up @@ -157,69 +121,25 @@ func (l language) isEnabledByDefault() bool {
// wishes to utilize the default version found in languageVersions.
const defaultVersionMagicString = "default"

type libraryVersion struct {
major, minor int64
segmentsSpecified int
original string
}

func newLibraryVersion(in string) (*libraryVersion, error) {
noV, _ := strings.CutPrefix(strings.TrimSpace(in), "v")
split := strings.Split(noV, ".")

var major, minor int64
outer:
for i, v := range split {
val, err := strconv.ParseInt(v, 10, 64)
if err != nil {
return nil, fmt.Errorf("error parsing version: %w", err)
}

switch i {
case 0:
major = val
case 1:
minor = val
break outer
}
}

return &libraryVersion{
major: major,
minor: minor,
segmentsSpecified: len(split),
original: in,
}, nil
}

func mustLibraryVersion(in string) *libraryVersion {
v, err := newLibraryVersion(in)
if err != nil {
panic(err)
}
return v
}

// languageVersions defines the major library versions we consider "default" for each
// supported language. If not set, we will default to "latest", see defaultLibVersion.
//
// If this language does not appear in supportedLanguages, it will not be injected.
var languageVersions = map[language]*libraryVersion{
java: mustLibraryVersion("v1.46"), // https://datadoghq.atlassian.net/browse/APMON-1064
dotnet: mustLibraryVersion("v3.10"), // https://datadoghq.atlassian.net/browse/APMON-1390
python: mustLibraryVersion("v2.21"), // https://datadoghq.atlassian.net/browse/APMON-1068
ruby: mustLibraryVersion("v2.10"), // https://datadoghq.atlassian.net/browse/APMON-1066
js: mustLibraryVersion("v5.37"), // https://datadoghq.atlassian.net/browse/APMON-1065
php: mustLibraryVersion("v1.6"), // https://datadoghq.atlassian.net/browse/APMON-1128
var languageVersions = map[language]string{
java: "v1", // https://datadoghq.atlassian.net/browse/APMON-1064
dotnet: "v3", // https://datadoghq.atlassian.net/browse/APMON-1390
python: "v2", // https://datadoghq.atlassian.net/browse/APMON-1068
ruby: "v2", // https://datadoghq.atlassian.net/browse/APMON-1066
js: "v5", // https://datadoghq.atlassian.net/browse/APMON-1065
php: "v1", // https://datadoghq.atlassian.net/browse/APMON-1128
}

func (l language) defaultLibVersion() string {
langVersion, ok := languageVersions[l]
if !ok {
return "latest"
}

return langVersion.original
return langVersion
}

type libInfo struct {
Expand Down
Loading