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

chore: migrate ebpf manager from device to distro name env #2401

Merged
merged 6 commits into from
Feb 7, 2025
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
2 changes: 2 additions & 0 deletions api/k8sconsts/envvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ const (
OdigosEnvVarNamespace = "ODIGOS_WORKLOAD_NAMESPACE"
OdigosEnvVarContainerName = "ODIGOS_CONTAINER_NAME"
OdigosEnvVarPodName = "ODIGOS_POD_NAME"
OdigosEnvVarDistroName = "ODIGOS_DISTRO_NAME"
)

func OdigosInjectedEnvVars() []string {
return []string{
OdigosEnvVarNamespace,
OdigosEnvVarContainerName,
OdigosEnvVarPodName,
OdigosEnvVarDistroName,
}
}
1 change: 1 addition & 0 deletions distros/yamls/python-enterprise.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ spec:
delimiter: ':'
agentDirectories:
- directoryName: "{{ODIGOS_AGENTS_DIR}}/python-ebpf"
- directoryName: "{{ODIGOS_AGENTS_DIR}}/python"
94 changes: 20 additions & 74 deletions instrumentor/controllers/agentenabled/pods_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
"github.com/odigos-io/odigos/common"
"github.com/odigos-io/odigos/distros"
"github.com/odigos-io/odigos/distros/distro"
"github.com/odigos-io/odigos/instrumentor/controllers/agentenabled/podswebhook"
"github.com/odigos-io/odigos/instrumentor/controllers/utils"
podutils "github.com/odigos-io/odigos/instrumentor/internal/pod"
webhookdeviceinjector "github.com/odigos-io/odigos/instrumentor/internal/webhook_device_injector"
Expand Down Expand Up @@ -92,7 +92,12 @@ func (p *PodsWebhook) Default(ctx context.Context, obj runtime.Object) error {
continue
}

containerVolumeMounted, err := injectOdigosToContainer(containerConfig, podContainerSpec)
if !containerConfig.AgentEnabled || containerConfig.OtelDistroName == "" {
// container config exists, but no agent should be injected by webhook to this container
continue
}

containerVolumeMounted, err := injectOdigosToContainer(containerConfig, podContainerSpec, *pw)
if err != nil {
logger.Error(err, "failed to inject ODIGOS agent to container")
continue
Expand All @@ -102,7 +107,7 @@ func (p *PodsWebhook) Default(ctx context.Context, obj runtime.Object) error {

if volumeMounted {
// only mount the volume if at least one container has a volume to mount
mountPodVolume(pod)
podswebhook.MountPodVolume(pod)
}

// Inject ODIGOS environment variables and instrumentation device into all containers
Expand Down Expand Up @@ -148,8 +153,6 @@ func (p *PodsWebhook) podWorkload(ctx context.Context, pod *corev1.Pod) (*k8scon

func (p *PodsWebhook) injectOdigosInstrumentation(ctx context.Context, pod *corev1.Pod, ic *odigosv1.InstrumentationConfig, pw *k8sconsts.PodWorkload) error {
logger := log.FromContext(ctx)
// Environment variables that remain consistent across all containers
commonEnvVars := getCommonEnvVars()

otelSdkToUse, err := getRelevantOtelSDKs(ctx, p.Client, *pw)
if err != nil {
Expand All @@ -175,16 +178,17 @@ func (p *PodsWebhook) injectOdigosInstrumentation(ctx context.Context, pod *core
continue
}

webhookdeviceinjector.InjectOdigosInstrumentationDevice(*pw, container, otelSdk, runtimeDetails)
webhookenvinjector.InjectOdigosAgentEnvVars(logger, *pw, container, otelSdk, runtimeDetails)

// Check if the environment variables are already present, if so skip inject them again.
if envVarsExist(container.Env, commonEnvVars) {
continue
// amir: 07 feb 2025. hard-coded temporary list which is removed once all distros migrate away from device
if (runtimeDetails.Language == common.JavascriptProgrammingLanguage && otelSdk == common.OtelSdkEbpfEnterprise) ||
(runtimeDetails.Language == common.GoProgrammingLanguage && otelSdk == common.OtelSdkEbpfCommunity) ||
(runtimeDetails.Language == common.JavaProgrammingLanguage && otelSdk == common.OtelSdkEbpfEnterprise) ||
(runtimeDetails.Language == common.MySQLProgrammingLanguage && otelSdk == common.OtelSdkEbpfEnterprise) {
// Skip device injection for distros that no longer use it
} else {
webhookdeviceinjector.InjectOdigosInstrumentationDevice(*pw, container, otelSdk, runtimeDetails)
}

containerNameEnv := corev1.EnvVar{Name: k8sconsts.OdigosEnvVarContainerName, Value: container.Name}
container.Env = append(container.Env, append(commonEnvVars, containerNameEnv)...)
webhookenvinjector.InjectOdigosAgentEnvVars(logger, *pw, container, otelSdk, runtimeDetails)

if shouldInjectServiceName(runtimeDetails.Language, otelSdk) {
// Ensure the serviceName is fetched only once per pod
Expand Down Expand Up @@ -215,20 +219,7 @@ func (p *PodsWebhook) injectOdigosInstrumentation(ctx context.Context, pod *core
return nil
}

func mountDirectory(containerSpec *corev1.Container, dir string) {
// TODO: assuming the directory always starts with {{ODIGOS_AGENTS_DIR}}. This should be validated.
// Should we return errors here to validate static values?
relativePath := strings.TrimPrefix(dir, distro.AgentPlaceholderDirectory+"/")
absolutePath := strings.ReplaceAll(dir, distro.AgentPlaceholderDirectory, k8sconsts.OdigosAgentsDirectory)
containerSpec.VolumeMounts = append(containerSpec.VolumeMounts, corev1.VolumeMount{
Name: k8sconsts.OdigosAgentMountVolumeName,
SubPath: relativePath,
MountPath: absolutePath,
ReadOnly: true,
})
}

func injectOdigosToContainer(containerConfig *odigosv1.ContainerAgentConfig, podContainerSpec *corev1.Container) (bool, error) {
func injectOdigosToContainer(containerConfig *odigosv1.ContainerAgentConfig, podContainerSpec *corev1.Container, pw k8sconsts.PodWorkload) (bool, error) {

distroName := containerConfig.OtelDistroName

Expand All @@ -239,38 +230,14 @@ func injectOdigosToContainer(containerConfig *odigosv1.ContainerAgentConfig, pod

volumeMounted := false
for _, agentDirectory := range distroMetadata.AgentDirectories {
mountDirectory(podContainerSpec, agentDirectory.DirectoryName)
podswebhook.MountDirectory(podContainerSpec, agentDirectory.DirectoryName)
volumeMounted = true
}
podswebhook.InjectOdigosK8sEnvVars(podContainerSpec, distroName, pw.Namespace)

return volumeMounted, nil
}

func mountPodVolume(pod *corev1.Pod) {
pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{
Name: k8sconsts.OdigosAgentMountVolumeName,
VolumeSource: corev1.VolumeSource{
HostPath: &corev1.HostPathVolumeSource{
Path: k8sconsts.OdigosAgentsDirectory,
},
},
})
}

func envVarsExist(containerEnv []corev1.EnvVar, commonEnvVars []corev1.EnvVar) bool {
envMap := make(map[string]struct{})
for _, envVar := range containerEnv {
envMap[envVar.Name] = struct{}{} // Inserting empty struct as value
}

for _, commonEnvVar := range commonEnvVars {
if _, exists := envMap[commonEnvVar.Name]; exists { // Checking if key exists
return true
}
}
return false
}

func getWorkloadKindAttributeKey(podWorkload *k8sconsts.PodWorkload) attribute.Key {
switch podWorkload.Kind {
case k8sconsts.WorkloadKindDeployment:
Expand Down Expand Up @@ -336,27 +303,6 @@ func shouldInjectServiceName(pl common.ProgrammingLanguage, otelsdk common.OtelS
return false
}

func getCommonEnvVars() []corev1.EnvVar {
return []corev1.EnvVar{
{
Name: k8sconsts.OdigosEnvVarNamespace,
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{
FieldPath: "metadata.namespace",
},
},
},
{
Name: k8sconsts.OdigosEnvVarPodName,
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{
FieldPath: "metadata.name",
},
},
},
}
}

// checks for the service name on the annotation, or fallback to the workload name
func (p *PodsWebhook) getServiceNameForEnv(ctx context.Context, logger logr.Logger, podWorkload *k8sconsts.PodWorkload) *string {
workloadObj := workload.ClientObjectFromWorkloadKind(podWorkload.Kind)
Expand Down
50 changes: 50 additions & 0 deletions instrumentor/controllers/agentenabled/podswebhook/env.go
Copy link
Collaborator

Choose a reason for hiding this comment

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

we already have `instrumentor/internal/webhook_env_injector"

I agree that they operate on different kinds of env vars, but I think those logics should be in the same package

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as I refactor this part, I intend to move the functions to the controller, similar to how it's done in the rollout directory

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package podswebhook

import (
"github.com/odigos-io/odigos/api/k8sconsts"
corev1 "k8s.io/api/core/v1"
)

func InjectOdigosK8sEnvVars(container *corev1.Container, distroName string, ns string) {

// check for existing env vars so we don't introduce them again
existingEnvNames := make(map[string]struct{})
for _, envVar := range container.Env {
existingEnvNames[envVar.Name] = struct{}{}
}

injectEnvVarToPodContainer(&existingEnvNames, container, k8sconsts.OdigosEnvVarContainerName, container.Name)
injectEnvVarToPodContainer(&existingEnvNames, container, k8sconsts.OdigosEnvVarDistroName, distroName)
injectEnvVarObjectFieldRefToPodContainer(&existingEnvNames, container, k8sconsts.OdigosEnvVarPodName, "metadata.name")
injectEnvVarToPodContainer(&existingEnvNames, container, k8sconsts.OdigosEnvVarNamespace, ns)
}

func injectEnvVarObjectFieldRefToPodContainer(existingEnvNames *map[string]struct{}, container *corev1.Container, envVarName, envVarRef string) {
if _, exists := (*existingEnvNames)[envVarName]; exists {
return
}

container.Env = append(container.Env, corev1.EnvVar{
Name: envVarName,
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{
FieldPath: envVarRef,
},
},
})

(*existingEnvNames)[envVarName] = struct{}{}
}

func injectEnvVarToPodContainer(existingEnvNames *map[string]struct{}, container *corev1.Container, envVarName, envVarValue string) {
if _, exists := (*existingEnvNames)[envVarName]; exists {
return
}

container.Env = append(container.Env, corev1.EnvVar{
Name: envVarName,
Value: envVarValue,
})

(*existingEnvNames)[envVarName] = struct{}{}
}
33 changes: 33 additions & 0 deletions instrumentor/controllers/agentenabled/podswebhook/mount.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package podswebhook

import (
"strings"

"github.com/odigos-io/odigos/api/k8sconsts"
"github.com/odigos-io/odigos/distros/distro"
corev1 "k8s.io/api/core/v1"
)

func MountDirectory(containerSpec *corev1.Container, dir string) {
// TODO: assuming the directory always starts with {{ODIGOS_AGENTS_DIR}}. This should be validated.
// Should we return errors here to validate static values?
relativePath := strings.TrimPrefix(dir, distro.AgentPlaceholderDirectory+"/")
absolutePath := strings.ReplaceAll(dir, distro.AgentPlaceholderDirectory, k8sconsts.OdigosAgentsDirectory)
containerSpec.VolumeMounts = append(containerSpec.VolumeMounts, corev1.VolumeMount{
Name: k8sconsts.OdigosAgentMountVolumeName,
SubPath: relativePath,
MountPath: absolutePath,
ReadOnly: true,
})
}

func MountPodVolume(pod *corev1.Pod) {
pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{
Name: k8sconsts.OdigosAgentMountVolumeName,
VolumeSource: corev1.VolumeSource{
HostPath: &corev1.HostPathVolumeSource{
Path: k8sconsts.OdigosAgentsDirectory,
},
},
})
}
28 changes: 28 additions & 0 deletions k8sutils/pkg/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,34 @@ var (
ErrContainerNotInPodSpec = errors.New("container not found in pod spec")
)

func LanguageAndSdk(pod *v1.Pod, containerName string, distroName string) (common.ProgrammingLanguage, common.OtelSdk, error) {
if distroName != "" {
// TODO: so we can remove the device slowly while having backward compatibility,
// we map here the distroNames one by one.
// this is temporary, and should be refactored once device is removed
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aside from removing the device fallback, is there another refactor needed here in the future?
From the comment, it seems like there is a generic way to convert distro name to language and sdk.

I assume the plan is to not have language and sdk in the futrure just use the distro name,
For that, we will need to modify the factories map keys used by the manager to be distro name.
Is that right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes exactly

switch distroName {
case "golang-community":
return common.GoProgrammingLanguage, common.OtelSdkEbpfCommunity, nil
case "golang-enterprise":
return common.GoProgrammingLanguage, common.OtelSdkEbpfEnterprise, nil
case "java-enterprise":
return common.JavaProgrammingLanguage, common.OtelSdkNativeEnterprise, nil
case "java-ebpf-instrumentations":
return common.JavaProgrammingLanguage, common.OtelSdkEbpfEnterprise, nil
case "python-enterprise":
return common.PythonProgrammingLanguage, common.OtelSdkEbpfEnterprise, nil
case "nodejs-enterprise":
return common.JavascriptProgrammingLanguage, common.OtelSdkEbpfEnterprise, nil
case "mysql-enterprise":
return common.MySQLProgrammingLanguage, common.OtelSdkEbpfEnterprise, nil
}
}

// TODO: this is fallback for migration from device (so that we can handle pods that have not been updated yet)
// remove this once device is removed
return LanguageSdkFromPodContainer(pod, containerName)
}

func LanguageSdkFromPodContainer(pod *v1.Pod, containerName string) (common.ProgrammingLanguage, common.OtelSdk, error) {
for i := range pod.Spec.Containers {
container := pod.Spec.Containers[i]
Expand Down
2 changes: 1 addition & 1 deletion odiglet/pkg/ebpf/distribution_matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type podDeviceDistributionMatcher struct{}
func (dm *podDeviceDistributionMatcher) Distribution(ctx context.Context, e K8sProcessDetails) (instrumentation.OtelDistribution, error) {
// get the language and sdk for this process event
// based on the pod spec and the container name from the process event
lang, sdk, err := odgiosK8s.LanguageSdkFromPodContainer(e.pod, e.containerName)
lang, sdk, err := odgiosK8s.LanguageAndSdk(e.pod, e.containerName, e.distroName)
if err != nil {
return instrumentation.OtelDistribution{}, fmt.Errorf("failed to get language and sdk: %w", err)
}
Expand Down
1 change: 1 addition & 0 deletions odiglet/pkg/ebpf/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
type K8sProcessDetails struct {
pod *corev1.Pod
containerName string
distroName string
pw *k8sconsts.PodWorkload
procEvent detector.ProcessEvent
}
Expand Down
11 changes: 11 additions & 0 deletions odiglet/pkg/ebpf/resolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ func (dr *k8sDetailsResolver) Resolve(ctx context.Context, event detector.Proces
return K8sProcessDetails{}, errContainerNameNotReported
}

distroName, found := distroNameFromProcEvent(event)
if !found {
// TODO: this is ok for migration period. Once device is removed, this should be an error
}

podWorkload, err := workload.PodWorkloadObjectOrError(ctx, pod)
if err != nil {
return K8sProcessDetails{}, fmt.Errorf("failed to find workload object from pod manifest owners references: %w", err)
Expand All @@ -35,6 +40,7 @@ func (dr *k8sDetailsResolver) Resolve(ctx context.Context, event detector.Proces
return K8sProcessDetails{
pod: pod,
containerName: containerName,
distroName: distroName,
pw: podWorkload,
procEvent: event,
}, nil
Expand Down Expand Up @@ -67,6 +73,11 @@ func containerNameFromProcEvent(event detector.ProcessEvent) (string, bool) {
return containerName, ok
}

func distroNameFromProcEvent(event detector.ProcessEvent) (string, bool) {
distronName, ok := event.ExecDetails.Environments[k8sconsts.OdigosEnvVarDistroName]
return distronName, ok
}

type k8sConfigGroupResolver struct{}

func (cr *k8sConfigGroupResolver) Resolve(ctx context.Context, d K8sProcessDetails, dist instrumentation.OtelDistribution) (K8sConfigGroup, error) {
Expand Down
5 changes: 0 additions & 5 deletions tests/common/assert/simple-demo-instrumented.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,6 @@ spec:
- "true"
containers:
- name: membership
resources:
limits:
instrumentation.odigos.io/go-ebpf-community: "1"
requests:
instrumentation.odigos.io/go-ebpf-community: "1"
status:
containerStatuses:
- name: membership
Expand Down
Loading