From 7e309988114841f8924c439fff035031930afa69 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 12 Jan 2024 04:20:14 -0800 Subject: [PATCH] `ELASTIC_APM_ACTIVATION_METHOD=K8S_ATTACH`, prefer `ELASTIC_APM_SERVER_URL`; integration.yml workflow tweak (#87) * test: drop redundant kind cluster creation in integration.yml workflow * use ELASTIC_APM_SERVER_URL instead of the plural; the plural works with java, but not the Node.js or other APM agents * set ELASTIC_APM_ACTIVATION_METHOD=K8S_ATTACH in code rather than values.yaml This means that custom 'webhookConfig.agent.*' names do not need to repeat this envvar. It *does* mean that newer Node.js and .NET APM agents that use the newer value (used to be 'K8S') will be required for the 'activation_method' to get set correctly. Closes: https://github.com/elastic/apm-k8s-attacher/issues/91 * fix tests * improve the workflow step name --- .github/workflows/integration.yml | 17 +++++++---------- charts/apm-attacher/values.yaml | 3 --- docs/apm-k8s-attacher.asciidoc | 8 ++++---- main_test.go | 2 +- patch.go | 5 +++++ patch_test.go | 9 ++++++--- test/nodejs/deployment.yaml | 1 + testdata/bad.yaml | 2 +- testdata/bad2.yaml | 2 +- testdata/good.yaml | 2 +- 10 files changed, 27 insertions(+), 24 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 5d1fdcd..5cbefad 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -1,6 +1,6 @@ name: Agents Integration Testing -on: +on: workflow_dispatch: push: branches: @@ -18,10 +18,10 @@ on: paths-ignore: - '**.md' - '**.asciidoc' - + env: AGENT_TESTS: java nodejs - + jobs: integration-test: runs-on: ubuntu-latest @@ -31,15 +31,12 @@ jobs: with: fetch-depth: 0 - - name: Create kind cluster - uses: helm/kind-action@v1.4.0 - - - name: Create Local Registry - run: + - name: Create Kind cluster and local Docker registry + run: sh test/mock/kind-with-registry.sh - name: Create Mock APM server image - run: + run: cd test/mock; docker build -t mock-apm-server .; docker tag mock-apm-server localhost:5001/registry/mock-apm-server; @@ -72,7 +69,7 @@ jobs: version: v3.11.2 - name: Install webhook - run: + run: cd charts; helm install webhook-chart apm-attacher/ --namespace=elastic-apm --create-namespace --values ../test/mock/test_values.yaml; sleep 5; diff --git a/charts/apm-attacher/values.yaml b/charts/apm-attacher/values.yaml index 1f48844..5cabcdc 100644 --- a/charts/apm-attacher/values.yaml +++ b/charts/apm-attacher/values.yaml @@ -30,13 +30,11 @@ webhookConfig: artifact: "/usr/agent/elastic-apm-agent.jar" environment: JAVA_TOOL_OPTIONS: "-javaagent:/elastic/apm/agent/elastic-apm-agent.jar" - ELASTIC_APM_ACTIVATION_METHOD: "K8S_ATTACH" nodejs: image: docker.elastic.co/observability/apm-agent-nodejs:latest artifact: "/opt/nodejs/node_modules/elastic-apm-node" environment: NODE_OPTIONS: "-r /elastic/apm/agent/elastic-apm-node/start" - ELASTIC_APM_ACTIVATION_METHOD: "K8S" dotnet: image: docker.elastic.co/observability/apm-agent-dotnet:latest artifact: "/usr/agent/apm-dotnet-agent" @@ -46,4 +44,3 @@ webhookConfig: CORECLR_PROFILER_PATH: "/usr/agent/apm-dotnet-agent/libelastic_apm_profiler.so" ELASTIC_APM_PROFILER_HOME: "/usr/agent/apm-dotnet-agent" ELASTIC_APM_PROFILER_INTEGRATIONS: "/usr/agent/apm-dotnet-agent/integrations.yml" - ELASTIC_APM_ACTIVATION_METHOD: "K8S" diff --git a/docs/apm-k8s-attacher.asciidoc b/docs/apm-k8s-attacher.asciidoc index dece2d4..3265a64 100644 --- a/docs/apm-k8s-attacher.asciidoc +++ b/docs/apm-k8s-attacher.asciidoc @@ -218,7 +218,7 @@ agents: image: docker.elastic.co/observability/apm-agent-java:latest artifact: "/usr/agent/elastic-apm-agent.jar" environment: - ELASTIC_APM_SERVER_URLS: "http://192.168.1.10:8200" + ELASTIC_APM_SERVER_URL: "http://192.168.1.10:8200" ELASTIC_APM_ENVIRONMENT: "dev" ELASTIC_APM_LOG_LEVEL: "debug" ELASTIC_APM_PROFILING_INFERRED_SPANS_ENABLED: "true" @@ -227,7 +227,7 @@ agents: image: docker.elastic.co/observability/apm-agent-java:1.44.0 <1> artifact: "/usr/agent/elastic-apm-agent.jar" environment: - ELASTIC_APM_SERVER_URLS: "http://192.168.1.11:8200" + ELASTIC_APM_SERVER_URL: "http://192.168.1.11:8200" ELASTIC_APM_ENVIRONMENT: "prod" ELASTIC_APM_LOG_LEVEL: "info" ELASTIC_APM_PROFILING_INFERRED_SPANS_ENABLED: "true" @@ -237,7 +237,7 @@ agents: artifact: "/opt/nodejs/node_modules/elastic-apm-node" environment: NODE_OPTIONS: "-r /elastic/apm/agent/elastic-apm-node/start" - ELASTIC_APM_SERVER_URLS: "http://192.168.1.11:8200" + ELASTIC_APM_SERVER_URL: "http://192.168.1.11:8200" ELASTIC_APM_SERVICE_NAME: "petclinic" ELASTIC_APM_LOG_LEVEL: "info" ---- @@ -255,5 +255,5 @@ your image sets will be ignored in favour of the value set here or in the https: You may not see data flow into the {stack} right away; that's normal. The addition of a pod annotation does not trigger an automatic restart. -Therefore, existing pods will will not be effected by the APM Attacher. Only new pods--as they are created via the natural lifecycle of a Kubernetes deployment--will be instrumented. +Therefore, existing pods will will not be affected by the APM Attacher. Only new pods--as they are created via the natural lifecycle of a Kubernetes deployment--will be instrumented. Restarting pods you'd like instrumented manually will speed up this process, but that workflow is too specific to individual deployments to make any recommendations. diff --git a/main_test.go b/main_test.go index 4663789..6598e4e 100644 --- a/main_test.go +++ b/main_test.go @@ -34,7 +34,7 @@ func TestYAMLParse(t *testing.T) { java := cfg.Agents["java"] assert.Equal(t, java.Image, "docker.com/elastic/agent-java:1.2.3") assert.Len(t, java.Environment, 5) - assert.Contains(t, java.Environment, "ELASTIC_APM_SERVER_URLS") + assert.Contains(t, java.Environment, "ELASTIC_APM_SERVER_URL") assert.Contains(t, java.Environment, "ELASTIC_APM_SERVICE_NAME") assert.Contains(t, java.Environment, "ELASTIC_APM_ENVIRONMENT") assert.Contains(t, java.Environment, "ELASTIC_APM_LOG_LEVEL") diff --git a/patch.go b/patch.go index fab20ec..d020757 100644 --- a/patch.go +++ b/patch.go @@ -143,6 +143,11 @@ func generateEnvironmentVariables(config agentConfig) ([]corev1.EnvVar, error) { Name: "ELASTIC_APM_SERVER_URL", Value: "http://$(HOST_IP):8200", }) } + if _, ok := config.Environment["ELASTIC_APM_ACTIVATION_METHOD"]; !ok { + vars = append(vars, corev1.EnvVar{ + Name: "ELASTIC_APM_ACTIVATION_METHOD", Value: "K8S_ATTACH", + }) + } for k, v := range kubernetesEnvironmentVariables { vars = append(vars, corev1.EnvVar{ Name: k, diff --git a/patch_test.go b/patch_test.go index 232b98f..74ddac9 100644 --- a/patch_test.go +++ b/patch_test.go @@ -37,9 +37,10 @@ func TestEnvVarPatch(t *testing.T) { patches := createEnvVariablesPatches(vars, true, 0) assert.Len(t, patches, 1) environmentVariables := patches[0].Value.([]corev1.EnvVar) - assert.Len(t, environmentVariables, 8) + assert.Len(t, environmentVariables, 9) assert.Equal(t, "ELASTIC_APM_SECRET_TOKEN", environmentVariables[0].Name) assert.Equal(t, "ELASTIC_APM_API_KEY", environmentVariables[1].Name) + assert.Equal(t, "ELASTIC_APM_ACTIVATION_METHOD", environmentVariables[2].Name) m := map[string]struct{}{ "KUBERNETES_NODE_NAME": {}, @@ -51,7 +52,7 @@ func TestEnvVarPatch(t *testing.T) { delete(m, envVar.Name) } assert.Len(t, m, 0) - customVars := environmentVariables[6:] + customVars := environmentVariables[7:] sort.Slice(customVars, func(i, j int) bool { return customVars[i].Name < customVars[j].Name }) @@ -66,12 +67,14 @@ func TestAddAPMServerURLIfNotPresent(t *testing.T) { patches := createEnvVariablesPatches(vars, true, 0) assert.Len(t, patches, 1) environmentVariables := patches[0].Value.([]corev1.EnvVar) - assert.Len(t, environmentVariables, 8) + assert.Len(t, environmentVariables, 9) assert.Equal(t, "ELASTIC_APM_SECRET_TOKEN", environmentVariables[0].Name) assert.Equal(t, "ELASTIC_APM_API_KEY", environmentVariables[1].Name) assert.Equal(t, "HOST_IP", environmentVariables[2].Name) assert.Equal(t, "ELASTIC_APM_SERVER_URL", environmentVariables[3].Name) assert.Equal(t, "http://$(HOST_IP):8200", environmentVariables[3].Value) + assert.Equal(t, "ELASTIC_APM_ACTIVATION_METHOD", environmentVariables[4].Name) + assert.Equal(t, "K8S_ATTACH", environmentVariables[4].Value) } func TestInitContainerPatch(t *testing.T) { diff --git a/test/nodejs/deployment.yaml b/test/nodejs/deployment.yaml index a311cf3..9526b39 100644 --- a/test/nodejs/deployment.yaml +++ b/test/nodejs/deployment.yaml @@ -1,3 +1,4 @@ +# This file isn't currently used, but can be nice for manual testing or demos. apiVersion: apps/v1 kind: Deployment metadata: diff --git a/testdata/bad.yaml b/testdata/bad.yaml index dda3336..dbf0785 100644 --- a/testdata/bad.yaml +++ b/testdata/bad.yaml @@ -1,7 +1,7 @@ agents: java: environment: - ELASTIC_APM_SERVER_URLS: "http://34.78.173.219:8200" + ELASTIC_APM_SERVER_URL: "http://34.78.173.219:8200" ELASTIC_APM_SERVICE_NAME: "petclinic" ELASTIC_APM_ENVIRONMENT: "test" ELASTIC_APM_LOG_LEVEL: "debug" diff --git a/testdata/bad2.yaml b/testdata/bad2.yaml index 03cf99a..ae37d8a 100644 --- a/testdata/bad2.yaml +++ b/testdata/bad2.yaml @@ -2,7 +2,7 @@ agents: java: image: docker.com/elastic/agent-java:1.2.3 environment: - ELASTIC_APM_SERVER_URLS: "http://34.78.173.219:8200" + ELASTIC_APM_SERVER_URL: "http://34.78.173.219:8200" ELASTIC_APM_SERVICE_NAME: "petclinic" ELASTIC_APM_ENVIRONMENT: "test" ELASTIC_APM_LOG_LEVEL: "debug" diff --git a/testdata/good.yaml b/testdata/good.yaml index cb40017..958fb2c 100644 --- a/testdata/good.yaml +++ b/testdata/good.yaml @@ -3,7 +3,7 @@ agents: image: docker.com/elastic/agent-java:1.2.3 artifact: foo environment: - ELASTIC_APM_SERVER_URLS: "http://34.78.173.219:8200" + ELASTIC_APM_SERVER_URL: "http://34.78.173.219:8200" ELASTIC_APM_SERVICE_NAME: "petclinic" ELASTIC_APM_ENVIRONMENT: "test" ELASTIC_APM_LOG_LEVEL: "debug"